Repository navigation
fix(sdk): add AbortSignal.any fallback in chat transport for Safari < 17.4 #5100
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 |
|---|---|---|
|
|
@@ -278,3 +278,30 @@ export function waitBeforeEofResubscribe(attempt: number, signal: AbortSignal): | |
| if (signal.aborted) done(); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Combine multiple `AbortSignal` instances into a single signal, falling back | ||
| * to manual listener forwarding on runtimes without `AbortSignal.any` (such as | ||
| * Safari / iOS Safari < 17.4). | ||
| */ | ||
| export function anyAbortSignal(signals: AbortSignal[]): AbortSignal { | ||
| if (typeof AbortSignal.any === "function") { | ||
| return AbortSignal.any(signals); | ||
| } | ||
|
|
||
| const controller = new AbortController(); | ||
| const aborted = signals.find((s) => s.aborted); | ||
| if (aborted) { | ||
| controller.abort(aborted.reason); | ||
| return controller.signal; | ||
| } | ||
|
|
||
| for (const s of signals) { | ||
| s.addEventListener("abort", () => controller.abort(s.reason), { | ||
| once: true, | ||
| signal: controller.signal, | ||
| }); | ||
|
Comment on lines
+299
to
+303
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. 🟡 Finished streams retain abort listeners On older Safari, Learn moreThe fallback removes a source listener only when the composed controller aborts. An Example: A page reuses one Recommended fix: Give the fallback an explicit disposal mechanism and call it on stream teardown, including the outer reconnect composition; alternatively ensure every composed controller aborts on every terminal path. Verify normal completion and settled-EOF cleanup as well as cancellation. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| return controller.signal; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2950,4 +2950,74 @@ describe("TriggerChatTransport", () => { | |
| expect(textChunks).toHaveLength(2); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Safari < 17.4 compatibility (no AbortSignal.any)", () => { | ||
| it("streams and propagates caller aborts when AbortSignal.any is undefined", async () => { | ||
|
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. |
||
| const originalAny = AbortSignal.any; | ||
| Object.defineProperty(AbortSignal, "any", { | ||
| value: undefined, | ||
| configurable: true, | ||
| writable: true, | ||
| }); | ||
|
|
||
| try { | ||
| global.fetch = vi.fn().mockImplementation(async (url: string | URL) => { | ||
|
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. 🟡 Browser regression test uses prohibited mocks The new Safari test replaces Learn moreThe repository's Example: The compatibility check serves synthetic responses from a Recommended fix: Use an integration-style test with a real local HTTP endpoint and restore the original environment afterward, or move signal-composition assertions into a focused test that needs no fetch replacement. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| const urlStr = typeof url === "string" ? url : url.toString(); | ||
| if (isSessionStreamAppendUrl(urlStr)) return defaultAppendResponse(); | ||
| if (isSessionOutSubscribeUrl(urlStr)) { | ||
| return defaultSseResponse([ | ||
| { type: "text-delta", id: "p1", delta: "Safari fallback" }, | ||
| { type: "trigger:turn-complete" }, | ||
| ]); | ||
| } | ||
| throw new Error(`Unexpected URL: ${urlStr}`); | ||
| }); | ||
|
|
||
| const transport = new TriggerChatTransport({ | ||
| task: "my-chat-task", | ||
| accessToken: () => "pat", | ||
| sessions: { | ||
| "chat-safari": { publicAccessToken: "p", isStreaming: true }, | ||
| }, | ||
| }); | ||
|
|
||
| const callerAbort = new AbortController(); | ||
| const stream = await transport.sendMessages({ | ||
| trigger: "submit-message", | ||
| chatId: "chat-safari", | ||
| messageId: undefined, | ||
| messages: [createUserMessage("Hi from Safari 16")], | ||
| abortSignal: callerAbort.signal, | ||
| }); | ||
| const chunks = await drainChunks(stream); | ||
| expect(chunks).toEqual([{ type: "text-delta", id: "p1", delta: "Safari fallback" }]); | ||
|
Comment on lines
+2984
to
+2993
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. |
||
|
|
||
| const preAborted = new AbortController(); | ||
| preAborted.abort(new Error("user stopped")); | ||
| let receivedAbortOnSubscribe = false; | ||
| global.fetch = vi.fn().mockImplementation(async (url: string | URL, init?: RequestInit) => { | ||
| const urlStr = typeof url === "string" ? url : url.toString(); | ||
| if (isSessionOutSubscribeUrl(urlStr)) { | ||
| receivedAbortOnSubscribe = Boolean(init?.signal?.aborted); | ||
| return defaultSseResponse([{ type: "trigger:turn-complete" }]); | ||
| } | ||
| throw new Error(`Unexpected URL: ${urlStr}`); | ||
| }); | ||
|
|
||
| const resumed = await transport.reconnectToStream({ | ||
| chatId: "chat-safari", | ||
| abortSignal: preAborted.signal, | ||
| }); | ||
| await drainChunks(resumed!); | ||
| expect(receivedAbortOnSubscribe).toBe(true); | ||
| } finally { | ||
| Object.defineProperty(AbortSignal, "any", { | ||
| value: originalAny, | ||
| configurable: true, | ||
| writable: true, | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
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.
🟡 Safari fix lacks a release note
The
anyAbortSignalchange fixes a customer-facing SDK failure, but this PR adds no changeset. The fix lacks the required patch release entry.Learn more
The repository's
AGENTS.mdandCONTRIBUTING.mdrequire a changeset for user-facing fixes in public packages. This change lets customers use chat transport on older Safari versions, so it meets that requirement.Example: A customer upgrading the SDK to avoid
AbortSignal.any is not a functioncannot find this fix in the package's changeset-driven release notes.Recommended fix: Add a patch changeset for
@trigger.dev/sdkwith a user-focused description of Safari chat compatibility.Was this helpful? React with 👍 or 👎 to provide feedback.