fix(queue): retry queued requests on transient errors instead of dropping them

[ErikBjare]

Problem

Diagnosed while investigating sustained aw-server CPU load (see ActivityWatch/aw-core#147): the server was 503-rejecting heartbeats (Heartbeat lock could not be acquired within timeout), and it turned out each rejection permanently lost data, despite the on-disk request queue existing precisely to prevent that.

Three bugs in RequestQueue._dispatch_request's error classification:

1. HTTP 503 dropped: any status other than 400/500 fell into the Unknown error, not retrying branch, which pops the request from the persistqueue permanently. 503 Service Unavailable is the most retry-worthy status there is.
2. The 400/500 branches were dead code: Response.__bool__ returns Response.ok, which is False for every error status — so if e.response and e.response.status_code == 400 never matched, and all HTTP errors (including the 500s the code claimed to retry) were dropped.
3. Queue drained on server outage: a plain ConnectionError mid-dispatch (server died after connect; only ConnectTimeout was caught) hit the drop branch, and since connected stayed True the dispatch loop kept popping and discarding queued requests one by one — draining the entire on-disk queue into the void, the exact scenario the class docstring says it protects against.

Fix

No new retry machinery — the persistqueue is the retry mechanism. "Retry" here means returning without _task_done() so the request stays at the head of the queue, exactly the path ConnectTimeout already took. The change is purely classification:

• Keep queued & retry: ConnectionError/Timeout (also sets connected = False so the run loop reconnects), and HTTP 429/500/502/503/504 (with the existing 0.5 s backoff).
• Drop: everything else (e.g. HTTP 400 bad payload), so a poison request can't wedge the queue — with e.response is not None so the check actually works.

Retrying is safe for heartbeats (the only thing in this queue): the 503 lock-rejection happens before any server-side processing, and even a true duplicate (processed but response lost) merges into the last event as a no-op. FIFO head-retry also preserves ordering, which heartbeat merging requires — dropping requests from the middle of the stream broke merge chains and fragmented events.

Tests

• test_dispatch_retries_transient_server_errors (parametrized over 429/500/502/503/504): request survives the error, is retried, and is popped once the server recovers. Also guards the Response.__bool__ pitfall.
• test_dispatch_drops_client_errors: HTTP 400 is dropped and doesn't block the queue.
• test_dispatch_keeps_queue_on_connection_error: request stays queued and the queue marks itself disconnected.

[greptile-apps]

Greptile Summary

Retry queued heartbeat requests after transient transport and server failures instead of dropping them.

• Honors bounded delta-seconds Retry-After values using a stop-aware wait.
• Drops non-retryable client errors so malformed requests do not block the FIFO queue.
• Adds tests for transient HTTP errors, client errors, connection failures, and retry-delay bounds.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain.

Important Files Changed

Filename	Overview
aw_client/client.py	Classifies transport and transient HTTP failures as retryable, applies bounded Retry-After delays, and retains queued requests until successful delivery.
tests/test_requestqueue.py	Adds focused coverage for transient retries, client-error dropping, connection-error retention, and Retry-After parsing and bounds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Dispatch queued heartbeat] --> B{Request result}
    B -->|Success| C[Remove queue head]
    B -->|Connection error or timeout| D[Mark disconnected]
    D --> E[Keep queue head]
    B -->|HTTP 429, 500, 502, 503, or 504| F[Calculate bounded retry delay]
    F --> G[Stop-aware wait]
    G --> E
    B -->|Other request error| H[Drop queue head]

Loading

_{Reviews (2): Last reviewed commit: "fix(queue): honor Retry-After on transie..." | Re-trigger Greptile}

[greptile-apps]

429 retry delay ignores Retry-After

HTTP 429 responses retain the FIFO head but retry it after a fixed 0.5-second delay without honoring Retry-After, generating avoidable requests during the rate-limit window and blocking later heartbeats for longer.

[ErikBjare]

Fixed in b31ba5a — retry delay now honors Retry-After (delta-seconds form, floored at 0.5s, capped at 60s; HTTP-date form falls back to default), and uses the stop-aware wait() instead of sleep() so a long delay can't block shutdown.

[ErikBjare]

@greptileai review

[TimeToBuildBob]

@ErikBjare — this branch is currently CONFLICTING against master (4 commits behind), and that now blocks two dependent PRs: #127 and #128, both stacked on this branch and otherwise converged + mergeable.

The conflict is small and localized to aw_client/client.py: #118 added the persistqueue_path argument to RequestQueue.__init__ at the same site where this branch adds RETRY_STATUS_CODES. I rebased it locally (kept both changes) and verified tests/test_requestqueue.py 12 passed and ruff check clean — but the branch lives in ActivityWatch/aw-client, and my SSH identity (TimeToBuildBob) has no push access there, so I can't push the rebase.

Your call which is easier:

1. You rebase + force-push this branch, or
2. I push the rebased branch to my fork and open a replacement PR (and we close this one).

Either works for me; option 2 is ready to go.

[TimeToBuildBob]

Carry PR opened as #130 (#130) — rebased on current master, resolving the persistqueue_path vs RETRY_STATUS_CODES conflict by keeping both. The dependent PRs #127 and #128 have been restacked accordingly with base changed to master. Erik: feel free to close #130 and rebase this PR directly instead — the carry is just a path forward if this branch stays conflicting.