feat(ai): add abort signals and timeouts to media generation activities - #1047
Merged
AlemTuzlak merged 2 commits intoAug 7, 2026
Conversation
Media generation activities accept optional `timeout` and `abortSignal`. Core composes them into a request-specific effective signal, races the adapter call so hung providers reject, clears timeout resources on settle, and routes aborts to middleware `onAbort`. Fal adapters forward the signal to fal.subscribe/queue.submit per request rather than via global fal.config.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughMedia activities now support caller abort signals and timeouts. Core composes and manages cancellation, routes aborts through middleware, and forwards signals to adapters. FAL adapters pass signals to request-level provider calls without global configuration. ChangesMedia Activity Cancellation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant MediaActivity
participant MediaAdapter
participant ProviderSDK
participant GenerationMiddleware
Caller->>MediaActivity: invoke with timeout or abortSignal
MediaActivity->>MediaAdapter: call with effective abortSignal
MediaAdapter->>ProviderSDK: submit request with abortSignal
ProviderSDK-->>MediaAdapter: result or cancellation
MediaActivity->>GenerationMiddleware: runGenerationAbort on cancellation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Changeset Version Preview21 package(s) bumped directly, 31 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 3b61c4e
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/ai/src/utilities/activity-abort.ts (1)
175-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClassify by error shape first, and share
ABORT_ERROR_NAMES.
isActivityAbortErrorreturnstruewhenever the signal is aborted, before it inspects the error. If the timeout fires while an unrelated provider error propagates, the real failure routes toonAbortand never reachesonError. Check the error name first, then fall back to the signal state.
packages/ai/src/activities/error-payload.tsdefines its ownABORT_ERROR_NAMESset fortoRunErrorPayload. Export one shared constant so the two abort classifiers cannot drift.♻️ Proposed ordering change
export function isActivityAbortError( error: unknown, signal?: AbortSignal, ): boolean { - if (signal?.aborted) return true - if (!error || typeof error !== 'object') return false - const name = (error as { name?: unknown }).name - return typeof name === 'string' && ABORT_ERROR_NAMES.has(name) + if (error && typeof error === 'object') { + const name = (error as { name?: unknown }).name + if (typeof name === 'string') return ABORT_ERROR_NAMES.has(name) + } + return signal?.aborted === true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/src/utilities/activity-abort.ts` around lines 175 - 183, Update isActivityAbortError to classify the error name first, returning true only for recognized abort names, then fall back to signal?.aborted when no abort error shape is present. Export the existing ABORT_ERROR_NAMES constant from error-payload.ts and reuse that shared constant in activity-abort.ts, removing the duplicate definition.packages/ai/tests/activity-abort.test.ts (1)
127-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the timer count to prove the timer was cleared.
The test name claims the timer is cleared, but
expect(generateImages).toHaveBeenCalledTimes(1)does not verify that. Nothing in this test would call the adapter a second time, so the assertion passes even if the timeout timer is still pending. Usevi.getTimerCount()to check the pending-timer state directly.Consider also adding a test that asserts a long-lived caller
AbortSignalaccumulates no listeners across repeatedgenerateImagecalls. That guards the composed-signal cleanup path.💚 Proposed assertion
await expect(resultPromise).resolves.toMatchObject({ id: 'img-1', }) - // Advancing past the original timeout must not throw or leave a hanging - // timer that would abort a subsequent unrelated operation. - await vi.advanceTimersByTimeAsync(5_000) - expect(generateImages).toHaveBeenCalledTimes(1) + // The activity must clear its timeout timer on success. + expect(vi.getTimerCount()).toBe(0) + + // Advancing past the original timeout must not throw. + await vi.advanceTimersByTimeAsync(5_000) + expect(generateImages).toHaveBeenCalledTimes(1)As per coding guidelines: "Use Vitest for unit tests".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/tests/activity-abort.test.ts` around lines 127 - 150, Update the successful-completion test around generateImage to assert vi.getTimerCount() is zero after resolution and remains zero after advancing timers, replacing the ineffective generateImages call-count assertion as the timer-cleanup check. Also add a Vitest test covering repeated generateImage calls with a long-lived caller AbortSignal, verifying composed-signal listeners do not accumulate.Source: Coding guidelines
packages/ai-fal/tests/image-adapter.test.ts (1)
152-194: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlace this test alongside the adapter source.
Move this test file to
packages/ai-fal/src/adapters/image.test.ts. The coding guideline requires colocated*.test.tsfiles.As per coding guidelines, “Test files should be placed alongside source code as *.test.ts files using Vitest with happy-dom for DOM testing.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-fal/tests/image-adapter.test.ts` around lines 152 - 194, Move the image adapter test suite containing the request-specific and timeout abortSignal cases to the adapter source directory as image.test.ts, preserving its Vitest setup and test behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-fal/tests/image-adapter.test.ts`:
- Around line 167-175: Strengthen the cancellation test around mockSubscribe by
capture the abort signal passed in options, abort the existing caller controller
with a specific reason, and assert that the captured signal is aborted with that
reason. Keep the existing request-scoped options and fal.config() assertions
unchanged.
In `@packages/ai/src/activities/generateAudio/index.ts`:
- Around line 236-247: Update the adapter option interfaces used by
generateAudio, specifically AudioGenerationOptions and any provider-specific
extensions, to declare the optional abortSignal passed by runGenerateAudio.
Ensure the adapter.generateAudio call preserves and accepts this signal without
type errors, while retaining existing provider option fields.
In `@packages/ai/src/activities/generateImage/index.ts`:
- Around line 290-293: Ensure the middleware-start phase invoking
runGenerationStart is covered by cleanup for abortControls, so a rejection
clears the timeout before propagating the error. Update the surrounding flow in
the image-generation activity without changing normal successful execution or
later error handling.
In `@packages/ai/src/activities/summarize/index.ts`:
- Around line 102-113: Update runStreamingSummarize to compose and pass the
timeout and abortSignal controls to adapter.summarizeStream, matching
runSummarize. In its non-streaming fallback, preserve the caller’s run identity
and forward the effective abort controls when invoking runSummarize, while
retaining the existing streaming behavior.
In `@packages/ai/src/utilities/activity-abort.ts`:
- Around line 25-40: Update combineAbortSignals to return both the combined
signal and a disposer that removes its abort listeners, while preserving
existing undefined, aborted, and propagation behavior. Store and invoke that
disposer from ActivityAbortControls.clear() alongside timer cleanup, ensuring
repeated or settled activities release listeners.
---
Nitpick comments:
In `@packages/ai-fal/tests/image-adapter.test.ts`:
- Around line 152-194: Move the image adapter test suite containing the
request-specific and timeout abortSignal cases to the adapter source directory
as image.test.ts, preserving its Vitest setup and test behavior unchanged.
In `@packages/ai/src/utilities/activity-abort.ts`:
- Around line 175-183: Update isActivityAbortError to classify the error name
first, returning true only for recognized abort names, then fall back to
signal?.aborted when no abort error shape is present. Export the existing
ABORT_ERROR_NAMES constant from error-payload.ts and reuse that shared constant
in activity-abort.ts, removing the duplicate definition.
In `@packages/ai/tests/activity-abort.test.ts`:
- Around line 127-150: Update the successful-completion test around
generateImage to assert vi.getTimerCount() is zero after resolution and remains
zero after advancing timers, replacing the ineffective generateImages call-count
assertion as the timer-cleanup check. Also add a Vitest test covering repeated
generateImage calls with a long-lived caller AbortSignal, verifying
composed-signal listeners do not accumulate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 494f418f-6908-44a9-b3e1-6570afa7ff0c
📒 Files selected for processing (16)
.changeset/activity-abort-timeout.mdpackages/ai-fal/src/adapters/audio.tspackages/ai-fal/src/adapters/image.tspackages/ai-fal/src/adapters/speech.tspackages/ai-fal/src/adapters/transcription.tspackages/ai-fal/src/adapters/video.tspackages/ai-fal/tests/image-adapter.test.tspackages/ai/src/activities/generateAudio/index.tspackages/ai/src/activities/generateImage/index.tspackages/ai/src/activities/generateSpeech/index.tspackages/ai/src/activities/generateTranscription/index.tspackages/ai/src/activities/generateVideo/index.tspackages/ai/src/activities/summarize/index.tspackages/ai/src/types.tspackages/ai/src/utilities/activity-abort.tspackages/ai/tests/activity-abort.test.ts
Comment on lines
+167
to
+175
| expect(mockSubscribe).toHaveBeenCalledTimes(1) | ||
| const [, options] = mockSubscribe.mock.calls[0]! | ||
| expect(options.abortSignal).toBeInstanceOf(AbortSignal) | ||
| // Must be request-scoped options, not a side effect of fal.config(). | ||
| expect(mockConfig).toHaveBeenCalled() | ||
| for (const call of mockConfig.mock.calls) { | ||
| expect(call[0]).not.toHaveProperty('abortSignal') | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Verify caller cancellation propagation.
The assertion only verifies the signal type. A new unrelated signal would pass this test.
Abort controller and verify that the signal captured from fal.subscribe() becomes aborted with the caller reason.
Proposed test change
const [, options] = mockSubscribe.mock.calls[0]!
expect(options.abortSignal).toBeInstanceOf(AbortSignal)
+ controller.abort('caller cancelled')
+ expect(options.abortSignal.aborted).toBe(true)
+ expect(options.abortSignal.reason).toBe('caller cancelled')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(mockSubscribe).toHaveBeenCalledTimes(1) | |
| const [, options] = mockSubscribe.mock.calls[0]! | |
| expect(options.abortSignal).toBeInstanceOf(AbortSignal) | |
| // Must be request-scoped options, not a side effect of fal.config(). | |
| expect(mockConfig).toHaveBeenCalled() | |
| for (const call of mockConfig.mock.calls) { | |
| expect(call[0]).not.toHaveProperty('abortSignal') | |
| } | |
| }) | |
| expect(mockSubscribe).toHaveBeenCalledTimes(1) | |
| const [, options] = mockSubscribe.mock.calls[0]! | |
| expect(options.abortSignal).toBeInstanceOf(AbortSignal) | |
| controller.abort('caller cancelled') | |
| expect(options.abortSignal.aborted).toBe(true) | |
| expect(options.abortSignal.reason).toBe('caller cancelled') | |
| // Must be request-scoped options, not a side effect of fal.config(). | |
| expect(mockConfig).toHaveBeenCalled() | |
| for (const call of mockConfig.mock.calls) { | |
| expect(call[0]).not.toHaveProperty('abortSignal') | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-fal/tests/image-adapter.test.ts` around lines 167 - 175,
Strengthen the cancellation test around mockSubscribe by capture the abort
signal passed in options, abort the existing caller controller with a specific
reason, and assert that the captured signal is aborted with that reason. Keep
the existing request-scoped options and fal.config() assertions unchanged.
Comment on lines
+236
to
+247
| const rawResult = await raceWithAbort( | ||
| adapter.generateAudio({ | ||
| ...rest, | ||
| model, | ||
| logger, | ||
| ...(abortControls.signal | ||
| ? { abortSignal: abortControls.signal } | ||
| : {}), | ||
| }), | ||
| abortControls.signal, | ||
| ) | ||
| abortControls.clear() |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check that media adapter option interfaces declare abortSignal.
set -euo pipefail
fd -e ts . packages/ai/src/activities --exec rg -n -C4 'abortSignal' {} \
| rg -n 'GenerationOptions|Options|abortSignal'
echo '--- option interface declarations ---'
for name in AudioGenerationOptions TTSOptions TTSGenerationOptions TranscriptionOptions VideoJobCreateOptions ImageGenerationOptions SummarizeOptions; do
echo "== $name =="
ast-grep run --pattern "interface $name { \$\$\$ }" --lang typescript packages/ai/src || true
doneRepository: TanStack/ai
Length of output: 2663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- candidate interface names and declarations ---'
rg -n "interface .*Options|abortSignal\\??:\\s*AbortSignal" packages/ai/src/activities -g '*.ts' | rg "interface|abortSignal" | head -200
echo '--- files mentioning generateAudio / generateSpeech / generateSpeechToText / createVideoJob / generateImage ---'
rg -n "generateAudio|generateSpeech|generateSpeechToText|createVideoJob|generateImage" packages/ai/src/activities -g '*.ts' | head -200
echo '--- audio activity relevant section ---'
cat -n packages/ai/src/activities/generateAudio/index.ts | sed -n '1,130p;220,260p'
echo '--- related activity option exports/imports ---'
for f in packages/ai/src/activities/generateAudio/index.ts packages/ai/src/activities/generateSpeech/index.ts packages/ai/src/activities/generateSpeechToText/index.ts packages/ai/src/activities/createVideoJob/index.ts packages/ai/src/activities/generateImage/index.ts; do
if [ -f "$f" ]; then
echo "==$f=="
cat -n "$f" | sed -n '1,80p;220,260p'
fi
doneRepository: TanStack/ai
Length of output: 27941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package files and scripts ---'
git ls-files 'package.json' 'packages/*/package.json' | sed -n '1,80p'
for f in package.json packages/ai/package.json; do
if [ -f "$f" ]; then
echo "== $f =="
sed -n '1,180p' "$f"
fi
done
echo '--- local TypeScript/tsc availability ---'
if command -v tsc >/dev/null 2>&1; then tsc --version; else echo 'tsc not on path'; fi
if [ -d packages/ai/node_modules/typescript ]; then tsc --version; else echo 'no packages/ai/node_modules/typescript'; fi
echo '--- exact generated option interfaces ---'
for file in \
packages/ai/src/activities/generateAudio/index.ts \
packages/ai/src/activities/generateSpeech/index.ts \
packages/ai/src/activities/generateTranscription/index.ts \
packages/ai/src/activities/generateVideo/index.ts \
packages/ai/src/activities/generateImage/index.ts \
packages/ai/src/activities/summarize/index.ts; do
if [ -f "$file" ]; then
echo "== $file =="
sed -n '50,120p;120,145p;150,180p' "$file"
fi
done
echo '--- adapter method signatures ---'
for file in \
packages/ai/src/activities/generateAudio/adapter.ts \
packages/ai/src/activities/generateSpeech/adapter.ts \
packages/ai/src/activities/generateTranscription/adapter.ts \
packages/ai/src/activities/generateVideo/adapter.ts \
packages/ai/src/activities/generateImage/adapter.ts; do
if [ -f "$file" ]; then
echo "== $file =="
sed -n '65,95p;170,195p' "$file"
fi
done
echo '--- TypeScript behavior probe for omitting unused value ---'
node - 2>&1 <<'JS'
const code = `
interface ProvidedOptions {
prompt: string;
duration?: number;
modelOptions?: object;
model: string;
logger: unknown;
}
async function generateAudio(options: ProvidedOptions) { return 1; }
const rest = { prompt: 'x', duration: 1, model: 'm', logger: {}, abortSignal: new AbortController().signal };
await generateAudio({
...rest,
...(rest.abortSignal ? { abortSignal: rest.abortSignal } : {})
});
`
console.log(code)
JS
echo '--- search adapter option declarations with abortSignal ---'
rg -n "abortSignal\\??:\\s*AbortSignal" packages/ai/src/activities/generate{Audio,Speech,Transcription,Video,Image}/adapter.ts || trueRepository: TanStack/ai
Length of output: 41635
Add abortSignal to the adapter option interfaces.
runGenerateAudio now spreads abortSignal into adapter.generateAudio(...), but generateAudio is typed with AudioGenerationOptions<TProviderOptions> while AudioActivityOptions is the value type. If provider adapters define AudioGenerationOptions, make those option interfaces include abortSignal; otherwise the adapter call drops the timeout/caller signal or fails type checking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/activities/generateAudio/index.ts` around lines 236 - 247,
Update the adapter option interfaces used by generateAudio, specifically
AudioGenerationOptions and any provider-specific extensions, to declare the
optional abortSignal passed by runGenerateAudio. Ensure the
adapter.generateAudio call preserves and accepts this signal without type
errors, while retaining existing provider option fields.
Comment on lines
+290
to
+293
| const abortControls = createActivityAbortControls({ | ||
| timeout, | ||
| abortSignal: callerAbortSignal, | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Clear abort controls when pre-request middleware fails.
If runGenerationStart() rejects at Line 307, execution does not enter the later try/catch. The timeout timer remains active until expiry.
Put the middleware-start phase inside cleanup coverage, or clear abortControls before rethrowing its error. This violates the stated timer-cleanup objective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/activities/generateImage/index.ts` around lines 290 - 293,
Ensure the middleware-start phase invoking runGenerationStart is covered by
cleanup for abortControls, so a rejection clears the timeout before propagating
the error. Update the surrounding flow in the image-generation activity without
changing normal successful execution or later error handling.
Comment on lines
+102
to
+113
| /** | ||
| * Maximum duration of this activity invocation in milliseconds. | ||
| * No SDK-wide default — choose a value suitable for the provider and job. | ||
| * Composed with {@link abortSignal}; the first abort wins. | ||
| */ | ||
| timeout?: number | ||
| /** | ||
| * Caller cancellation signal (request disconnects, job/runtime cancellation). | ||
| * Composed with {@link timeout} into an effective signal forwarded to the | ||
| * adapter. Request-specific — not stored on global provider client config. | ||
| */ | ||
| abortSignal?: AbortSignal |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether runStreamingSummarize consumes timeout/abortSignal.
set -euo pipefail
f=packages/ai/src/activities/summarize/index.ts
ast-grep outline "$f" --items all
echo '--- runStreamingSummarize body ---'
ast-grep run --pattern 'async function* runStreamingSummarize($$$) { $$$ }' --lang typescript "$f" || \
ast-grep run --pattern 'function runStreamingSummarize($$$) { $$$ }' --lang typescript "$f"
echo '--- abort references in file ---'
rg -n 'abortSignal|abortControls|timeout|raceWithAbort' "$f"Repository: TanStack/ai
Length of output: 1422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=packages/ai/src/activities/summarize/index.ts
echo '--- file line count ---'
wc -l "$f"
echo '--- summarize/export section ---'
sed -n '200,280p' "$f"
echo '--- runSummarize section ---'
sed -n '234,345p' "$f"
echo '--- all exported/defined sums ---'
rg -n 'export function summarize|function runStreamingSummarize|createSummarizeOptions|SummarizeActivityOptions|stream \?|stream:' "$f"
echo '--- broader summing/search ---'
rg -n -C 3 'runStreamingSummarize|runSummarize|timeout|abortSignal' "$f"Repository: TanStack/ai
Length of output: 8960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=packages/ai/src/activities/summarize/index.ts
echo '--- runStreamingSummarize body ---'
sed -n '368,460p' "$f" | cat -n
echo '--- imports and helper definitions ---'
sed -n '1,45p' "$f" | cat -n
rg -n 'createActivityAbortControls|raceWithAbort|isActivityAbortError|abortReasonMessage|runGenerationAbort|runGenerationError|runGenerationStart|runGenerationFinish|runGenerationUsage' packages/ai/src -g '*.ts'Repository: TanStack/ai
Length of output: 19074
Wire timeout and abort controls into runStreamingSummarize.
summarize() dispatches stream: true to runStreamingSummarize, but only runSummarize creates abort controls. The native streaming path calls adapter.summarizeStream(summarizeOptions) without passing an abort signal, and the fallback runSummarize({ ...options, stream: false, ... }) loses the caller’s run identity and abort controls before calling the non-streaming adapter. Apply the same controls as runSummarize in both streaming branches, or remove these options from SummarizeActivityOptions for streaming.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/activities/summarize/index.ts` around lines 102 - 113, Update
runStreamingSummarize to compose and pass the timeout and abortSignal controls
to adapter.summarizeStream, matching runSummarize. In its non-streaming
fallback, preserve the caller’s run identity and forward the effective abort
controls when invoking runSummarize, while retaining the existing streaming
behavior.
Comment on lines
+25
to
+40
| export function combineAbortSignals( | ||
| a: AbortSignal | undefined, | ||
| b: AbortSignal | undefined, | ||
| ): AbortSignal | undefined { | ||
| if (!a) return b | ||
| if (!b) return a | ||
| if (a.aborted) return a | ||
| if (b.aborted) return b | ||
| const controller = new AbortController() | ||
| const onAbort = (source: AbortSignal) => () => { | ||
| controller.abort(source.reason) | ||
| } | ||
| a.addEventListener('abort', onAbort(a), { once: true }) | ||
| b.addEventListener('abort', onAbort(b), { once: true }) | ||
| return controller.signal | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Remove the abort listeners when the activity settles.
combineAbortSignals registers an abort listener on the caller signal and never removes it. The listener closes over controller, so the combined AbortController stays reachable from the caller signal.
Callers often own a signal whose lifetime exceeds one activity call, for example a long-lived job or runtime signal reused across invocations. Each invocation then adds one permanent listener to that signal, so retained memory grows with the number of calls. ActivityAbortControls.clear() only clears the timer, so it does not release these listeners.
Return a disposer from combineAbortSignals and call it from clear().
♻️ Proposed fix: dispose composed listeners in clear()
-export function combineAbortSignals(
- a: AbortSignal | undefined,
- b: AbortSignal | undefined,
-): AbortSignal | undefined {
- if (!a) return b
- if (!b) return a
- if (a.aborted) return a
- if (b.aborted) return b
- const controller = new AbortController()
- const onAbort = (source: AbortSignal) => () => {
- controller.abort(source.reason)
- }
- a.addEventListener('abort', onAbort(a), { once: true })
- b.addEventListener('abort', onAbort(b), { once: true })
- return controller.signal
-}
+export function combineAbortSignals(
+ a: AbortSignal | undefined,
+ b: AbortSignal | undefined,
+): { signal: AbortSignal | undefined; dispose: () => void } {
+ const noop = () => undefined
+ if (!a) return { signal: b, dispose: noop }
+ if (!b) return { signal: a, dispose: noop }
+ if (a.aborted) return { signal: a, dispose: noop }
+ if (b.aborted) return { signal: b, dispose: noop }
+ const controller = new AbortController()
+ const onA = () => controller.abort(a.reason)
+ const onB = () => controller.abort(b.reason)
+ a.addEventListener('abort', onA, { once: true })
+ b.addEventListener('abort', onB, { once: true })
+ return {
+ signal: controller.signal,
+ dispose: () => {
+ a.removeEventListener('abort', onA)
+ b.removeEventListener('abort', onB)
+ },
+ }
+}Then wire it into the controls:
- const signal = combineAbortSignals(options.abortSignal, timeoutSignal)
+ const composed = combineAbortSignals(options.abortSignal, timeoutSignal)
return {
- signal,
+ signal: composed.signal,
clear: () => {
if (timeoutId !== undefined) {
clearTimeout(timeoutId)
timeoutId = undefined
}
+ composed.dispose()
},
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/utilities/activity-abort.ts` around lines 25 - 40, Update
combineAbortSignals to return both the combined signal and a disposer that
removes its abort listeners, while preserving existing undefined, aborted, and
propagation behavior. Store and invoke that disposer from
ActivityAbortControls.clear() alongside timer cleanup, ensuring repeated or
settled activities release listeners.
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
AlemTuzlak
approved these changes
Aug 7, 2026
AlemTuzlak
deleted the
981-featai-add-abort-signals-and-timeouts-to-media-generation-activities
branch
August 7, 2026 09:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
timeoutandabortSignalto media activities (generateImage,generateAudio,generateVideo,generateSpeech,generateTranscription,summarize).onAbort(notonError).@tanstack/ai-falforwards the request-specific signal tofal.subscribe()/fal.queue.submit()— never via globalfal.config().Closes #981
Test plan
onAbortonce notonErrorfal.subscribe()receives request-specificabortSignalpnpm test:typesfor@tanstack/aiand@tanstack/ai-falUsage
Summary by CodeRabbit