Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
229 changes: 229 additions & 0 deletions src/api/providers/utils/__tests__/abort-signal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,227 @@ import {
isRequestAborted,
mergeAbortSignalAndTimeout,
mergeAbortSignals,
rejectOnAbort,
resolveModelWithAbort,
throwIfAborted,
} from "../abort-signal"
import { withSettleGuard } from "../../../../test-utils/settle-guard"

describe("rejectOnAbort", () => {
it("resolves with the pending value when it settles before the signal aborts", async () => {
const controller = new AbortController()

await expect(
withSettleGuard(rejectOnAbort(Promise.resolve("done"), controller.signal, "TestProvider")),
).resolves.toBe("done")
expect(controller.signal.aborted).toBe(false)
})

it("rejects with the provider abort error when the signal aborts first", async () => {
const controller = new AbortController()
// Never settles: the race must end purely via the abort.
const pending = new Promise<never>(() => {})
const race = rejectOnAbort(pending, controller.signal, "TestProvider")
controller.abort()

await expect(withSettleGuard(race)).rejects.toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})
})

it("rejects immediately when the signal is already aborted", async () => {
const controller = new AbortController()
controller.abort()
const pending = new Promise<never>(() => {})

await expect(withSettleGuard(rejectOnAbort(pending, controller.signal, "TestProvider"))).rejects.toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})
})

it("does not leave a rejecting pending unhandled when the signal is already aborted", async () => {
// The pre-aborted branch returns before consuming `pending`, so it must still
// attach a rejection handler; otherwise a rejecting lookup is reported as an
// unhandled rejection even though the caller already cancelled.
const controller = new AbortController()
controller.abort()
const lookupError = new Error("lookup failed")
const pending = Promise.reject(lookupError)

const unhandled: unknown[] = []
const onUnhandledRejection = (reason: unknown) => unhandled.push(reason)
process.on("unhandledRejection", onUnhandledRejection)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
try {
await expect(
withSettleGuard(rejectOnAbort(pending, controller.signal, "TestProvider")),
).rejects.toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})

// Node reports an unhandled rejection only after the microtask queue drains.
await new Promise((resolve) => setTimeout(resolve, 0))
expect(unhandled).toHaveLength(0)
} finally {
// Remove the process-level listener on every exit path: a failed assertion
// must not leak it into the remaining tests in this file.
process.off("unhandledRejection", onUnhandledRejection)
}
})

it("propagates the pending rejection when the signal stays active", async () => {
const controller = new AbortController()
const boom = new Error("lookup failed")

await expect(
withSettleGuard(rejectOnAbort(Promise.reject(boom), controller.signal, "TestProvider")),
).rejects.toBe(boom)
})

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")

// 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. First require the registration to have happened at all,
// so a missing registration cannot silently degrade to an undefined comparison.
expect(addSpy).toHaveBeenCalledTimes(1)
const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
expect(typeof registeredListener).toBe("function")
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)

// 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. First require the registration to have happened at all,
// so a missing registration cannot silently degrade to an undefined comparison.
expect(addSpy).toHaveBeenCalledTimes(1)
const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
expect(typeof registeredListener).toBe("function")
expect(removeSpy).toHaveBeenCalledWith("abort", registeredListener)
addSpy.mockRestore()
removeSpy.mockRestore()
})
})

describe("resolveModelWithAbort", () => {
it("fast-fails with the provider abort error when the signal is already aborted", async () => {
const controller = new AbortController()
controller.abort()
let lookupRan = false

await expect(
withSettleGuard(
resolveModelWithAbort(
async () => {
lookupRan = true
return "model"
},
controller.signal,
"TestProvider",
),
),
).rejects.toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})
// The entry guard must run before the lookup starts at all.
expect(lookupRan).toBe(false)
})

it("resolves the model when no abort signal is present", async () => {
await expect(resolveModelWithAbort(async () => "model", undefined, "TestProvider")).resolves.toBe("model")
})

it("propagates a raw resolution failure unchanged when no abort signal is present", async () => {
const boom = new Error("lookup failed")

await expect(
resolveModelWithAbort(
async () => {
throw boom
},
undefined,
"TestProvider",
),
).rejects.toBe(boom)
})

it("rejects with the provider abort error when the signal aborts while resolution is pending", async () => {
const controller = new AbortController()
let release: (value: string) => void = () => {}
const pending = new Promise<string>((resolve) => {
release = resolve
})
const race = withSettleGuard(resolveModelWithAbort(async () => pending, controller.signal, "TestProvider"))

setTimeout(() => controller.abort(), 10)

await expect(race).rejects.toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})
// A late resolution after the abort won the race must not surface.
release("late model")
})

it("normalizes an abort-flavored resolution failure to the provider abort error", async () => {
const controller = new AbortController()
const abortFlavored = new Error("The operation was aborted.")
abortFlavored.name = "AbortError"

const error = await resolveModelWithAbort(
async () => {
throw abortFlavored
},
controller.signal,
"TestProvider",
).catch((caught: unknown) => caught)

expect(error).not.toBe(abortFlavored)
expect(error).toMatchObject({
name: "AbortError",
message: "The TestProvider request was aborted",
})
})

it("propagates non-abort resolution failures unchanged when a signal is present", async () => {
const controller = new AbortController()
const boom = new Error("lookup failed")

await expect(
withSettleGuard(
resolveModelWithAbort(
async () => {
throw boom
},
controller.signal,
"TestProvider",
),
),
).rejects.toBe(boom)
})
})

describe("abort-signal utilities", () => {
describe("mergeAbortSignalAndTimeout", () => {
Expand Down Expand Up @@ -167,6 +386,16 @@ describe("abort-signal utilities", () => {
const controller = new AbortController()
expect(isRequestAborted(new Error("boom"), controller.signal)).toBe(false)
})

it("requires an Error instance for the name and message checks", () => {
// A plain object that merely looks like an abort must not be
// classified as one: the instanceof guard keeps such failures
// propagating unchanged so callers can inspect the real shape.
const fakeAbort = { name: "AbortError", message: "Request was aborted." }
expect(isRequestAborted(fakeAbort)).toBe(false)
expect(isRequestAborted(Object.assign(Object.create(null), { name: "APIUserAbortError" }))).toBe(false)
expect(isRequestAborted("Request was aborted.")).toBe(false)
})
})

describe("createAbortError", () => {
Expand Down
93 changes: 82 additions & 11 deletions src/api/providers/utils/abort-signal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,20 @@ export type OpenAiRequestOptions = {

/**
* Whether a failure indicates an aborted request: the caller's signal fired,
* the SDK raised a native abort error, or the error carries the OpenAI SDK
* abort error message (exactly "Request was aborted."). The message check
* is an exact match on purpose: a substring match would misclassify
* unrelated errors that merely mention aborting.
* an `Error` carries a native abort error name (`AbortError`,
* `APIUserAbortError`), or an `Error` carries the OpenAI SDK abort error
* message (exactly "Request was aborted.").
*
* The name and message checks require an `Error` instance on purpose: a
* plain object that merely looks like an abort must propagate unchanged so
* callers can inspect the real failure shape. The message check is an exact
* match on purpose: a substring match would misclassify unrelated errors
* that merely mention aborting.
*/
export function isRequestAborted(error: unknown, signal?: AbortSignal): boolean {
const candidate = error as { name?: string; message?: string }
return (
Boolean(signal?.aborted) ||
candidate?.name === "AbortError" ||
candidate?.name === "APIUserAbortError" ||
candidate?.message === "Request was aborted."
)
const hasAbortName = error instanceof Error && (error.name === "AbortError" || error.name === "APIUserAbortError")
const hasSdkAbortMessage = error instanceof Error && error.message === "Request was aborted."
return Boolean(signal?.aborted) || hasAbortName || hasSdkAbortMessage
}

/**
Expand All @@ -93,3 +94,73 @@ export function createAbortError(providerName: string): Error {
abortError.name = "AbortError"
return abortError
}

/**
* Await `pending` but reject with the provider's abort error when `signal`
* aborts first. For async phases that have no native signal support (model
* discovery) yet must still settle promptly on cancellation. The underlying
* promise keeps running (its settlement is ignored) — cancellation is
* cooperative at this boundary.
*
* The abort listener is detached once `pending` settles (success or
* failure), so repeated calls on one signal do not accumulate listeners.
*/
export function rejectOnAbort<T>(pending: Promise<T>, signal: AbortSignal, providerName: string): Promise<T> {
if (signal.aborted) {
// The caller has already cancelled, so this branch never consumes `pending`;
// attach a handler anyway so a rejecting `pending` cannot surface as an
// unhandled rejection.
void pending.catch(() => {})
return Promise.reject(createAbortError(providerName))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return new Promise<T>((resolve, reject) => {
const onAbort = () => reject(createAbortError(providerName))
// Stryker disable next-line ObjectLiteral,BooleanLiteral: a signal fires its abort event exactly once and the settle handler removes this listener, so the once flag is unobservable
signal.addEventListener("abort", onAbort, { once: true })
void pending.then(
(value) => {
signal.removeEventListener("abort", onAbort)
resolve(value)
},
(error) => {
signal.removeEventListener("abort", onAbort)
reject(error)
},
)
})
}

/**
* Resolve a provider's model metadata inside a cancellation scope.
*
* A pre-aborted signal rejects immediately with the standardized AbortError
* before any lookup starts; a signal that fires while the lookup is pending
* settles via {@link rejectOnAbort} instead of waiting for the catalog to
* resolve. Abort failures from the lookup itself are normalized to the
* provider AbortError; any other resolution failure propagates unchanged.
*
* Providers pass their own resolution step, so the entry guard, the race, and
* the normalization logic exist once and are exercised through every
* provider's spec.
*/
export async function resolveModelWithAbort<T>(
fetchModel: () => Promise<T>,
abortSignal: AbortSignal | undefined,
providerName: string,
): Promise<T> {
if (abortSignal?.aborted) {
throw createAbortError(providerName)
}

try {
if (abortSignal) {
return await rejectOnAbort(fetchModel(), abortSignal, providerName)
}
return await fetchModel()
} catch (error) {
if (isRequestAborted(error, abortSignal)) {
throw createAbortError(providerName)
}
throw error
}
}
26 changes: 26 additions & 0 deletions src/test-utils/settle-guard.ts
Original file line number Diff line number Diff line change
@@ -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)
},
)
})
}
Loading