fix(web): abort hung API and slash-command fetches - #168
Conversation
Browser api() and slash-command dispatch used fetch with no AbortSignal, so a stalled API host left the UI pending forever. Apply AbortSignal.timeout(30s) unless the caller already provided a signal. Keep R2 Client.Timeout at 0 (streaming-safe contract). 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 15, 2026, 10:53 PM ET / August 16, 2026, 02:53 UTC. ClawSweeper reviewWhat this changesThe PR applies a 30-second default abort signal to browser JSON API and slash-command requests while retaining caller signals and excluding multipart uploads. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 4 items remain Keep open: the timeout approach is narrowly scoped, but merge still needs real browser-path proof and removal of the release-owned changelog entry. Likely related people: steipete (API client) and shakkernerd (slash dispatch). Priority: P2 Review scores
Verification
How this fits togetherClickClack’s browser app uses shared request helpers to load workspace data and dispatch registered slash commands to its API. The web bundle is embedded in the API service, so the generated assets must reflect the source helper. flowchart LR
A[Workspace or composer] --> B[Web request helper]
B --> C{Signal supplied?}
C -->|Yes| D[Use caller signal]
C -->|No| E[Add 30 second timeout]
D --> F[ClickClack API]
E --> F
F --> G[Response or abort]
G --> H[UI recovery]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the shared default timeout, remove the release-owned changelog edit, and add redacted browser network or live-output proof showing a controlled stalled API request aborting and the UI recovering. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main’s shared JSON and slash-command fetches have no abort signal, and a controlled endpoint that accepts then stalls provides a high-confidence browser reproduction path. Is this the best way to solve the issue? Yes, conditionally: a shared default with caller-signal and multipart exceptions is the narrowest repair, but it needs real browser proof and should not modify release-owned changelog content. Full review comments:
Overall correctness: patch is correct AGENTS.md: found, but no applicable review policy affected this item. Codex review notes: model internal, reasoning high; reviewed against 18acea79465c. 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
HistoryReview history (1 earlier review cycle)
|
What Problem This Solves
Fixes an issue where users opening ClickClack (loading workspaces, sending a slash command, refreshing profile data) would see the UI spin forever when the API host accepted the TCP connection and then stalled. The browser
fetchfor/api/*andPOST /api/hooks/slash/:channelhad no AbortSignal, so the request never finished and the page never recovered.This is the web JSON/slash client, not artifact preview (that path already passes a signal) and not the R2 upload client (its
Timeout == 0contract stays in place for streaming).Why This Change Was Made
Both browser call sites now go through
applyDefaultFetchTimeout, which attachesAbortSignal.timeout(30_000)when the caller did not already passinit.signal. Callers that need a longer or shorter bound can still supply their own signal. The embedded web bundle is rebuilt so the Go binary serves the same helper.APIErroris written with an explicitstatusfield so Node's strip-only TypeScript loader can importapi()from the existingnode --testsuite.User Impact
If the API host hangs, workspace loads and slash commands fail after 30 seconds instead of leaving the composer or settings pane pending with no way out. Successful requests are unchanged. Operators who already pass a signal keep that signal.
Evidence
Live
nodeagainst the patchedapps/web/src/lib/api.tsand the embedded production chunk:Compiled helper in
apps/api/internal/webassets/dist/_app/immutable/chunks/DoZJIpeX.js:Without a default signal, a stand-in fetch that never returns stays pending (
before.hasSignal=false). After the patch the same stand-in is aborted (AbortError) and a caller-supplied signal is left in place.Real behavior proof
Behavior or issue addressed: Browser API and slash-command fetches never aborted, so a stalled API host froze workspace loads and slash dispatch.
Real environment tested: macOS, Node v26.7.0, branch
fix/web-fetch-timeoutat/tmp/oc-impl-clickclack-fetch, patched web sources plus the rebuilt embedded chunkDoZJIpeX.js.Exact steps or command run after this patch:
Evidence after fix: terminal output from the patched tree:
Observed result after fix: The default helper now attaches a 30s
AbortSignal. A fetch that never returns is rejected withAbortErrorinstead of hanging. A caller-provided signal is reused. The embedded production chunk containsAbortSignal.timeoutand the3e4default.What was not tested: A browser session against a real stalled production API host. Artifact preview fetch and the R2
Client.Timeout == 0streaming path were left unchanged.Related
04402c82from #94 (split frontend/APIfetch); slash dispatch from #82.