Skip to content

Commit a3fb762

Browse files
easonliang28easonliang28
committed
feat(api): add abort-signal cancellation-scope helpers to the shared utils
Extend src/api/providers/utils/abort-signal.ts with the abort-signal series helpers used by the gateway providers: - isRequestAborted(error, signal): wider abort detection - an aborted signal, a DOM AbortError, the OpenAI/Anthropic SDK APIUserAbortError (name check), or the exact SDK abort message "Request was aborted." - trusting name/message only on real Error instances so a plain object that merely looks like an abort propagates unchanged - createAbortError(providerName): fresh error satisfying the Task.ts abort contract (name "AbortError", message ending in "aborted") - rejectOnAbort(pending, signal, providerName): settle a signal-less async phase (model discovery) on the provider AbortError when the signal fires first; the abort listener detaches when pending settles - resolveModelWithAbort(fetchModel, signal, providerName): run model resolution inside a cancellation scope - entry fast-fail for a pre-aborted signal, the rejectOnAbort race while the lookup is pending, and normalization of abort-flavored lookup failures; any other resolution failure propagates unchanged Includes direct unit tests for the resolveModelWithAbort cancellation scope (pre-aborted fast-fail, no-signal pass-through, mid-resolution race, abort normalization, non-abort propagation), the isRequestAborted instanceof tightening tests, and the settle-guard test utility. Unit 1/3 of the #1295 split (content source: 62f596c). Part of the abort-signal series (round 1). Builds on #674, #901, #1008. Addresses #404.
1 parent 99025b1 commit a3fb762

3 files changed

Lines changed: 304 additions & 57 deletions

File tree

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

Lines changed: 199 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,196 @@ import {
33
isRequestAborted,
44
mergeAbortSignalAndTimeout,
55
mergeAbortSignals,
6-
throwIfAborted,
6+
rejectOnAbort,
7+
resolveModelWithAbort,
78
} from "../abort-signal"
9+
import { withSettleGuard } from "../../../../test-utils/settle-guard"
10+
11+
describe("rejectOnAbort", () => {
12+
it("resolves with the pending value when it settles before the signal aborts", async () => {
13+
const controller = new AbortController()
14+
15+
await expect(
16+
withSettleGuard(rejectOnAbort(Promise.resolve("done"), controller.signal, "TestProvider")),
17+
).resolves.toBe("done")
18+
expect(controller.signal.aborted).toBe(false)
19+
})
20+
21+
it("rejects with the provider abort error when the signal aborts first", async () => {
22+
const controller = new AbortController()
23+
// Never settles: the race must end purely via the abort.
24+
const pending = new Promise<never>(() => {})
25+
const race = rejectOnAbort(pending, controller.signal, "TestProvider")
26+
controller.abort()
27+
28+
await expect(withSettleGuard(race)).rejects.toMatchObject({
29+
name: "AbortError",
30+
message: "The TestProvider request was aborted",
31+
})
32+
})
33+
34+
it("rejects immediately when the signal is already aborted", async () => {
35+
const controller = new AbortController()
36+
controller.abort()
37+
const pending = new Promise<never>(() => {})
38+
39+
await expect(withSettleGuard(rejectOnAbort(pending, controller.signal, "TestProvider"))).rejects.toMatchObject({
40+
name: "AbortError",
41+
message: "The TestProvider request was aborted",
42+
})
43+
})
44+
45+
it("propagates the pending rejection when the signal stays active", async () => {
46+
const controller = new AbortController()
47+
const boom = new Error("lookup failed")
48+
49+
await expect(
50+
withSettleGuard(rejectOnAbort(Promise.reject(boom), controller.signal, "TestProvider")),
51+
).rejects.toBe(boom)
52+
})
53+
54+
it("detaches the abort listener once the pending settles", async () => {
55+
const controller = new AbortController()
56+
const addSpy = vi.spyOn(controller.signal, "addEventListener")
57+
const removeSpy = vi.spyOn(controller.signal, "removeEventListener")
58+
59+
await expect(
60+
withSettleGuard(rejectOnAbort(Promise.resolve("done"), controller.signal, "TestProvider")),
61+
).resolves.toBe("done")
62+
63+
// The settle path must remove the exact listener that was registered, not just any
64+
// function: removing a different reference would leave the original abort listener
65+
// attached to the signal. First require the registration to have happened at all,
66+
// so a missing registration cannot silently degrade to an undefined comparison.
67+
expect(addSpy).toHaveBeenCalledTimes(1)
68+
const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
69+
expect(typeof registeredListener).toBe("function")
70+
expect(removeSpy).toHaveBeenCalledWith("abort", registeredListener)
71+
addSpy.mockRestore()
72+
removeSpy.mockRestore()
73+
})
74+
75+
it("detaches the abort listener when the pending rejects", async () => {
76+
const controller = new AbortController()
77+
const addSpy = vi.spyOn(controller.signal, "addEventListener")
78+
const removeSpy = vi.spyOn(controller.signal, "removeEventListener")
79+
const lookupError = new Error("lookup failed")
80+
81+
await expect(
82+
withSettleGuard(rejectOnAbort(Promise.reject(lookupError), controller.signal, "TestProvider")),
83+
).rejects.toBe(lookupError)
84+
85+
// The settle path must remove the exact listener that was registered, not just any
86+
// function: removing a different reference would leave the original abort listener
87+
// attached to the signal. First require the registration to have happened at all,
88+
// so a missing registration cannot silently degrade to an undefined comparison.
89+
expect(addSpy).toHaveBeenCalledTimes(1)
90+
const registeredListener = addSpy.mock.calls[0]?.[1] as EventListener | undefined
91+
expect(typeof registeredListener).toBe("function")
92+
expect(removeSpy).toHaveBeenCalledWith("abort", registeredListener)
93+
addSpy.mockRestore()
94+
removeSpy.mockRestore()
95+
})
96+
})
97+
98+
describe("resolveModelWithAbort", () => {
99+
it("fast-fails with the provider abort error when the signal is already aborted", async () => {
100+
const controller = new AbortController()
101+
controller.abort()
102+
let lookupRan = false
103+
104+
await expect(
105+
withSettleGuard(
106+
resolveModelWithAbort(
107+
async () => {
108+
lookupRan = true
109+
return "model"
110+
},
111+
controller.signal,
112+
"TestProvider",
113+
),
114+
),
115+
).rejects.toMatchObject({
116+
name: "AbortError",
117+
message: "The TestProvider request was aborted",
118+
})
119+
// The entry guard must run before the lookup starts at all.
120+
expect(lookupRan).toBe(false)
121+
})
122+
123+
it("resolves the model when no abort signal is present", async () => {
124+
await expect(resolveModelWithAbort(async () => "model", undefined, "TestProvider")).resolves.toBe("model")
125+
})
126+
127+
it("propagates a raw resolution failure unchanged when no abort signal is present", async () => {
128+
const boom = new Error("lookup failed")
129+
130+
await expect(
131+
resolveModelWithAbort(
132+
async () => {
133+
throw boom
134+
},
135+
undefined,
136+
"TestProvider",
137+
),
138+
).rejects.toBe(boom)
139+
})
140+
141+
it("rejects with the provider abort error when the signal aborts while resolution is pending", async () => {
142+
const controller = new AbortController()
143+
let release: (value: string) => void = () => {}
144+
const pending = new Promise<string>((resolve) => {
145+
release = resolve
146+
})
147+
const race = withSettleGuard(resolveModelWithAbort(async () => pending, controller.signal, "TestProvider"))
148+
149+
setTimeout(() => controller.abort(), 10)
150+
151+
await expect(race).rejects.toMatchObject({
152+
name: "AbortError",
153+
message: "The TestProvider request was aborted",
154+
})
155+
// A late resolution after the abort won the race must not surface.
156+
release("late model")
157+
})
158+
159+
it("normalizes an abort-flavored resolution failure to the provider abort error", async () => {
160+
const controller = new AbortController()
161+
const abortFlavored = new Error("The operation was aborted.")
162+
abortFlavored.name = "AbortError"
163+
164+
const error = await resolveModelWithAbort(
165+
async () => {
166+
throw abortFlavored
167+
},
168+
controller.signal,
169+
"TestProvider",
170+
).catch((caught: unknown) => caught)
171+
172+
expect(error).not.toBe(abortFlavored)
173+
expect(error).toMatchObject({
174+
name: "AbortError",
175+
message: "The TestProvider request was aborted",
176+
})
177+
})
178+
179+
it("propagates non-abort resolution failures unchanged when a signal is present", async () => {
180+
const controller = new AbortController()
181+
const boom = new Error("lookup failed")
182+
183+
await expect(
184+
withSettleGuard(
185+
resolveModelWithAbort(
186+
async () => {
187+
throw boom
188+
},
189+
controller.signal,
190+
"TestProvider",
191+
),
192+
),
193+
).rejects.toBe(boom)
194+
})
195+
})
8196

9197
describe("abort-signal utilities", () => {
10198
describe("mergeAbortSignalAndTimeout", () => {
@@ -106,34 +294,6 @@ describe("abort-signal utilities", () => {
106294
})
107295
})
108296

109-
describe("throwIfAborted", () => {
110-
it("does not throw when signal is undefined", () => {
111-
expect(() => throwIfAborted()).not.toThrow()
112-
})
113-
114-
it("does not throw when signal is not aborted", () => {
115-
const controller = new AbortController()
116-
117-
expect(() => throwIfAborted(controller.signal)).not.toThrow()
118-
})
119-
120-
it("throws an AbortError when signal is already aborted", () => {
121-
const controller = new AbortController()
122-
controller.abort()
123-
124-
let caught: unknown
125-
try {
126-
throwIfAborted(controller.signal)
127-
} catch (error) {
128-
caught = error
129-
}
130-
131-
expect(caught).toBeInstanceOf(Error)
132-
expect((caught as Error).name).toBe("AbortError")
133-
expect((caught as Error).message).toBe("This operation was aborted")
134-
})
135-
})
136-
137297
describe("isRequestAborted", () => {
138298
it("returns true when the caller signal is aborted", () => {
139299
const controller = new AbortController()
@@ -167,6 +327,16 @@ describe("abort-signal utilities", () => {
167327
const controller = new AbortController()
168328
expect(isRequestAborted(new Error("boom"), controller.signal)).toBe(false)
169329
})
330+
331+
it("requires an Error instance for the name and message checks", () => {
332+
// A plain object that merely looks like an abort must not be
333+
// classified as one: the instanceof guard keeps such failures
334+
// propagating unchanged so callers can inspect the real shape.
335+
const fakeAbort = { name: "AbortError", message: "Request was aborted." }
336+
expect(isRequestAborted(fakeAbort)).toBe(false)
337+
expect(isRequestAborted(Object.assign(Object.create(null), { name: "APIUserAbortError" }))).toBe(false)
338+
expect(isRequestAborted("Request was aborted.")).toBe(false)
339+
})
170340
})
171341

172342
describe("createAbortError", () => {

‎src/api/providers/utils/abort-signal.ts‎

Lines changed: 79 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,6 @@ export function mergeAbortSignals(primarySignal: AbortSignal, secondarySignal?:
3636
return AbortSignal.any([primarySignal, secondarySignal])
3737
}
3838

39-
/**
40-
* Throw an AbortError if the given signal is already aborted.
41-
*
42-
* Use as a fast-fail guard at the top of request-building code paths so
43-
* callers receive a consistent `name === "AbortError"` when the operation
44-
* was cancelled before it started, without building or issuing the request.
45-
*/
46-
export function throwIfAborted(signal?: AbortSignal): void {
47-
if (!signal?.aborted) {
48-
return
49-
}
50-
51-
const abortError = new Error("This operation was aborted")
52-
abortError.name = "AbortError"
53-
throw abortError
54-
}
55-
5639
/**
5740
* Request options this series passes to the OpenAI SDK call. The SDK's
5841
* `RequestOptions` declares `signal` as `AbortSignal | null | undefined`,
@@ -66,19 +49,20 @@ export type OpenAiRequestOptions = {
6649

6750
/**
6851
* Whether a failure indicates an aborted request: the caller's signal fired,
69-
* the SDK raised a native abort error, or the error carries the OpenAI SDK
70-
* abort error message (exactly "Request was aborted."). The message check
71-
* is an exact match on purpose: a substring match would misclassify
72-
* unrelated errors that merely mention aborting.
52+
* an `Error` carries a native abort error name (`AbortError`,
53+
* `APIUserAbortError`), or an `Error` carries the OpenAI SDK abort error
54+
* message (exactly "Request was aborted.").
55+
*
56+
* The name and message checks require an `Error` instance on purpose: a
57+
* plain object that merely looks like an abort must propagate unchanged so
58+
* callers can inspect the real failure shape. The message check is an exact
59+
* match on purpose: a substring match would misclassify unrelated errors
60+
* that merely mention aborting.
7361
*/
7462
export function isRequestAborted(error: unknown, signal?: AbortSignal): boolean {
75-
const candidate = error as { name?: string; message?: string }
76-
return (
77-
Boolean(signal?.aborted) ||
78-
candidate?.name === "AbortError" ||
79-
candidate?.name === "APIUserAbortError" ||
80-
candidate?.message === "Request was aborted."
81-
)
63+
const hasAbortName = error instanceof Error && (error.name === "AbortError" || error.name === "APIUserAbortError")
64+
const hasSdkAbortMessage = error instanceof Error && error.message === "Request was aborted."
65+
return Boolean(signal?.aborted) || hasAbortName || hasSdkAbortMessage
8266
}
8367

8468
/**
@@ -93,3 +77,70 @@ export function createAbortError(providerName: string): Error {
9377
abortError.name = "AbortError"
9478
return abortError
9579
}
80+
81+
/**
82+
* Await `pending` but reject with the provider's abort error when `signal`
83+
* aborts first. For async phases that have no native signal support (model
84+
* discovery) yet must still settle promptly on cancellation. The underlying
85+
* promise keeps running (its settlement is ignored) — cancellation is
86+
* cooperative at this boundary.
87+
*
88+
* The abort listener is detached once `pending` settles (success or
89+
* failure), so repeated calls on one signal do not accumulate listeners.
90+
*/
91+
export function rejectOnAbort<T>(pending: Promise<T>, signal: AbortSignal, providerName: string): Promise<T> {
92+
if (signal.aborted) {
93+
return Promise.reject(createAbortError(providerName))
94+
}
95+
96+
return new Promise<T>((resolve, reject) => {
97+
const onAbort = () => reject(createAbortError(providerName))
98+
// 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
99+
signal.addEventListener("abort", onAbort, { once: true })
100+
void pending.then(
101+
(value) => {
102+
signal.removeEventListener("abort", onAbort)
103+
resolve(value)
104+
},
105+
(error) => {
106+
signal.removeEventListener("abort", onAbort)
107+
reject(error)
108+
},
109+
)
110+
})
111+
}
112+
113+
/**
114+
* Resolve a provider's model metadata inside a cancellation scope.
115+
*
116+
* A pre-aborted signal rejects immediately with the standardized AbortError
117+
* before any lookup starts; a signal that fires while the lookup is pending
118+
* settles via {@link rejectOnAbort} instead of waiting for the catalog to
119+
* resolve. Abort failures from the lookup itself are normalized to the
120+
* provider AbortError; any other resolution failure propagates unchanged.
121+
*
122+
* Providers pass their own resolution step, so the entry guard, the race, and
123+
* the normalization logic exist once and are exercised through every
124+
* provider's spec.
125+
*/
126+
export async function resolveModelWithAbort<T>(
127+
fetchModel: () => Promise<T>,
128+
abortSignal: AbortSignal | undefined,
129+
providerName: string,
130+
): Promise<T> {
131+
if (abortSignal?.aborted) {
132+
throw createAbortError(providerName)
133+
}
134+
135+
try {
136+
if (abortSignal) {
137+
return await rejectOnAbort(fetchModel(), abortSignal, providerName)
138+
}
139+
return await fetchModel()
140+
} catch (error) {
141+
if (isRequestAborted(error, abortSignal)) {
142+
throw createAbortError(providerName)
143+
}
144+
throw error
145+
}
146+
}

0 commit comments

Comments
 (0)