Repository navigation
Send one response for GET /api/activity - #67
Merged
Merged
Conversation
The /api/activity branch in _api had no return after sending the levels, so it fell through to the final 404 and wrote a second response on the same connection. A client reusing the kept-alive connection got that 404 as the answer to its next request. A raw-socket test sends two pipelined requests and checks that both get their own 200. Fixes #66. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Self-review rubric (run against CI on head of
Findings:
The fix is one line with an empirically confirmed regression test, and CI is green. This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
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
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
GET /api/activity(andHEAD) insrc/arcade/server.py_apiwas missing areturnafter its 200 response. Control fell through to the trailing404 not found, so a second response was written on the same keep-alive connection, and the client's next request on that connection received the stray 404. Onereturnis added.test_activity_answers_once_on_a_kept_alive_connection, sends two pipelined requests over a raw socket and asserts exactly two200 OKstatus lines, with the second body being/version.json's. A raw socket is used becausehttp.clientreads ahead and silently discards the extra bytes, which is why the existing activity test never caught this.## [Unreleased]→### Fixed.Closes #66
Test plan
python3 -m pytest -q: 408 passed, 13 skipped (local, Python 3)server.pyfix stashed, the new test FAILS (only one status line arrives before the stray 404); with the fix restored, it PASSEScheck-registry examples/games.yamlwas run by CI (the local run was blocked by the sandbox)test (3.8),test (3.12),dockerDeferred
Coverage note
The request routing is covered by the loopback server tests. The behaviour behind Traefik's pooled connections is not exercised by CI, but the fix only stops an extra write, so nothing more is expected to change there.
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
🤖 Generated with Claude Code
drafted by Claude on behalf of Daniel Stephenson