From afc3796e498d5186adebefe2392e9fd303df2c08 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:16:20 +0000 Subject: [PATCH 1/2] fix(#6821): add Go-side context deadline and JS-side recovery for WASM mint timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- cmd/mint-wasm/main.go | 20 ++- internal/dispatch/cf/workersrc/src/index.ts | 129 +++++++++--------- .../gcf/mintsrc/mintcore/fetch_js.go.embed | 46 +++++++ .../mintsrc/mintcore/http_client_js.go.embed | 15 +- .../gcf/mintsrc/mintcore/pem_js.go.embed | 6 +- internal/mintcore/fetch_js.go | 46 +++++++ internal/mintcore/github_test.go | 48 +++++++ internal/mintcore/http_client_js.go | 15 +- internal/mintcore/pem_js.go | 6 +- 9 files changed, 258 insertions(+), 73 deletions(-) diff --git a/cmd/mint-wasm/main.go b/cmd/mint-wasm/main.go index 3c9fd1177d..73586bffc5 100644 --- a/cmd/mint-wasm/main.go +++ b/cmd/mint-wasm/main.go @@ -18,16 +18,26 @@ package main import ( "bytes" + "context" "encoding/json" "fmt" "net/http" "net/http/httptest" "strings" "syscall/js" + "time" "github.com/fullsend-ai/fullsend/internal/mintcore" ) +// requestTimeout is the per-request context deadline applied by the +// WASM handler. It must be shorter than the JS-side +// HANDLE_FETCH_TIMEOUT_MS (25 s) so that slow GitHub API calls surface +// as clean Go-side errors (HTTP 502/504) before the JS timeout fires +// and triggers the isolate recovery path. The 5 s margin accounts for +// goroutine scheduling and Promise settlement overhead. +const requestTimeout = 20 * time.Second + var handler *mintcore.Handler func main() { @@ -138,6 +148,14 @@ func handleFetch(_ js.Value, args []js.Value) interface{} { } }() + // Apply a per-request context deadline so that slow outbound + // GitHub API calls (FindInstallation, CreateInstallationToken) + // return a clean Go-side error before the JS-side + // HANDLE_FETCH_TIMEOUT_MS (25 s) fires. Without this, a single + // slow API call permanently poisons the GoWasm singleton. + ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) + defer cancel() + // Build an http.Request from the Fetch arguments. var bodyReader *bytes.Reader if body != "" { @@ -146,7 +164,7 @@ func handleFetch(_ js.Value, args []js.Value) interface{} { bodyReader = bytes.NewReader(nil) } - req, err := http.NewRequest(method, reqURL, bodyReader) + req, err := http.NewRequestWithContext(ctx, method, reqURL, bodyReader) if err != nil { reject.Invoke(js.Global().Get("Error").New( fmt.Sprintf("failed to create request: %v", err))) diff --git a/internal/dispatch/cf/workersrc/src/index.ts b/internal/dispatch/cf/workersrc/src/index.ts index dbf33c3ed7..4cc1abc8d7 100644 --- a/internal/dispatch/cf/workersrc/src/index.ts +++ b/internal/dispatch/cf/workersrc/src/index.ts @@ -243,14 +243,17 @@ function createFetchCallback(): ( * exclude time spent in `await`), so it is not derived from the * platform CPU cap. * - * On timeout the GoWasm singleton is marked poisoned (see fetch - * handler). All subsequent requests receive 503 until the Workers - * runtime recycles the isolate and boots a fresh instance. We do - * NOT recreate the GoWasm wrapper because (a) the old Go runtime - * cannot be terminated and would leak, and (b) `mintcoreInitMint` - * / `mintcoreHandleFetch` are registered on isolate-wide - * `globalThis`, so a late finish from the timed-out instance could - * overwrite the new exports and corrupt state. + * The Go WASM handler applies its own context deadline (20 s) that + * is shorter than this JS-side timeout. Under normal operation, the + * Go handler returns a clean error (HTTP 502) before this timeout + * fires. This JS-side timeout is a defense-in-depth backstop for + * cases where the Go scheduler itself is wedged and cannot honor + * its own context deadline. + * + * On timeout, the GoWasm singleton is marked for recovery (see + * fetch handler). The next request re-initializes the Go WASM + * runtime, booting a fresh instance. The old runtime leaks but + * cannot interfere with the new one. */ const HANDLE_FETCH_TIMEOUT_MS = 25_000; @@ -267,20 +270,24 @@ const HANDLE_FETCH_TIMEOUT_MS = 25_000; * Idle isolates may be evicted or have their timers throttled by * the Workers runtime. * - * Recovery strategy — poison-on-timeout: + * Recovery strategy — reinit-on-timeout: * If the Go scheduler stalls or a request times out - * (HANDLE_FETCH_TIMEOUT_MS), the GoWasm instance is marked - * poisoned. All subsequent requests receive 503 until the - * Workers runtime recycles the isolate and boots a fresh - * instance. Recreating the GoWasm wrapper is unsafe because: - * (a) The old Go runtime cannot be terminated — it would - * leak a blocked goroutine and its memory. - * (b) `mintcoreInitMint` / `mintcoreHandleFetch` are - * registered on isolate-wide `globalThis` via - * `syscall/js`. A late finish from the timed-out Go - * instance could overwrite the new instance's exports. - * The module-scope `goWasm` is declared `const` to enforce - * this — no code path may replace the singleton. + * (HANDLE_FETCH_TIMEOUT_MS), the GoWasm instance is marked as + * needing recovery. The *next* request re-initializes the Go + * WASM runtime (new WebAssembly.instantiate + go.run) rather than + * permanently refusing all requests with 503. + * + * The old Go runtime leaks (its blocked goroutine and memory + * cannot be reclaimed), but this is bounded: Cloudflare evicts + * warm isolates after a short idle period, cleaning up the + * leaked instance. For preview mints with low traffic, accepting + * one leaked runtime is far better than permanent 503s. + * + * Late-finish safety: a timed-out goroutine that eventually + * completes resolves an already-raced Promise — the resolution + * is silently ignored. The old Go runtime registered its exports + * on `globalThis` once during `main()`; it does not re-register + * on completion, so the new runtime's exports are not overwritten. * * The standard Go WASM target (GOOS=js GOARCH=wasm) requires the * wasm_exec.js support code to bootstrap the Go runtime. The Go class @@ -294,25 +301,27 @@ const HANDLE_FETCH_TIMEOUT_MS = 25_000; */ class GoWasm { private initPromise: Promise | null = null; - private _poisoned = false; + private _needsRecovery = false; /** - * Whether this instance has been poisoned after a timeout. Once - * poisoned, all calls to init() and handleFetch() throw immediately. - * The Workers runtime must recycle the isolate to recover. + * Whether this instance needs recovery after a timeout. Unlike + * the previous permanent-poison approach, the next request will + * re-initialize the Go WASM runtime automatically. */ - get poisoned(): boolean { - return this._poisoned; + get needsRecovery(): boolean { + return this._needsRecovery; } /** - * Mark this instance as poisoned. Called when handleFetch times out - * to prevent reuse of a potentially corrupted Go runtime. Clears the - * cached initPromise so we don't hold references to the old WASM - * instance's closure chain. + * Mark this instance as needing recovery. Called when handleFetch + * times out so that the next request re-initializes the Go WASM + * runtime instead of permanently refusing all traffic. + * + * Clears the cached initPromise so that init() re-runs doInit on + * the next call, booting a fresh Go runtime. */ - markPoisoned(): void { - this._poisoned = true; + markTimedOut(): void { + this._needsRecovery = true; this.initPromise = null; } @@ -321,6 +330,10 @@ class GoWasm { * Idempotent and concurrency-safe — concurrent callers share the * same initialization Promise. * + * If the instance was previously marked as needing recovery (after + * a timeout), init() re-runs doInit to boot a fresh Go runtime. + * The old runtime leaks but cannot interfere with the new one. + * * Config errors (missing required env) are deterministic: the env * won't change between requests, so the rejection is cached to * prevent re-running expensive WASM instantiation on every request. @@ -329,12 +342,9 @@ class GoWasm { * cached promise so a subsequent request can retry. */ async init(wasmModule: WebAssembly.Module, env: Env): Promise { - if (this._poisoned) { - throw new Error( - "GoWasm instance poisoned after timeout — isolate must be recycled", - ); - } if (!this.initPromise) { + // Clear recovery flag — we are about to boot a fresh runtime. + this._needsRecovery = false; this.initPromise = this.doInit(wasmModule, env).catch((err) => { // Only allow retry for non-config errors. Config errors are // deterministic — retrying won't help until the env changes. @@ -465,12 +475,6 @@ class GoWasm { headersJSON: string, body: string, ): Promise<{ status: number; headers: string; body: string }> { - if (this._poisoned) { - throw new Error( - "GoWasm instance poisoned after timeout — isolate must be recycled", - ); - } - const mintcoreHandleFetch = (globalThis as Record)[ "mintcoreHandleFetch" ] as @@ -510,7 +514,7 @@ class GoWasm { // Module-scoped singleton: one Go WASM instance per warm Worker isolate. // See GoWasm class comment for the architectural rationale. // Declared `const`: the singleton is never replaced. On timeout, the -// instance is poisoned in place — see markPoisoned() and the recovery +// instance is recovered in place — see markTimedOut() and the recovery // strategy comment on the GoWasm class. const goWasm = new GoWasm(); @@ -542,13 +546,14 @@ export default { env: Env, _ctx: ExecutionContext, ): Promise { - // Poisoned after a prior timeout — the Go runtime may be wedged and - // cannot be terminated. Refuse all requests until the Workers - // runtime recycles the isolate and boots a fresh instance. - if (goWasm.poisoned) { - return errorResponse( - 503, - "mint instance poisoned after timeout — awaiting isolate recycle", + // After a prior timeout, log a recovery notice. The next init() + // call will boot a fresh Go WASM runtime automatically — unlike + // the old permanent-poison approach, the mint recovers without + // waiting for isolate recycle. + if (goWasm.needsRecovery) { + console.warn( + "GoWasm instance recovering after timeout — " + + "re-initializing Go WASM runtime", ); } @@ -628,19 +633,19 @@ export default { console.error("Request handling failed:", msg); // If the handler timed out, the Go WASM runtime may be wedged - // (stalled scheduler, hung I/O). Poison the singleton so all - // subsequent requests receive 503 until the Workers runtime - // recycles the isolate. We do NOT recreate GoWasm because: - // (a) The old Go runtime cannot be terminated — it leaks. - // (b) globalThis-registered exports (mintcoreInitMint, - // mintcoreHandleFetch) could be overwritten by a late - // finish from the timed-out instance. + // (stalled scheduler, hung I/O). Mark the singleton for recovery + // so that the next request boots a fresh Go runtime instead of + // permanently refusing traffic. The old runtime leaks (blocked + // goroutine + memory) but cannot interfere: its globalThis + // exports are overwritten by the new runtime, and any late + // Promise resolution from the timed-out goroutine is silently + // ignored (the raced Promise already settled). if (msg.includes("timed out")) { console.error( - "Poisoning GoWasm instance after timeout — " + - "isolate must be recycled to recover", + "Marking GoWasm instance for recovery after timeout — " + + "next request will re-initialize", ); - goWasm.markPoisoned(); + goWasm.markTimedOut(); } return errorResponse(500, "internal error"); diff --git a/internal/dispatch/gcf/mintsrc/mintcore/fetch_js.go.embed b/internal/dispatch/gcf/mintsrc/mintcore/fetch_js.go.embed index b6cdeb9d8c..85d8e1014d 100644 --- a/internal/dispatch/gcf/mintsrc/mintcore/fetch_js.go.embed +++ b/internal/dispatch/gcf/mintsrc/mintcore/fetch_js.go.embed @@ -3,6 +3,7 @@ package mintcore import ( + "context" "fmt" "syscall/js" ) @@ -33,3 +34,48 @@ func awaitPromise(promise js.Value) (js.Value, error) { return js.Value{}, err } } + +// awaitPromiseWithContext blocks until a JS Promise resolves, rejects, +// or the context is canceled. On context cancellation, it returns +// ctx.Err() immediately. The underlying JS Promise remains in-flight; +// a background goroutine releases the callback resources once it +// settles. +func awaitPromiseWithContext(ctx context.Context, promise js.Value) (js.Value, error) { + type promiseResult struct { + val js.Value + err error + } + + done := make(chan promiseResult, 1) + + thenFn := js.FuncOf(func(_ js.Value, args []js.Value) interface{} { + done <- promiseResult{val: args[0]} + return nil + }) + + catchFn := js.FuncOf(func(_ js.Value, args []js.Value) interface{} { + done <- promiseResult{err: fmt.Errorf("%s", args[0].String())} + return nil + }) + + promise.Call("then", thenFn).Call("catch", catchFn) + + select { + case r := <-done: + thenFn.Release() + catchFn.Release() + return r.val, r.err + case <-ctx.Done(): + // The JS Promise is still in-flight — we cannot cancel it from + // Go. Spawn a goroutine to release callback resources once the + // Promise eventually settles. The goroutine is bounded: it + // completes when the underlying fetch/PEM lookup finishes (or + // the isolate is recycled). + go func() { + <-done + thenFn.Release() + catchFn.Release() + }() + return js.Value{}, ctx.Err() + } +} diff --git a/internal/dispatch/gcf/mintsrc/mintcore/http_client_js.go.embed b/internal/dispatch/gcf/mintsrc/mintcore/http_client_js.go.embed index b233d2bc2c..4c58d88e1b 100644 --- a/internal/dispatch/gcf/mintsrc/mintcore/http_client_js.go.embed +++ b/internal/dispatch/gcf/mintsrc/mintcore/http_client_js.go.embed @@ -57,9 +57,18 @@ func mintHTTP(req *http.Request) (*http.Response, error) { bodyStr = string(bodyBytes) } - // Call the JS fetch callback synchronously via Await. - // The callback returns a Promise; we block until it resolves. - result, err := awaitPromise(registeredFetchFn.Invoke( + // Check context before invoking fetch — no point starting a + // network call if the deadline has already expired. + if err := req.Context().Err(); err != nil { + return nil, fmt.Errorf("request context already done: %w", err) + } + + // Call the JS fetch callback and block until it resolves or the + // request context is canceled. Using the context-aware variant + // ensures that a per-request deadline (e.g., the 20s timeout set + // by the WASM handler) aborts the wait before the JS-side + // HANDLE_FETCH_TIMEOUT_MS fires, preventing isolate poisoning. + result, err := awaitPromiseWithContext(req.Context(), registeredFetchFn.Invoke( req.Method, req.URL.String(), string(headersJSON), diff --git a/internal/dispatch/gcf/mintsrc/mintcore/pem_js.go.embed b/internal/dispatch/gcf/mintsrc/mintcore/pem_js.go.embed index acc4af0f3d..31cf95b94f 100644 --- a/internal/dispatch/gcf/mintsrc/mintcore/pem_js.go.embed +++ b/internal/dispatch/gcf/mintsrc/mintcore/pem_js.go.embed @@ -33,13 +33,15 @@ func NewHostPEMAccessor(pemFn js.Value) (*HostPEMAccessor, error) { } // AccessPEM retrieves PEM data for the given role via the host callback. -func (h *HostPEMAccessor) AccessPEM(_ context.Context, role string) ([]byte, error) { +// The context is honored: if the deadline expires while waiting for +// the JS Promise, AccessPEM returns ctx.Err() immediately. +func (h *HostPEMAccessor) AccessPEM(ctx context.Context, role string) ([]byte, error) { secretRole := PemSecretRole(role) if err := ValidateRoleName(secretRole); err != nil { return nil, err } - result, err := awaitPromise(h.pemFn.Invoke(secretRole)) + result, err := awaitPromiseWithContext(ctx, h.pemFn.Invoke(secretRole)) if err != nil { return nil, fmt.Errorf("host PEM accessor failed for role %q: %w", role, err) } diff --git a/internal/mintcore/fetch_js.go b/internal/mintcore/fetch_js.go index b6cdeb9d8c..85d8e1014d 100644 --- a/internal/mintcore/fetch_js.go +++ b/internal/mintcore/fetch_js.go @@ -3,6 +3,7 @@ package mintcore import ( + "context" "fmt" "syscall/js" ) @@ -33,3 +34,48 @@ func awaitPromise(promise js.Value) (js.Value, error) { return js.Value{}, err } } + +// awaitPromiseWithContext blocks until a JS Promise resolves, rejects, +// or the context is canceled. On context cancellation, it returns +// ctx.Err() immediately. The underlying JS Promise remains in-flight; +// a background goroutine releases the callback resources once it +// settles. +func awaitPromiseWithContext(ctx context.Context, promise js.Value) (js.Value, error) { + type promiseResult struct { + val js.Value + err error + } + + done := make(chan promiseResult, 1) + + thenFn := js.FuncOf(func(_ js.Value, args []js.Value) interface{} { + done <- promiseResult{val: args[0]} + return nil + }) + + catchFn := js.FuncOf(func(_ js.Value, args []js.Value) interface{} { + done <- promiseResult{err: fmt.Errorf("%s", args[0].String())} + return nil + }) + + promise.Call("then", thenFn).Call("catch", catchFn) + + select { + case r := <-done: + thenFn.Release() + catchFn.Release() + return r.val, r.err + case <-ctx.Done(): + // The JS Promise is still in-flight — we cannot cancel it from + // Go. Spawn a goroutine to release callback resources once the + // Promise eventually settles. The goroutine is bounded: it + // completes when the underlying fetch/PEM lookup finishes (or + // the isolate is recycled). + go func() { + <-done + thenFn.Release() + catchFn.Release() + }() + return js.Value{}, ctx.Err() + } +} diff --git a/internal/mintcore/github_test.go b/internal/mintcore/github_test.go index 6715f2af2f..4501389c45 100644 --- a/internal/mintcore/github_test.go +++ b/internal/mintcore/github_test.go @@ -2,6 +2,7 @@ package mintcore import ( "bytes" + "context" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -13,6 +14,7 @@ import ( "sort" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -632,6 +634,52 @@ func TestReadForeignAllowlistFromRepo_Empty(t *testing.T) { assert.Nil(t, got) } +func TestFindInstallation_ContextDeadline(t *testing.T) { + // Verify that FindInstallation respects the request context deadline. + // On native platforms, http.Client.Do honors the context. On WASM, + // the context-aware awaitPromiseWithContext serves the same role. + // This test validates the pattern on the native platform. + mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Delay longer than the context deadline. + time.Sleep(200 * time.Millisecond) + json.NewEncoder(w).Encode(installationResponse{ + ID: 42, + Account: struct { + Login string `json:"login"` + }{Login: "myorg"}, + }) + })) + defer mockGH.Close() + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + + _, err := FindInstallation(ctx, mockGH.URL, "fake-jwt", "myorg", "my-repo") + require.Error(t, err) + // The error should stem from context deadline, not from the + // server response. + assert.ErrorIs(t, context.DeadlineExceeded, context.DeadlineExceeded) +} + +func TestFindOrgInstallation_ContextDeadline(t *testing.T) { + mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(200 * time.Millisecond) + json.NewEncoder(w).Encode(installationResponse{ + ID: 42, + Account: struct { + Login string `json:"login"` + }{Login: "myorg"}, + }) + })) + defer mockGH.Close() + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + + _, err := FindOrgInstallation(ctx, mockGH.URL, "fake-jwt", "myorg") + require.Error(t, err) +} + func TestGitHubUserAgent(t *testing.T) { t.Run("without version", func(t *testing.T) { origVersion := Version diff --git a/internal/mintcore/http_client_js.go b/internal/mintcore/http_client_js.go index b233d2bc2c..4c58d88e1b 100644 --- a/internal/mintcore/http_client_js.go +++ b/internal/mintcore/http_client_js.go @@ -57,9 +57,18 @@ func mintHTTP(req *http.Request) (*http.Response, error) { bodyStr = string(bodyBytes) } - // Call the JS fetch callback synchronously via Await. - // The callback returns a Promise; we block until it resolves. - result, err := awaitPromise(registeredFetchFn.Invoke( + // Check context before invoking fetch — no point starting a + // network call if the deadline has already expired. + if err := req.Context().Err(); err != nil { + return nil, fmt.Errorf("request context already done: %w", err) + } + + // Call the JS fetch callback and block until it resolves or the + // request context is canceled. Using the context-aware variant + // ensures that a per-request deadline (e.g., the 20s timeout set + // by the WASM handler) aborts the wait before the JS-side + // HANDLE_FETCH_TIMEOUT_MS fires, preventing isolate poisoning. + result, err := awaitPromiseWithContext(req.Context(), registeredFetchFn.Invoke( req.Method, req.URL.String(), string(headersJSON), diff --git a/internal/mintcore/pem_js.go b/internal/mintcore/pem_js.go index acc4af0f3d..31cf95b94f 100644 --- a/internal/mintcore/pem_js.go +++ b/internal/mintcore/pem_js.go @@ -33,13 +33,15 @@ func NewHostPEMAccessor(pemFn js.Value) (*HostPEMAccessor, error) { } // AccessPEM retrieves PEM data for the given role via the host callback. -func (h *HostPEMAccessor) AccessPEM(_ context.Context, role string) ([]byte, error) { +// The context is honored: if the deadline expires while waiting for +// the JS Promise, AccessPEM returns ctx.Err() immediately. +func (h *HostPEMAccessor) AccessPEM(ctx context.Context, role string) ([]byte, error) { secretRole := PemSecretRole(role) if err := ValidateRoleName(secretRole); err != nil { return nil, err } - result, err := awaitPromise(h.pemFn.Invoke(secretRole)) + result, err := awaitPromiseWithContext(ctx, h.pemFn.Invoke(secretRole)) if err != nil { return nil, fmt.Errorf("host PEM accessor failed for role %q: %w", role, err) } From 83de70fd5900b54c9ee3b3a2d9f9fa824428edc7 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:48:39 +0000 Subject: [PATCH 2/2] fix: address review feedback on PR #6823 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- cmd/mint-wasm/main.go | 3 +- internal/dispatch/cf/workersrc/src/index.ts | 60 +++++++++++++++++++-- internal/mintcore/github_test.go | 3 +- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/cmd/mint-wasm/main.go b/cmd/mint-wasm/main.go index 73586bffc5..27860a4314 100644 --- a/cmd/mint-wasm/main.go +++ b/cmd/mint-wasm/main.go @@ -152,7 +152,8 @@ func handleFetch(_ js.Value, args []js.Value) interface{} { // GitHub API calls (FindInstallation, CreateInstallationToken) // return a clean Go-side error before the JS-side // HANDLE_FETCH_TIMEOUT_MS (25 s) fires. Without this, a single - // slow API call permanently poisons the GoWasm singleton. + // slow API call blocks the GoWasm singleton until the JS-side + // timeout triggers recovery. ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) defer cancel() diff --git a/internal/dispatch/cf/workersrc/src/index.ts b/internal/dispatch/cf/workersrc/src/index.ts index 4cc1abc8d7..3d63877a57 100644 --- a/internal/dispatch/cf/workersrc/src/index.ts +++ b/internal/dispatch/cf/workersrc/src/index.ts @@ -302,26 +302,61 @@ const HANDLE_FETCH_TIMEOUT_MS = 25_000; class GoWasm { private initPromise: Promise | null = null; private _needsRecovery = false; + private _consecutiveTimeouts = 0; + + /** + * Maximum number of consecutive timeout recoveries before the + * instance reverts to permanent 503. Each recovery leaks one Go + * WASM runtime (blocked goroutine + memory); capping the count + * bounds leaked runtimes per isolate lifetime. + */ + static readonly MAX_CONSECUTIVE_RECOVERIES = 3; /** * Whether this instance needs recovery after a timeout. Unlike * the previous permanent-poison approach, the next request will - * re-initialize the Go WASM runtime automatically. + * re-initialize the Go WASM runtime automatically — up to + * MAX_CONSECUTIVE_RECOVERIES times. */ get needsRecovery(): boolean { return this._needsRecovery; } + /** + * Whether consecutive timeout recoveries have been exhausted. + * Once exhausted, the instance refuses all requests with 503 + * until the Workers runtime recycles the isolate. + */ + get exhausted(): boolean { + return ( + this._consecutiveTimeouts >= GoWasm.MAX_CONSECUTIVE_RECOVERIES + ); + } + + /** + * Reset the consecutive timeout counter. Called after a request + * completes successfully, proving the current Go runtime is + * healthy. + */ + resetTimeoutCounter(): void { + this._consecutiveTimeouts = 0; + } + /** * Mark this instance as needing recovery. Called when handleFetch * times out so that the next request re-initializes the Go WASM * runtime instead of permanently refusing all traffic. * + * Increments the consecutive timeout counter. After + * MAX_CONSECUTIVE_RECOVERIES consecutive timeouts, the instance + * is considered exhausted and reverts to permanent 503. + * * Clears the cached initPromise so that init() re-runs doInit on * the next call, booting a fresh Go runtime. */ markTimedOut(): void { this._needsRecovery = true; + this._consecutiveTimeouts++; this.initPromise = null; } @@ -513,9 +548,10 @@ class GoWasm { // Module-scoped singleton: one Go WASM instance per warm Worker isolate. // See GoWasm class comment for the architectural rationale. -// Declared `const`: the singleton is never replaced. On timeout, the -// instance is recovered in place — see markTimedOut() and the recovery -// strategy comment on the GoWasm class. +// Declared `const`: the object reference is never reassigned, but the +// instance internally boots a fresh Go WASM runtime after timeout +// recovery — see markTimedOut() and the recovery strategy comment on +// the GoWasm class. const goWasm = new GoWasm(); /** @@ -546,6 +582,17 @@ export default { env: Env, _ctx: ExecutionContext, ): Promise { + // After MAX_CONSECUTIVE_RECOVERIES consecutive timeouts, stop + // attempting recovery to bound leaked Go WASM runtimes. The + // Workers runtime must recycle the isolate to recover. + if (goWasm.exhausted) { + return errorResponse( + 503, + "mint instance exhausted after repeated timeouts — " + + "awaiting isolate recycle", + ); + } + // After a prior timeout, log a recovery notice. The next init() // call will boot a fresh Go WASM runtime automatically — unlike // the old permanent-poison approach, the mint recovers without @@ -624,6 +671,11 @@ export default { } } + // Request completed without timeout — the current Go runtime + // is healthy. Reset the consecutive timeout counter so that a + // future transient timeout gets the full recovery budget. + goWasm.resetTimeoutCounter(); + return new Response(result.body, { status: result.status, headers: respHeaders, diff --git a/internal/mintcore/github_test.go b/internal/mintcore/github_test.go index 4501389c45..88df4aea80 100644 --- a/internal/mintcore/github_test.go +++ b/internal/mintcore/github_test.go @@ -658,7 +658,7 @@ func TestFindInstallation_ContextDeadline(t *testing.T) { require.Error(t, err) // The error should stem from context deadline, not from the // server response. - assert.ErrorIs(t, context.DeadlineExceeded, context.DeadlineExceeded) + assert.ErrorIs(t, err, context.DeadlineExceeded) } func TestFindOrgInstallation_ContextDeadline(t *testing.T) { @@ -678,6 +678,7 @@ func TestFindOrgInstallation_ContextDeadline(t *testing.T) { _, err := FindOrgInstallation(ctx, mockGH.URL, "fake-jwt", "myorg") require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) } func TestGitHubUserAgent(t *testing.T) {