From 769531e94fe931fb8fc0ecf431b2215b97df581b Mon Sep 17 00:00:00 2001 From: nicodes Date: Sun, 6 Sep 2026 16:06:33 -0600 Subject: [PATCH] Fix configuration HTTP deadlines across stalled frames --- .github/actions/test/action.yml | 20 ++++-- README.md | 20 +++++- addon/plugin.cfg | 2 +- addon/src/env_json_module.gd | 43 +++++++++-- tests/http_deadline_test.gd | 124 ++++++++++++++++++++++++++++++++ tests/test.sh | 11 ++- 6 files changed, 205 insertions(+), 15 deletions(-) create mode 100644 tests/http_deadline_test.gd diff --git a/.github/actions/test/action.yml b/.github/actions/test/action.yml index ecdedef..89721f9 100644 --- a/.github/actions/test/action.yml +++ b/.github/actions/test/action.yml @@ -28,12 +28,20 @@ runs: set -euo pipefail test -f addon/plugin.cfg - # Godot comes from a LOCAL action, copied from the one castledrop and prizm - # carry. The version used to be a download URL written into ci.yml -- the - # engine this addon is tested against lived in workflow YAML. - - uses: ./.github/actions/setup-godot - with: - godot-version: '4.4.1' + - name: Install verified Godot test binary + shell: bash + run: | + set -euo pipefail + archive="$RUNNER_TEMP/gd-env-godot.zip" + directory="$RUNNER_TEMP/gd-env-godot" + curl --fail --location --retry 3 --max-time 180 \ + https://github.com/godotengine/godot-builds/releases/download/4.7.2-stable/Godot_v4.7.2-stable_linux.x86_64.zip \ + --output "$archive" + printf '%s %s\n' '9aa00f7a605200940bce3027a567b782f49bd8e940dd06ae9e987bd65aee1b1467edd56ed84fcdcbdd44354bf613bdbb4e5d2913e925850368e150c59ed54c65' "$archive" | sha512sum --check + mkdir -p "$directory" + unzip -o "$archive" Godot_v4.7.2-stable_linux.x86_64 -d "$directory" + chmod +x "$directory/Godot_v4.7.2-stable_linux.x86_64" + echo "GODOT_BIN=$directory/Godot_v4.7.2-stable_linux.x86_64" >> "$GITHUB_ENV" # No `if: hashFiles(...)` guard. This step used to skip itself when # tests/test.sh was absent, which is indistinguishable from the script being diff --git a/README.md b/README.md index d7cdc60..8043f5f 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,24 @@ var api_url: String = str(dotenv.get("API_URL", "http://localhost:3000")) - Treat client-side config as public. Do not ship secrets in exported games. - Use game code to decide which config source wins when multiple sources are available. +## HTTP deadlines + +`load_dict_from_http` measures `timeout_s` from request start using +[monotonic ticks](https://docs.godotengine.org/en/4.7/classes/class_time.html). +A zero timeout disables the deadline; negative and nonfinite values fail with +`invalid_timeout`. The request belongs to the supplied node and is cancelled +when that owner is freed. Callbacks complete once for success or failure. +`LoadResult.request_result` preserves `HTTPRequest.Result` for transport outcomes +and is `-1` when no HTTP completion occurred. Existing error strings are retained. + +Godot 4.7.2's HTTP timeout can consume the frame preceding request start. An +isolated reproduction expires a new one-second request in under one millisecond +after a 1.2-second frame. Owned monotonic deadlines avoid that stale frame delta. +Nonblocking HTTP polling also lets cancellation finish when the peer withholds +its response; native threaded blocking reads can otherwise stall cancellation. +The loopback regression suite exercises those paths, time scale zero, successful +completion, disabled deadlines and owner cleanup. + ## Repository Layout - `addon/`: Godot plugin source packaged for GDAM and manual installation. @@ -60,7 +78,7 @@ Run locally with: ./tests/test.sh ``` -CI runs the same test script when available. +CI and releases run the same bounded script with a checksum-verified Godot 4.7.2 binary. Runtime errors and missing PASS markers fail the suite, including when the engine exits zero. ## License diff --git a/addon/plugin.cfg b/addon/plugin.cfg index 40571f3..88ece14 100644 --- a/addon/plugin.cfg +++ b/addon/plugin.cfg @@ -2,5 +2,5 @@ name="GD Env" description="Environment/config loading helpers (dotenv + JSON + OS env)." author="Avior Studio" -version="0.0.1" +version="0.0.2" script="plugin.gd" diff --git a/addon/src/env_json_module.gd b/addon/src/env_json_module.gd index ac30011..b17a324 100644 --- a/addon/src/env_json_module.gd +++ b/addon/src/env_json_module.gd @@ -8,6 +8,8 @@ class LoadResult extends RefCounted: var source: String var status_code: int var data: Dictionary[String, Variant] + ## HTTPRequest.Result, or -1 when no HTTP completion occurred. + var request_result: int = -1 var error_message: String func _init( @@ -23,6 +25,25 @@ class LoadResult extends RefCounted: self.data = data self.error_message = error_message +## Owns a wall-clock deadline without charging the frame before request start. +## HTTPRequest's built-in Timer can consume a stale process step on startup. +class DeadlineHttpRequest extends HTTPRequest: + var _deadline_usec: int = 0 + + func arm_deadline(timeout_s: float) -> void: + _deadline_usec = Time.get_ticks_usec() + int(timeout_s * 1000000.0) if timeout_s > 0.0 else 0 + set_process(_deadline_usec > 0) + + func disarm_deadline() -> void: + _deadline_usec = 0 + set_process(false) + + func _process(_delta: float) -> void: + if _deadline_usec > 0 and Time.get_ticks_usec() >= _deadline_usec: + disarm_deadline() + cancel_request() + request_completed.emit(HTTPRequest.RESULT_TIMEOUT, 0, PackedStringArray(), PackedByteArray()) + ## Loads the first existing file path from the provided candidate list. static func load_dict_from_first_existing(paths: PackedStringArray) -> LoadResult: for path: String in paths: @@ -66,9 +87,15 @@ static func load_dict_from_http( callback.call(LoadResult.new(false, url, 0, {}, "empty_url")) return - var request_node := HTTPRequest.new() - request_node.use_threads = not OS.has_feature("web") - request_node.timeout = timeout_s + if not is_finite(timeout_s) or timeout_s < 0.0: + callback.call(LoadResult.new(false, url, 0, {}, "invalid_timeout")) + return + + var request_node := DeadlineHttpRequest.new() + # Native threaded HTTP uses blocking reads; cancellation can wait on the peer. + # Configuration requests use nonblocking polling on every platform. + request_node.use_threads = false + request_node.timeout = 0.0 owner.add_child(request_node) var final_url: String = _resolve_web_relative_url(url) @@ -76,7 +103,9 @@ static func load_dict_from_http( final_url = _with_query_param(final_url, cache_bust_key, str(Time.get_unix_time_from_system())) var handler: Callable = func(result: int, response_code: int, _headers: PackedStringArray, body: PackedByteArray) -> void: + request_node.disarm_deadline() var out := LoadResult.new(false, final_url, response_code, {}, "") + out.request_result = result if result != HTTPRequest.RESULT_SUCCESS: out.error_message = "request_failed" @@ -87,15 +116,19 @@ static func load_dict_from_http( out = parse_json_dict(body_text) out.source = final_url out.status_code = response_code + out.request_result = result + request_node.queue_free() if callback.is_valid(): callback.call(out) - request_node.queue_free() - request_node.request_completed.connect(handler) + request_node.request_completed.connect(handler, CONNECT_ONE_SHOT) + request_node.arm_deadline(timeout_s) var err: int = request_node.request(final_url, PackedStringArray(), HTTPClient.METHOD_GET) if err != OK: + request_node.request_completed.disconnect(handler) + request_node.disarm_deadline() request_node.queue_free() callback.call(LoadResult.new(false, final_url, 0, {}, "request_error_" + str(err))) diff --git a/tests/http_deadline_test.gd b/tests/http_deadline_test.gd new file mode 100644 index 0000000..312a4da --- /dev/null +++ b/tests/http_deadline_test.gd @@ -0,0 +1,124 @@ +extends SceneTree + +const EnvJsonModule = preload("res://addon/src/env_json_module.gd") + +class HttpFixture extends Node: + var server := TCPServer.new() + var peers: Array[StreamPeerTCP] = [] + var respond: bool = true + var body: String = '{"ready":true}' + + func _process(_delta: float) -> void: + if server.is_connection_available(): + peers.append(server.take_connection()) + for peer: StreamPeerTCP in peers: + peer.poll() + if respond and peer.get_status() == StreamPeerTCP.STATUS_CONNECTED and peer.get_available_bytes() > 0: + peer.get_data(peer.get_available_bytes()) + var payload := body.to_utf8_buffer() + peer.put_data(("HTTP/1.1 200 OK\r\nContent-Length: %d\r\nConnection: close\r\n\r\n" % payload.size()).to_utf8_buffer()) + peer.put_data(payload) + + func _exit_tree() -> void: + for peer: StreamPeerTCP in peers: + peer.disconnect_from_host() + server.stop() + +var failures: Array[String] = [] +var results: Array[EnvJsonModule.LoadResult] = [] +var fixture: HttpFixture +var request_owner: Node +var url: String + +func _initialize() -> void: + call_deferred("_run") + +func _run() -> void: + fixture = HttpFixture.new() + root.add_child(fixture) + if fixture.server.listen(0, "127.0.0.1") != OK: + push_error("Cannot bind HTTP regression fixture") + quit(1) + return + url = "http://127.0.0.1:%d/config" % fixture.server.get_local_port() + request_owner = Node.new() + root.add_child(request_owner) + + # Start in process_frame following a long frame. A newly started built-in + # HTTPRequest timeout of 1s expires immediately from the preceding 1.2s. + await process_frame + OS.delay_msec(1200) + await process_frame + EnvJsonModule.load_dict_from_http(request_owner, url, _capture, 1.0, false) + await _wait_for_result() + _check(results.size() == 1 and results[0].success, "Stalled previous frame must not expire a new HTTP request") + if results.size() == 1: + _check(results[0].data.get("ready") == true, "HTTP response dictionary must reach callback") + _check(results[0].request_result == HTTPRequest.RESULT_SUCCESS, "Preserve successful transport result") + await _settle(1.1) + _check(results.size() == 1, "Completed request must not later time out") + _check(request_owner.get_child_count() == 0, "Completed request must release its owner child") + + results.clear() + fixture.respond = false + var started := Time.get_ticks_usec() + Engine.time_scale = 0.0 + EnvJsonModule.load_dict_from_http(request_owner, url, _capture, 0.15, false) + await _wait_for_result() + Engine.time_scale = 1.0 + _check(Time.get_ticks_usec() - started >= 150000, "Deadline must not expire before elapsed wall time") + _check(results.size() == 1 and not results[0].success, "Unanswered HTTP request must time out") + if results.size() == 1: + _check(results[0].request_result == HTTPRequest.RESULT_TIMEOUT, "Preserve exact transport timeout result") + await _settle(0.2) + _check(results.size() == 1 and request_owner.get_child_count() == 0, "Timeout must complete once and release request") + + results.clear() + EnvJsonModule.load_dict_from_http(request_owner, url, _capture, 0.1, false) + request_owner.queue_free() + await _settle(0.2) + _check(results.is_empty(), "Freed owner must cancel request and deadline without a callback") + + request_owner = Node.new() + root.add_child(request_owner) + results.clear() + EnvJsonModule.load_dict_from_http(request_owner, url, _capture, 0.0, false) + await _settle(0.2) + _check(results.is_empty(), "Zero deadline must disable timeout") + fixture.respond = true + await _wait_for_result() + _check(results.size() == 1 and results[0].success, "Zero deadline request must still complete") + await _settle(0.05) + for invalid: float in [-1.0, NAN, INF]: + results.clear() + EnvJsonModule.load_dict_from_http(request_owner, url, _capture, invalid, false) + _check(results.size() == 1 and results[0].error_message == "invalid_timeout", "Invalid deadline must fail synchronously") + _check(request_owner.get_child_count() == 0, "Invalid deadline must not allocate a request") + + request_owner.queue_free() + fixture.queue_free() + await process_frame + if failures.is_empty(): + print("PASS gd-env http_deadline_test") + quit(0) + else: + for failure: String in failures: + push_error(failure) + quit(1) + +func _capture(result: EnvJsonModule.LoadResult) -> void: + results.append(result) + +func _wait_for_result() -> void: + var stop := Time.get_ticks_usec() + 3000000 + while results.is_empty() and Time.get_ticks_usec() < stop: + await process_frame + +func _settle(seconds: float) -> void: + var stop := Time.get_ticks_usec() + int(seconds * 1000000.0) + while Time.get_ticks_usec() < stop: + await process_frame + +func _check(condition: bool, message: String) -> void: + if not condition: + failures.append(message) diff --git a/tests/test.sh b/tests/test.sh index b139473..a7ecf27 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -4,10 +4,17 @@ SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) ROOT_DIR=$(cd "$SCRIPT_DIR/.." && pwd) GODOT="${GODOT_BIN:-godot}" FAILURES=0 +LOG_DIR=$(mktemp -d) +trap 'rm -rf "$LOG_DIR"' EXIT +export XDG_DATA_HOME="$LOG_DIR/data" +export XDG_CONFIG_HOME="$LOG_DIR/config" for test in "$SCRIPT_DIR"/*_test.gd; do echo "Running $(basename "$test")..." - if ! "$GODOT" --headless --path "$ROOT_DIR" --script "$test" 2>&1; then + log="$LOG_DIR/$(basename "$test").log" + if ! timeout 30 "$GODOT" --headless --path "$ROOT_DIR" --script "$test" 2>&1 | tee "$log"; then + FAILURES=$((FAILURES + 1)) + elif grep -Eq '^(ERROR:|SCRIPT ERROR:|FAIL:)' "$log" || ! grep -q '^PASS gd-env ' "$log"; then FAILURES=$((FAILURES + 1)) fi done -exit $FAILURES \ No newline at end of file +exit $FAILURES