fix(request): reject before onTimeout so stream stalls fail instead of ending cleanly

withTimeout invoked onTimeout before rejecting. forwardStreamingResponse's
onTimeout cancels the stream reader, and cancelling settles the pending
read() with {done: true} ahead of the rejection in the microtask queue, so
Promise.race resolved: a stalled upstream stream was forwarded to the
client as a clean end-of-stream — truncated body with a normal end(), no
status.lastError, and the onStreamError failover hook never fired. The
stall branch of the catch block was unreachable.

Rejecting first enqueues the race's rejection ahead of any settlement the
onTimeout side effects can cause, restoring the intended stall handling:
the response is destroyed, lastError records the stall, and onStreamError
fires. readErrorBody (the only other caller) uses a no-op onTimeout and is
unaffected.

The regression test pinning the old behavior is flipped to the intended
expectations and documents the mechanism.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB

Author: claude

lib/request/stream-failover-runtime.ts

@@ -48,8 +48,12 @@ export async function withTimeout<T>(
 			promise,
 			new Promise<T>((_resolve, reject) => {
 				timeout = setTimeout(() => {
-					onTimeout();
+					// Reject BEFORE onTimeout: onTimeout side effects (e.g. cancelling a
+					// stream reader) can settle `promise` with a clean value, and a
+					// settlement enqueued ahead of this rejection would win the race —
+					// turning a stall into a silent success.
 					reject(new Error(message));
+					onTimeout();
 				}, Math.max(1, timeoutMs));
 			}),
 		]);

test/stream-failover-runtime.test.ts

@@ -232,14 +232,12 @@ describe("forwardStreamingResponse", () => {
 		expect(res.chunks).toEqual([]);
 	});
 
-	it("currently ends a stalled stream cleanly instead of failing it", async () => {
-		// Pins a BUG (not the intended design): on a stall, withTimeout's
-		// onTimeout cancels the reader, which settles the pending read() with
-		// {done: true} BEFORE the rejection lands, so Promise.race resolves.
-		// The stall is reported as a clean end: truncated body, res.end(), no
-		// lastError, and onStreamError (the failover hook) never fires. The
-		// stall branch of the catch block is unreachable today. Fixed in the
-		// stacked follow-up PR, which flips these expectations.
+	it("records the stall error, destroys the response, and reports failure on timeout", async () => {
+		// Regression: withTimeout used to invoke onTimeout (which cancels the
+		// reader and thereby settles the pending read() with {done: true})
+		// before rejecting, so the race resolved and a stalled upstream was
+		// forwarded as a clean end-of-stream — truncated body, res.end(), no
+		// lastError, and onStreamError (the failover hook) never fired.
 		const res = new FakeServerResponse();
 		const status = createStatus();
 		const upstream = new Response(streamOf(
@@ -250,13 +248,14 @@ describe("forwardStreamingResponse", () => {
 		const onStreamError = vi.fn();
 		await expect(
 			forwardStreamingResponse(upstream, res.asServerResponse(), status, onStreamError, 25),
-		).resolves.toBe(true);
+		).resolves.toBe(false);
 
-		expect(onStreamError).not.toHaveBeenCalled();
-		expect(status.lastError).toBeNull();
-		expect(res.destroyed).toBe(false);
+		expect(onStreamError).toHaveBeenCalledTimes(1);
+		expect(status.lastError).toBe("upstream stream stalled after 25ms");
+		expect(res.destroyed).toBe(true);
+		expect(res.destroyError).toBeInstanceOf(Error);
 		expect(Buffer.concat(res.chunks).toString("utf8")).toBe("data: a\n\n");
-		expect(res.ended).toBe(true);
+		expect(res.ended).toBe(false);
 	});
 
 	it("fails the stream when the upstream read rejects mid-stream", async () => {