Codex/fix http connection issues in tests - #15
Conversation
Add startup logging and better failure handling to RemoteGatewaySmokeTest. Introduces _slurp_if_exists helper and a gateway_startup.log file; child process redirects STDOUT/STDERR to this log, and we now check the gateway script exists and die if exec fails. In the parent, _wait_for_port result is inspected: on failure we collect child exit status, slurp and emit the startup log via diag, kill the child if still running, and return early. These changes make test failures easier to debug and ensure the forked process is cleaned up.
Retry picking a free TCP port and improve gateway launch diagnostics. _pick_random_free_port now attempts socket bind up to 20 times with a short sleep instead of returning a random fallback. start() now tries launching the remote gateway up to 3 times, writes per-attempt startup logs, uses the current perl interpreter ($^X) for exec, checks readiness with _wait_for_port, and emits richer diagnostic info (raw wait status, exit and signal) if a child exits early. Ensures child is killed/waited on when it fails to bind and surfaces the final startup log on failure.
Allow _wait_for_port to accept an optional $tries parameter (defaulting to 300) instead of the previous hardcoded 40 iterations. Update the call site in start() to pass 300 so the gateway startup check waits longer for the port to become available. This makes the smoke test more robust against slower startups.
Replace the previous random-port probing in tests with a ready-file handshake so the gateway can bind to an ephemeral port and report it back. RemoteGatewaySmokeTest: remove _pick_random_free_port, add _wait_for_ready_file_port, write a ready file path, start the gateway with listen-port 0 and wait for the ready file to obtain the actual port before probing. tools/remote_gateway.pl: add --ready-file option, set listen_port from the bound health socket, and write a JSON ready file (pid, listen_host, listen_port, socket, ready) when the gateway starts. This makes test startup more reliable and avoids races around port selection.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23fc217c09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while (index($raw, "\r\n\r\n") == -1 && length($raw) < $max_header) { | ||
| my $chunk = ''; | ||
| my $got = sysread($client, $chunk, 4096); | ||
| if (!defined $got || $got <= 0) { |
There was a problem hiding this comment.
handle_http_client reads headers with blocking sysread until \r\n\r\n appears, and this runs on the single event-loop thread. If a client connects and sends only a partial header (or stalls after connect), this loop blocks indefinitely and the gateway stops serving all other health/command/websocket traffic, so one slow client can DoS the service. Add a bounded timeout/nonblocking path for header reads (similar to read_exact_bytes).
Useful? React with 👍 / 👎.
| close $rfh; | ||
| } | ||
| load_users(); | ||
| load_sessions(); |
There was a problem hiding this comment.
The ready marker is written before load_users() and load_sessions() run, so startup failures in auth/session initialization (for example, missing or malformed --users-file) still leave a ready: true file behind. Any supervisor or test harness that keys off this file can treat a failed boot as healthy; write the ready file only after all required initialization has succeeded.
Useful? React with 👍 / 👎.
…ix-http-connection-issues-in-tests
1 participant
No description provided.