fix(#6821): add Go-side context deadline and JS-side recovery for WASM mint timeout - #6823
fix(#6821): add Go-side context deadline and JS-side recovery for WASM mint timeout#6823fullsend-ai-coder[bot] wants to merge 2 commits into
Conversation
…M mint timeout The mint WASM isolate permanently poisons after a GitHub API timeout, causing cascading 503s for all subsequent callers until isolate recycle. Root cause: the WASM-side mintHTTP did not honor request context deadlines — awaitPromise blocked until the JS Promise settled regardless of Go context cancellation. When a GitHub API call exceeded the JS-side 25s timeout (HANDLE_FETCH_TIMEOUT_MS), the GoWasm singleton was permanently poisoned with no programmatic recovery. Go-side fix (primary): - Add awaitPromiseWithContext in fetch_js.go that respects context cancellation, returning ctx.Err() immediately while a background goroutine cleans up the JS callback resources after the Promise settles - Update mintHTTP (http_client_js.go) and HostPEMAccessor.AccessPEM (pem_js.go) to use the context-aware variant - Add a 20s per-request context deadline in cmd/mint-wasm/main.go (requestTimeout), 5s below the JS-side 25s timeout, so slow GitHub API calls surface as clean Go-side errors before the JS timeout fires JS-side fix (defense-in-depth): - Replace permanent poisoning with recovery: after a timeout, the GoWasm singleton is marked for recovery (markTimedOut) instead of permanently poisoned (markPoisoned). The next request re-initializes the Go WASM runtime via doInit rather than returning 503 - The old Go runtime leaks (bounded by CF isolate lifetime) but cannot interfere: its globalThis exports are overwritten by the new runtime, and late Promise resolutions are silently ignored Closes #6821
|
🤖 Finished Review · ✅ Success · Started 8:20 PM UTC · Completed 8:40 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.08 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsTargeted bug fix for a high-severity issue (permanent WASM isolate poisoning) with clear scope across 9 files (385 lines). Low Tier 1 signals (no protected paths, no dependency changes, bot author), elevated Tier 2 (high churn and regression history in changed mint files), moderate Tier 3 (well-scoped issue with clear acceptance criteria). Score unchanged from prior assessment. Previous runRisk Assessment: moderate (2/5) DetailsTargeted bug fix for a high-severity issue (permanent WASM isolate poisoning) with clear scope across 9 files (331 lines). Low Tier 1 signals (no protected paths, no dependency changes, bot author), elevated Tier 2 (high churn and regression history in changed mint files), moderate Tier 3 (well-scoped issue with clear acceptance criteria). |
|
Looks good to me Previous runReviewFindingsHigh
Medium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 8:42 PM UTC · Completed 8:50 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.18 |
- Fix tautological ErrorIs assertion in TestFindInstallation_ContextDeadline (was comparing context.DeadlineExceeded against itself instead of err) - Add ErrorIs assertion to TestFindOrgInstallation_ContextDeadline to verify the error wraps context.DeadlineExceeded - Add consecutive timeout recovery counter to GoWasm class (cap at 3) to bound leaked WASM runtimes under sustained upstream slowness - Update stale comments: "permanently poisons" → recovery-aware wording, clarify const singleton comment re: internal runtime replacement Addresses #6823
🔧 Fix agent — iteration 1 (bot-triggered)Addressed all 5 review findings: fixed tautological ErrorIs assertion, added missing ErrorIs check in org installation test, added recovery counter to bound leaked WASM runtimes (cap at 3 consecutive timeouts), and updated two stale comments to match recovery semantics. Fixed (5):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:52 PM UTC · Completed 9:07 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.17 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
1 participant
Summary
Prevent permanent mint WASM isolate poisoning after GitHub API timeouts by adding Go-side context deadlines and replacing the JS-side permanent-poison mechanism with automatic recovery.
Related Issue
Closes #6821
Changes
Go-side (primary fix):
awaitPromiseWithContextininternal/mintcore/fetch_js.go— a context-aware variant ofawaitPromisethat returnsctx.Err()on cancellation while a background goroutine cleans up JS callback resourcesmintHTTPinhttp_client_js.goto use the context-aware variant, honoring request context deadlines on WASMHostPEMAccessor.AccessPEMinpem_js.goto honor the context parameter (previously ignored)cmd/mint-wasm/main.go(requestTimeout), 5s below the JS-side 25sHANDLE_FETCH_TIMEOUT_MS, so slow GitHub API calls return clean Go errors before the JS timeout firesJS-side (defense-in-depth):
markPoisoned) with recovery (markTimedOut) inGoWasmclassinitPromiseso the next request boots a fresh Go WASM runtime instead of returning 503globalThisexports are overwritten by the new runtimeTests:
TestFindInstallation_ContextDeadlineandTestFindOrgInstallation_ContextDeadlineverifying context deadline behavior on native platformEmbed sync:
.embedcopies forfetch_js.go,http_client_js.go,pem_js.goTesting
go test -race -count=1 ./...passes ininternal/mintcore/go vet ./...passesTestEmbeddedMintSourcepasses (embed sync verified)lint-mint-embed-syncpassesCloses #6821
Post-script verification
agent/6821-mint-wasm-timeout-recovery)1ac1750a661c6ccc170267c8b3919d2597cf7810..HEAD)