-
Notifications
You must be signed in to change notification settings - Fork 165
feat: add abort signal pass-through to ~19 providers #677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1057,4 +1057,78 @@ describe("AnthropicHandler", () => { | |||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| describe("abort signal", () => { | ||||||||||||||||||||||
| it("should pass abortSignal to the SDK options", async () => { | ||||||||||||||||||||||
| const controller = new AbortController() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| mockCreate.mockImplementation(async (options, requestOptions) => { | ||||||||||||||||||||||
| // Verify that the signal was passed | ||||||||||||||||||||||
| expect(requestOptions).toHaveProperty("signal", controller.signal) | ||||||||||||||||||||||
| return { | ||||||||||||||||||||||
| async *[Symbol.asyncIterator]() { | ||||||||||||||||||||||
| yield { | ||||||||||||||||||||||
| type: "message_start", | ||||||||||||||||||||||
| message: { usage: { input_tokens: 10, output_tokens: 5 } }, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| yield { | ||||||||||||||||||||||
| type: "content_block_delta", | ||||||||||||||||||||||
| delta: { type: "text_delta", text: "response" }, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const handler = new AnthropicHandler(mockOptions) | ||||||||||||||||||||||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }], { | ||||||||||||||||||||||
| taskId: "test", | ||||||||||||||||||||||
| tools: [], | ||||||||||||||||||||||
| abortSignal: controller.signal, | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const chunks: any[] = [] | ||||||||||||||||||||||
| for await (const chunk of stream) { | ||||||||||||||||||||||
| chunks.push(chunk) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| expect(chunks.length).toBeGreaterThan(0) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it("should work normally without abortSignal", async () => { | ||||||||||||||||||||||
| const handler = new AnthropicHandler(mockOptions) | ||||||||||||||||||||||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }]) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const chunks: any[] = [] | ||||||||||||||||||||||
| for await (const chunk of stream) { | ||||||||||||||||||||||
| chunks.push(chunk) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| expect(chunks.length).toBeGreaterThan(0) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it("should not pass signal when abortSignal is undefined", async () => { | ||||||||||||||||||||||
| mockCreate.mockImplementation(async (options, requestOptions) => { | ||||||||||||||||||||||
| // When no abortSignal is provided, requestOptions should be undefined or not have signal | ||||||||||||||||||||||
| expect(requestOptions).toBeUndefined() | ||||||||||||||||||||||
| return { | ||||||||||||||||||||||
|
Comment on lines
+1109
to
+1113
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert “no signal”, not “no options object”. Line 1112 is brittle: provider request options may include non-signal fields (e.g., headers) even when Suggested test fix- expect(requestOptions).toBeUndefined()
+ expect(requestOptions?.signal).toBeUndefined()As per coding guidelines, "Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling." 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||
| async *[Symbol.asyncIterator]() { | ||||||||||||||||||||||
| yield { | ||||||||||||||||||||||
| type: "message_start", | ||||||||||||||||||||||
| message: { usage: { input_tokens: 10, output_tokens: 5 } }, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const handler = new AnthropicHandler(mockOptions) | ||||||||||||||||||||||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }]) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const chunks: any[] = [] | ||||||||||||||||||||||
| for await (const chunk of stream) { | ||||||||||||||||||||||
| chunks.push(chunk) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| expect(chunks.length).toBeGreaterThan(0) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Abort test never triggers the abort path.
This test never calls
controller.abort(), so it validates normal streaming, not cancellation behavior.Suggested fix
it("should handle abort signal triggered during request", async () => { const controller = new AbortController() const handler = new AnthropicVertexHandler({ @@ const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }], { @@ abortSignal: controller.signal, }) - - const chunks: any[] = [] - for await (const chunk of stream) { - chunks.push(chunk) - } - - expect(chunks.length).toBeGreaterThan(0) + setTimeout(() => controller.abort(), 20) + await expect(async () => { + for await (const _chunk of stream) { + // consume stream + } + }).rejects.toThrow(/abort/i) })📝 Committable suggestion
🤖 Prompt for AI Agents