test(api): strengthen requesty abort assertions and share settle guard helper

Author: easonliang28

src/api/providers/__tests__/requesty.spec.ts

@@ -17,33 +17,7 @@ import { ApiHandlerCreateMessageMetadata } from "../../index"
 import { makeApiHandlerOptions } from "../../../test-utils/api"
 import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
 import { clearAllMocks } from "../../../test-utils/reset"
-
-/**
- * Stryker guard: fails fast if `promise` does not settle within `ms`.
- *
- * Stryker's per-mutant cutoff (timeoutMS 5s x timeoutFactor 1.5 ~= 7.5s) is shorter
- * than vitest's testTimeout (20s). A mutant that removes a settle call (or an abort
- * listener) leaves an awaited promise pending forever; without this guard the test
- * would outlive the cutoff and the mutant would be reported as Timeout (inconclusive).
- * Settling the guard at 500ms turns those mutants into fast failures (KILLED).
- */
-function withSettleGuard<T>(promise: Promise<T>, ms = 500): Promise<T> {
-	return new Promise<T>((resolve, reject) => {
-		const timer = setTimeout(() => {
-			reject(new Error(`settle guard timed out after ${ms}ms`))
-		}, ms)
-		void promise.then(
-			(value) => {
-				clearTimeout(timer)
-				resolve(value)
-			},
-			(error) => {
-				clearTimeout(timer)
-				reject(error)
-			},
-		)
-	})
-}
+import { withSettleGuard } from "../../../test-utils/settle-guard"
 
 const mockCreate = vitest.fn()
 
@@ -770,6 +744,7 @@ describe("RequestyHandler", () => {
 				expect.objectContaining({ model: expect.any(String) }),
 				expect.objectContaining({
 					timeout: 5000,
+					signal: expect.any(AbortSignal),
 				}),
 			)
 		})
@@ -795,6 +770,7 @@ describe("RequestyHandler", () => {
 				name: "AbortError",
 				message: "This operation was aborted",
 			})
+			expect(mockCreate).not.toHaveBeenCalled()
 		})
 
 		it("rejects with AbortError when the signal aborts during model lookup", async () => {

src/api/providers/requesty.ts

@@ -263,14 +263,14 @@ export class RequestyHandler extends BaseProvider implements SingleCompletionHan
 			// Aborted requests are user-initiated: surface them as AbortError (this also covers
 			// timeouts, which abort the same signal) instead of a completion error.
 			if (requestAbortSignal?.aborted) {
-				throw createAbortError("Requesty")
+				throw createAbortError(this.providerName)
 			}
 			throw handleOpenAIError(error, this.providerName)
 		}
 
 		if (requestAbortSignal?.aborted) {
 			// The response resolved after the request was aborted: do not return the late result.
-			throw createAbortError("Requesty")
+			throw createAbortError(this.providerName)
 		}
 		return response.choices[0]?.message.content || ""
 	}

src/api/providers/utils/__tests__/abort-signal.spec.ts

@@ -6,33 +6,7 @@ import {
 	rejectOnAbort,
 	throwIfAborted,
 } from "../abort-signal"
-
-/**
- * Stryker guard: fails fast if `promise` does not settle within `ms`.
- *
- * Stryker's per-mutant cutoff (timeoutMS 5s x timeoutFactor 1.5 ~= 7.5s) is shorter
- * than vitest's testTimeout (20s). A mutant that removes a settle call (or an abort
- * listener) leaves an awaited promise pending forever; without this guard the test
- * would outlive the cutoff and the mutant would be reported as Timeout (inconclusive).
- * Settling the guard at 500ms turns those mutants into fast failures (KILLED).
- */
-function withSettleGuard<T>(promise: Promise<T>, ms = 500): Promise<T> {
-	return new Promise<T>((resolve, reject) => {
-		const timer = setTimeout(() => {
-			reject(new Error(`settle guard timed out after ${ms}ms`))
-		}, ms)
-		void promise.then(
-			(value) => {
-				clearTimeout(timer)
-				resolve(value)
-			},
-			(error) => {
-				clearTimeout(timer)
-				reject(error)
-			},
-		)
-	})
-}
+import { withSettleGuard } from "../../../../test-utils/settle-guard"
 
 describe("rejectOnAbort", () => {
 	it("resolves with the pending value when it settles before the signal aborts", async () => {
@@ -64,6 +38,7 @@ describe("rejectOnAbort", () => {
 
 		await expect(withSettleGuard(rejectOnAbort(pending, controller.signal, "TestProvider"))).rejects.toMatchObject({
 			name: "AbortError",
+			message: "The TestProvider request was aborted",
 		})
 	})
 
@@ -78,26 +53,38 @@ describe("rejectOnAbort", () => {
 
 	it("detaches the abort listener once the pending settles", async () => {
 		const controller = new AbortController()
+		const addSpy = vi.spyOn(controller.signal, "addEventListener")
 		const removeSpy = vi.spyOn(controller.signal, "removeEventListener")
 
 		await expect(
 			withSettleGuard(rejectOnAbort(Promise.resolve("done"), controller.signal, "TestProvider")),
 		).resolves.toBe("done")
 
-		expect(removeSpy).toHaveBeenCalledWith("abort", expect.any(Function))
+		// The settle path must remove the exact listener that was registered, not just any
+		// function: removing a different reference would leave the original abort listener
+		// attached to the signal.
+		const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
+		expect(removeSpy).toHaveBeenCalledWith("abort", registeredListener)
+		addSpy.mockRestore()
 		removeSpy.mockRestore()
 	})
 
 	it("detaches the abort listener when the pending rejects", async () => {
 		const controller = new AbortController()
+		const addSpy = vi.spyOn(controller.signal, "addEventListener")
 		const removeSpy = vi.spyOn(controller.signal, "removeEventListener")
 		const lookupError = new Error("lookup failed")
 
 		await expect(
 			withSettleGuard(rejectOnAbort(Promise.reject(lookupError), controller.signal, "TestProvider")),
 		).rejects.toBe(lookupError)
 
-		expect(removeSpy).toHaveBeenCalledWith("abort", expect.any(Function))
+		// The settle path must remove the exact listener that was registered, not just any
+		// function: removing a different reference would leave the original abort listener
+		// attached to the signal.
+		const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
+		expect(removeSpy).toHaveBeenCalledWith("abort", registeredListener)
+		addSpy.mockRestore()
 		removeSpy.mockRestore()
 	})
 })

src/test-utils/settle-guard.ts

@@ -0,0 +1,26 @@
+/**
+ * Stryker guard: fails fast if `promise` does not settle within `ms`.
+ *
+ * Stryker's per-mutant cutoff (timeoutMS 5s x timeoutFactor 1.5 ~= 7.5s) is shorter
+ * than vitest's testTimeout (20s). A mutant that removes a settle call (or an abort
+ * listener) leaves an awaited promise pending forever; without this guard the test
+ * would outlive the cutoff and the mutant would be reported as Timeout (inconclusive).
+ * Settling the guard at 500ms turns those mutants into fast failures (KILLED).
+ */
+export function withSettleGuard<T>(promise: Promise<T>, ms = 500): Promise<T> {
+	return new Promise<T>((resolve, reject) => {
+		const timer = setTimeout(() => {
+			reject(new Error(`settle guard timed out after ${ms}ms`))
+		}, ms)
+		void promise.then(
+			(value) => {
+				clearTimeout(timer)
+				resolve(value)
+			},
+			(error) => {
+				clearTimeout(timer)
+				reject(error)
+			},
+		)
+	})
+}