fix: abort hung dashboard and OAuth token fetches - #111
Conversation
Dashboard JSON/playground fetches and the OAuth token exchange had no AbortSignal, so a stalled Worker or tokenUrl left the console and /v1/oauth/callback pending. Apply a 30s AbortSignal.timeout, preserve any caller signal via AbortSignal.any, and return the existing OAuth failed page when the token POST aborts. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs real behavior proof before merge. Reviewed August 16, 2026, 7:39 AM ET / 11:39 UTC. ClawSweeper reviewWhat this changesThe PR moves dashboard request helpers into a focused module, adds timeout signals for dashboard and OAuth token fetches, keeps playground requests at a 600-second budget, and adds focused tests. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until stronger real behavior proof is added - 3 items remain Keep open: current main still leaves these dashboard and OAuth fetches unbounded, and the current patch is targeted with no code-level blocker found. The final head changed the dashboard timeout to 60 seconds, but the supplied live proof still demonstrates the earlier 30-second behavior, so exact-head proof is needed before merge. Priority: P1 Review scores
Verification
How this fits togetherThe admin dashboard fetches Worker JSON endpoints, while the OAuth callback exchanges an authorization code with a provider. Both paths affect interactive console state and provider connection setup, so stalled upstreams need bounded user-visible failures. flowchart LR
A[Admin dashboard] --> B[Dashboard request helper]
C[OAuth callback] --> D[Provider token exchange]
B --> E[Timeout and caller abort signal]
D --> E
E --> F[Worker or provider endpoint]
F --> G[Bounded response or failure page]
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the 60-second dashboard headroom and 600-second playground budget, while keeping OAuth token exchange failure bounded at 30 seconds and evidenced on the final head. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main has both owner fetch calls without signals, and the PR supplies a concrete hanging TCP-server scenario; this review did not execute that scenario. Is this the best way to solve the issue? Yes, the timeout helper is a narrow shared solution that preserves caller cancellation and the playground budget, but current-head runtime proof must replace the earlier 30-second dashboard transcript. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ca27bc44227f. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Dashboard JSON stays at 30s. Playground model and service calls can take up to the Worker manifest maximum of 600 seconds. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Pushed @clawsweeper re-review |
What Problem This Solves
The admin console
request/playgroundRequesthelpers and the/v1/oauth/callbacktoken exchange callfetchwith noAbortSignal. If the Worker or the providertokenUrlstalls, the dashboard stays pending and the OAuth callback never returns a connected/failed page. The proxy path already combines the inbound request signal with a timeout; these two call sites did not.Evidence
Live Node against a TCP server that accepts the connection and never writes an HTTP response. The shared helper aborts in ~80ms when given an 80ms budget. The production dashboard
request()helper uses the default 30s budget and returnsTimeoutErrorinstead of hanging.Playground fetches use the Worker manifest ceiling (
PLAYGROUND_FETCH_TIMEOUT_MS = 600_000). Dashboard JSON stays at 30s. A caller can still pass a shorter budget (coverage uses180_000).A token-exchange throw (timeout or network) now returns the existing "Provider token exchange failed." callback page instead of an unhandled Worker exception.
Real behavior proof
Behavior or issue addressed: Dashboard JSON fetches and the OAuth token POST carry a 30s
AbortSignal.timeout. Playground model and service calls use a 600s budget so they can finish inside the Worker endpoint timeout. A hung peer finishes withTimeoutError(dashboard) or the OAuth failed page (callback).Real environment tested: macOS Darwin 25.6.0 arm64, Node v26.7.0, clawrouter checkout
/tmp/oc-impl-clawrouter-fetchonfix/fetch-abort-timeout. Livenodeagainst a local hanging HTTP server (accept, no response body).Exact steps or command run after this patch: Started a
node:httpserver that never writes a response. CalledfetchTimeoutSignal(undefined, 80)onPOST /oauth/token, then called productionrequest(hung, "/v1/session")so the default 30s budget is the one compiled intoadmin/src/dashboard-fetch.ts. Ranpnpm --dir admin testfor the 600s playground default and the explicit 180s override.Evidence after fix: terminal output copied below.
Observed result after fix: The 80ms helper path aborted with
TimeoutErrorat 83ms. Production dashboardrequest()recordedAbortSignal.timeout(30000)and aborted at 30004ms instead of remaining pending.playgroundRequestrecordsAbortSignal.timeout(600000)unless the caller passes a shortertimeoutMs. OAuth callback maps the same abort to the existing failed HTML page.What was not tested: Live Cloudflare Access admin session through a real provider
tokenUrl. Grant refresh inworker/providers.ts(refreshGrant) still has no token-fetch timeout. Browser screenshot of a 10-minute playground completion.Summary
fetchTimeoutSignalinshared/fetch-timeout.ts(30s dashboard default, 600s playground default, preserves a caller signal)/v1/oauth/callbacktoken POST uses the same helper and catches abort/network failuresfetchlanded in openclaw/clawrouter#55 (2026-06-22)AbortSignal.anyplus a timer inworker/proxy.ts