Fix with_server.py server hangs, orphaned processes, and silent failures#1400
Open
j0usa-commits wants to merge 1 commit into
Open
Fix with_server.py server hangs, orphaned processes, and silent failures#1400j0usa-commits wants to merge 1 commit into
j0usa-commits wants to merge 1 commit into
Conversation
Three related fixes to webapp-testing's with_server.py: 1. Redirect server output to a log file instead of subprocess.PIPE. The pipes were never read, so any server that logs steadily (npm run dev, or http.server logging each request) fills the ~64KB pipe buffer and blocks mid-run, freezing the server and hanging the test. Reproduced: a stock http.server froze after ~1000 requests. 2. Start each server in its own process group and stop the whole group on cleanup. With shell=True, terminate() only killed the wrapper shell; the actual server survived and kept the port bound, so the next run would silently test a stale server. Reproduced with the documented "cd backend && python server.py" usage pattern. 3. Include the tail of the server log in the startup-failure error. Previously the server's real error (stuck in the unread pipe) was never shown, leaving "failed to start within 30s" as the only clue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LKmYFWxnQJe4HaxBQTjyS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three bugs in
webapp-testing/scripts/with_server.py, the helper that starts dev servers, waits for them to be ready, runs a test command, and cleans up.1. Chatty servers freeze mid-run
Server output went to
subprocess.PIPE, but the pipes were never read. Once a server logged ~64KB (about a thousand log lines), its next write blocked on the full pipe buffer and the server stopped responding entirely. Any per-request logger triggers this —npm run dev, vite, Flask debug mode, Python'shttp.server.http.serverunder a 3,000-request client froze at ~1,100 requests; the client timed out.2. Cleanup leaves the real server running
With
shell=True,process.terminate()killed only the wrapper shell — the actual server survived with the port still bound. The next run then found the port open, reported "ready," and silently ran tests against the stale server. This happens with the docstring's own recommended usage (--server "cd backend && python server.py").start_new_session=True) and cleanup signals the whole group (SIGTERM, then SIGKILL after 5s); the port is released and no process remains. Non-POSIX platforms keep the old terminate/kill behavior.3. Startup failures hid the server's error
A server that crashed on boot wrote its error to the never-read pipe, so users saw only
Server failed to start on port X within 30s.FATAL: missing DATABASE_URL configproduced just the generic timeout error.