diff --git a/internal/wasm/wasmhttp/transport.go b/internal/wasm/wasmhttp/transport.go index ba59c5c6..35eea99e 100644 --- a/internal/wasm/wasmhttp/transport.go +++ b/internal/wasm/wasmhttp/transport.go @@ -184,8 +184,8 @@ func (t *WasmHTTPTransport) doFetch(req *http.Request, options map[string]interf defer textCatch.Release() errMsg := "unknown error reading response body" - if len(textArgs) > 0 && !textArgs[0].IsUndefined() { - if msg := textArgs[0].Get("message"); !msg.IsUndefined() { + if len(textArgs) > 0 && textArgs[0].Type() == js.TypeObject { + if msg := textArgs[0].Get("message"); msg.Type() == js.TypeString { errMsg = msg.String() } } @@ -208,13 +208,21 @@ func (t *WasmHTTPTransport) doFetch(req *http.Request, options map[string]interf errMsg := "unknown fetch error" isAbort := false - if len(catchArgs) > 0 && !catchArgs[0].IsUndefined() { + // Value.Get panics on any non-object, and a panic on a js.FuncOf + // goroutine takes the whole WASM process down, so gate on the type + // rather than excluding null and undefined one at a time. + if len(catchArgs) > 0 && catchArgs[0].Type() == js.TypeObject { if name := catchArgs[0].Get("name"); !name.IsUndefined() && name.String() == "AbortError" { isAbort = true } - if msg := catchArgs[0].Get("message"); !msg.IsUndefined() { + if msg := catchArgs[0].Get("message"); msg.Type() == js.TypeString { errMsg = msg.String() } + // Node and some browsers report connection-level failures as a bare + // "fetch failed", with the real reason (ETIMEDOUT, ENOTFOUND) on cause. + if detail := fetchErrorCause(catchArgs[0]); detail != "" { + errMsg = fmt.Sprintf("%s (cause: %s)", errMsg, detail) + } } // An abort is expected behavior (context cancellation or timeout), not @@ -279,6 +287,25 @@ func (t *WasmHTTPTransport) doFetch(req *http.Request, options map[string]interf } } +// fetchErrorCause pulls a human-readable reason off a rejected fetch's cause, +// returning "" when there isn't one. Prefers the cause's message (it usually +// carries the address too) and falls back to its code. +func fetchErrorCause(rejection js.Value) string { + cause := rejection.Get("cause") + switch cause.Type() { + case js.TypeString: + return cause.String() + case js.TypeObject: + if msg := cause.Get("message"); msg.Type() == js.TypeString && msg.String() != "" { + return msg.String() + } + if code := cause.Get("code"); code.Type() == js.TypeString { + return code.String() + } + } + return "" +} + // extractHeaders extracts headers from the fetch Response object func (t *WasmHTTPTransport) extractHeaders(response js.Value) map[string]string { headers := make(map[string]string) diff --git a/internal/wasm/wasmhttp/transport_test.go b/internal/wasm/wasmhttp/transport_test.go index e6c62288..f99eb98a 100644 --- a/internal/wasm/wasmhttp/transport_test.go +++ b/internal/wasm/wasmhttp/transport_test.go @@ -137,6 +137,58 @@ func newFakeResponse(status int, body string) js.Value { return resp } +// newRejectingFetch returns fetch behavior that rejects immediately with +// whatever newReason builds, so a test can model any rejection shape the +// runtime might hand back, not just an Error. +func newRejectingFetch(newReason func() js.Value) func(url string, opts js.Value) js.Value { + return func(url string, opts js.Value) js.Value { + var executor js.Func + executor = js.FuncOf(func(this js.Value, args []js.Value) interface{} { + defer executor.Release() + args[1].Invoke(newReason()) + return nil + }) + return js.Global().Get("Promise").New(executor) + } +} + +// newTextRejectingFetch resolves with a response whose text() rejects with +// whatever newReason builds, so a test can drive the body-read failure path. +func newTextRejectingFetch(newReason func() js.Value) func(url string, opts js.Value) js.Value { + return func(url string, opts js.Value) js.Value { + var textFunc js.Func + textFunc = js.FuncOf(func(this js.Value, args []js.Value) interface{} { + defer textFunc.Release() + return js.Global().Get("Promise").Call("reject", newReason()) + }) + + resp := js.ValueOf(map[string]interface{}{"status": 200}) + resp.Set("headers", js.ValueOf(map[string]interface{}{})) + resp.Set("text", textFunc) + + var executor js.Func + executor = js.FuncOf(func(this js.Value, args []js.Value) interface{} { + defer executor.Release() + args[0].Invoke(resp) + return nil + }) + return js.Global().Get("Promise").New(executor) + } +} + +// newFetchError builds an Error carrying message. setCause, when non-nil, +// decorates it before it is thrown, so a test can model Node's bare +// "fetch failed" plus a cause. +func newFetchError(message string, setCause func(err js.Value)) func() js.Value { + return func() js.Value { + err := js.Global().Get("Error").New(message) + if setCause != nil { + setCause(err) + } + return err + } +} + func newTestRequest(t *testing.T, ctx context.Context, url string) *http.Request { t.Helper() req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) @@ -208,6 +260,128 @@ func TestRoundTrip_TimeoutAbortsAndReturnsError(t *testing.T) { assert.Equal(t, int32(1), atomic.LoadInt32(&aborted), "expected the browser fetch to be aborted on timeout") } +// The cause shapes here came from real Node 22 fetch failures: undici's own +// connect timeout carries both a code and a message, a dual-stack refusal +// rejects with an AggregateError whose message is empty, and a blocked port +// carries a message with no code at all. +func TestRoundTrip_FetchRejectionSurfacesCause(t *testing.T) { + objectCause := func(fields map[string]interface{}) func(js.Value) { + return func(err js.Value) { + err.Set("cause", js.ValueOf(fields)) + } + } + + tests := []struct { + name string + setCause func(js.Value) + want string + noCause bool + }{ + { + name: "prefers the cause message", + setCause: objectCause(map[string]interface{}{"code": "UND_ERR_CONNECT_TIMEOUT", "message": "Connect Timeout Error (attempted address: 10.0.0.1:443, timeout: 10000ms)"}), + want: "fetch failed (cause: Connect Timeout Error (attempted address: 10.0.0.1:443, timeout: 10000ms))", + }, + { + name: "falls back to the code when an AggregateError leaves the message empty", + setCause: objectCause(map[string]interface{}{"code": "ECONNREFUSED", "message": ""}), + want: "fetch failed (cause: ECONNREFUSED)", + }, + { + name: "message with no code", + setCause: objectCause(map[string]interface{}{"message": "bad port"}), + want: "fetch failed (cause: bad port)", + }, + { + name: "string cause", + setCause: func(err js.Value) { err.Set("cause", "ECONNREFUSED") }, + want: "fetch failed (cause: ECONNREFUSED)", + }, + { + name: "no cause leaves the message alone", + want: "fetch failed", + noCause: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + installMockFetch(t, newRejectingFetch(newFetchError("fetch failed", tt.setCause))) + + transport := &WasmHTTPTransport{Timeout: 5 * time.Second} + req := newTestRequest(t, context.Background(), "https://example.invalid/test") + + resp, err := transport.RoundTrip(req) + + assert.Nil(t, resp) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + if tt.noCause { + assert.NotContains(t, err.Error(), "cause:") + } + }) + } +} + +// A spec-compliant fetch always rejects with an Error, but a service worker or +// a patched global can reject with anything; this guards the non-object +// handling in doFetch's catch handler. +func TestRoundTrip_NonObjectRejectionDoesNotPanic(t *testing.T) { + tests := []struct { + name string + reason func() js.Value + }{ + {name: "null", reason: js.Null}, + {name: "undefined", reason: js.Undefined}, + {name: "bare string", reason: func() js.Value { return js.ValueOf("boom") }}, + {name: "number", reason: func() js.Value { return js.ValueOf(500) }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + installMockFetch(t, newRejectingFetch(tt.reason)) + + transport := &WasmHTTPTransport{Timeout: 5 * time.Second} + req := newTestRequest(t, context.Background(), "https://example.invalid/test") + + resp, err := transport.RoundTrip(req) + + assert.Nil(t, resp) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown fetch error") + }) + } +} + +// Same hazard one level down: response.text() can reject with anything too, so +// the body-read catch handler needs the same type gating as the fetch one. +func TestRoundTrip_NonObjectBodyRejectionDoesNotPanic(t *testing.T) { + tests := []struct { + name string + reason func() js.Value + }{ + {name: "bare string", reason: func() js.Value { return js.ValueOf("boom") }}, + {name: "object with a non-string message", reason: func() js.Value { + return js.ValueOf(map[string]interface{}{"message": 42}) + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + installMockFetch(t, newTextRejectingFetch(tt.reason)) + + transport := &WasmHTTPTransport{Timeout: 5 * time.Second} + req := newTestRequest(t, context.Background(), "https://example.invalid/test") + + resp, err := transport.RoundTrip(req) + + assert.Nil(t, resp) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown error reading response body") + }) + } +} + func TestRoundTrip_SuccessNormalResponseUnaffected(t *testing.T) { var aborted int32 installMockFetch(t, newResolvingFetch(t, http.StatusOK, `{"ok":true}`, &aborted)) diff --git a/scripts/wasm-smoke.mjs b/scripts/wasm-smoke.mjs index e79376b5..d051c0fc 100644 --- a/scripts/wasm-smoke.mjs +++ b/scripts/wasm-smoke.mjs @@ -74,29 +74,53 @@ function runCommand(cmd, timeoutMs = 60000) { }); } -console.log(`wasm-smoke: running "${command}" against ${environment} via fetch transport`); - -let result; -try { - result = await runCommand(command); -} catch (e) { - fail(e.message); +// One attempt: run the command and validate what came back. Throws on any +// failure so the retry loop decides whether it's terminal. +async function attempt() { + const result = await runCommand(command); + + if (result.error) throw new Error(`command returned an error: ${result.error}`); + + const output = (result.output || '').trim(); + if (!output) throw new Error('command produced no output'); + + let parsed; + try { + parsed = JSON.parse(output); + } catch (e) { + throw new Error(`output was not valid JSON: ${e.message}\n--- output (first 500 chars) ---\n${output.slice(0, 500)}`); + } + + // The CLI reports failures as a JSON error envelope on stdout, so a command + // that ran fine at the JS level can still carry a fetch or API error. Check + // that before asserting the shape, or every failure reads as a bad array. + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed) && parsed.error) { + throw new Error(`command failed: ${parsed.error.message || output.slice(0, 200)}`); + } + + if (!Array.isArray(parsed) || parsed.length === 0) { + throw new Error(`expected a non-empty JSON array, got: ${output.slice(0, 200)}`); + } + + return parsed; } -if (result.error) fail(`command returned an error: ${result.error}`); +console.log(`wasm-smoke: running "${command}" against ${environment} via fetch transport`); -const output = (result.output || '').trim(); -if (!output) fail('command produced no output'); +// The check hits a live API, so one connect timeout shouldn't fail a PR. +const attempts = 2; +const retryDelayMs = 5000; let parsed; -try { - parsed = JSON.parse(output); -} catch (e) { - fail(`output was not valid JSON: ${e.message}\n--- output (first 500 chars) ---\n${output.slice(0, 500)}`); -} - -if (!Array.isArray(parsed) || parsed.length === 0) { - fail(`expected a non-empty JSON array, got: ${output.slice(0, 200)}`); +for (let i = 1; i <= attempts; i++) { + try { + parsed = await attempt(); + break; + } catch (e) { + if (i === attempts) fail(e.message); + console.log(`wasm-smoke: attempt ${i} failed (${e.message}), retrying in ${retryDelayMs / 1000}s`); + await sleep(retryDelayMs); + } } console.log(`wasm-smoke: OK, round-tripped ${parsed.length} records through the fetch transport`);