Skip to content

fix: log only replies that were sent, keep the drain line after a second signal, skip logging work when off - #105

Merged
cristiangirlea merged 1 commit into
mainfrom
fix/log-only-sent-replies
Oct 26, 2026
Merged

cristiangirlea merged 1 commit into
mainfrom
fix/log-only-sent-replies

Conversation

@cristiangirlea

Copy link
Copy Markdown
Owner

Fixes #104.

Three defects found by review after the logging and owner-cap merges, none of which leaks anything:

  • A reply that was never delivered could be logged as sent: the status was recorded before the write and the request line written from a finally, so a client reset mid-reply still logged a 200, and the busy 503 was logged even when its non-blocking send raised. A reply is now recorded as sent only after its body is written (the send_error override does the same after the standard library's whole reply, including a headers-only HEAD refusal), and the busy 503 is logged only when the send delivered every byte; otherwise no request line, as for a connection that ends before any reply. The protocol guide's Logging section says so.
  • The drain line could be lost after a second signal, because server_close waited one second for the drain thread. The thread now publishes its fields and sets an Event before calling shutdown(), and server_close writes the line from that result if the thread has not, exactly once and before stop, waiting up to five seconds; past that, stop proceeds without the line, which the guide also says.
  • With BRIDGE_LOG=off the request timer, the route and method helpers, the job timings and the drain's snapshot still ran; a cheap log.enabled() check now skips them.

Tests, seen failing first where the behaviour was wrong: a reply whose first write fails writes no request line, nor does a failed or short busy send; a drain thread held past one second still writes exactly one drain line with interrupted: true before stop, and past the five-second wait writes none; with logging off the timer and field construction are skipped, not just the output; captured lines for a 429 and a 503 draining with status, owner, schema and no sentinel; and the owner cap with cancelled jobs (concurrency 1, cap 2: cancelled jobs keep the cap until one expires, while another owner still submits).

Files: worker/ai_bridge/server.py, worker/ai_bridge/jobs.py, worker/ai_bridge/log.py, tests/python/test_server.py, tests/python/test_jobs.py, tests/python/test_log.py, docs/protocol.md, docs/testing.md, CHANGELOG.md.

Checks: the compose suites could not run locally because Docker Desktop stopped responding on the development machine mid-run ("Docker Desktop is unable to start"); 51 targeted tests ran green in the containers before that. The whole Python suite was then run directly on the Windows host with Python 3.13: 415 tests, with the same fifteen platform-only failures as main on that host (file-mode and shell-script cases that the containers cover) and one order-dependent error in test_a_busy_503_is_logged_without_anything_read, which passes when run alone. The CI gate on this pull request is therefore the check of record for this head; it runs the full sequence including ruff and mypy at strict. Review: independent review by Codex (gpt-6.1-sol): no defect, every listed test present and distinguishing main's behaviour.

@cristiangirlea
cristiangirlea merged commit 54ec959 into main Oct 26, 2026
13 checks passed
@cristiangirlea
cristiangirlea deleted the fix/log-only-sent-replies branch October 26, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: log only replies that were sent, keep the drain line after a second signal, skip logging work when off

1 participant