fix(react-hooks): prevent onComplete from firing prematurely when stream disconnects#2929
fix(react-hooks): prevent onComplete from firing prematurely when stream disconnects#2929
Conversation
…eam disconnects The onComplete callback in useRealtimeRun and useRealtimeRunWithStreams was firing whenever the SSE stream ended, regardless of whether the run had actually completed. This caused issues in self-hosted environments where reverse proxies (like Traefik) may close idle connections. The fix changes the condition from checking `run` to checking `run?.finishedAt`, ensuring onComplete only fires when the run has actually reached a terminal state. Fixes #2856 Co-authored-by: nicktrn <nicktrn@users.noreply.github.com>
🦋 Changeset detectedLatest commit: a7d7535 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change updates the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/react-hooks/src/hooks/useRealtime.ts (2)
151-159: Clarify theonCompletecontract now that error/stop won’t trigger it.The JSDoc says
onCompletefires on errors or when the subscription stops, but the newfinishedAtgate prevents those cases. Please update the docs (or add a separate error/stop callback) so consumers don’t get misled.📝 Suggested doc update
- /** - * Callback this is called when the run completes, an error occurs, or the subscription is stopped. - * - * `@param` {RealtimeRun<TTask>} run - The run object - * `@param` {Error} [err] - The error that occurred - */ + /** + * Callback called when the run reaches a terminal state (finishedAt is set). + * Subscription errors/stops are surfaced via `error`. + * + * `@param` {RealtimeRun<TTask>} run - The run object + * `@param` {Error} [err] - The error that occurred + */
317-325: SameonCompletecontract mismatch in the stream variant.Please keep the docs consistent with the finishedAt-only behavior (or expose a dedicated error/stop callback) to avoid breaking consumer expectations.
📝 Suggested doc update
- /** - * Callback this is called when the run completes, an error occurs, or the subscription is stopped. - * - * `@param` {RealtimeRun<TTask>} run - The run object - * `@param` {Error} [err] - The error that occurred - */ + /** + * Callback called when the run reaches a terminal state (finishedAt is set). + * Subscription errors/stops are surfaced via `error`. + * + * `@param` {RealtimeRun<TTask>} run - The run object + * `@param` {Error} [err] - The error that occurred + */
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-hooks/src/hooks/useRealtime.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
packages/react-hooks/src/hooks/useRealtime.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
packages/react-hooks/src/hooks/useRealtime.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/react-hooks/src/hooks/useRealtime.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
packages/react-hooks/src/hooks/useRealtime.ts
{packages,integrations}/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Add a changeset when modifying any public package in
packages/*orintegrations/*usingpnpm run changeset:add
Files:
packages/react-hooks/src/hooks/useRealtime.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `.withStreams()` to subscribe to realtime streams from task metadata in addition to run changes
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `.withStreams()` to subscribe to realtime streams from task metadata in addition to run changes
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `trigger.dev/react-hooks` package for realtime subscriptions in React components
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@nicktrn this PR only fixes the onComplete function. However this would still not return a Server-sent Event since you haven't set |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
hey @bbreunig I've commented on the issue with some additional details re the most likely cause: react strict mode. I thought about adding a more specific fix here but this seems cleaner and covers more potential edge cases. |
…eam disconnects (triggerdotdev#2929) ## Summary Fixes triggerdotdev#2856 - The `onComplete` callback in `useRealtimeRun` was firing prematurely ## Root Cause The callback was triggered when the long-poll stream ended, regardless of whether the run had actually completed. Reverse proxies often close idle connections, causing the stream to end prematurely. In this case it was caused by fetch abort due to React strict mode. ## Fix Changed the condition from checking if `run` exists to checking if `run?.finishedAt` exists, ensuring `onComplete` only fires when the run has reached a terminal state. --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: nicktrn <nicktrn@users.noreply.github.com>
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @trigger.dev/sdk@4.4.0 ### Minor Changes - Added `query.execute()` which lets you query your Trigger.dev data using TRQL (Trigger Query Language) and returns results as typed JSON rows or CSV. It supports configurable scope (environment, project, or organization), time filtering via `period` or `from`/`to` ranges, and a `format` option for JSON or CSV output. ([#3060](#3060)) ```typescript import { query } from "@trigger.dev/sdk"; import type { QueryTable } from "@trigger.dev/sdk"; // Basic untyped query const result = await query.execute("SELECT run_id, status FROM runs LIMIT 10"); // Type-safe query using QueryTable to pick specific columns const typedResult = await query.execute<QueryTable<"runs", "run_id" | "status" | "triggered_at">>( "SELECT run_id, status, triggered_at FROM runs LIMIT 10" ); typedResult.results.forEach((row) => { console.log(row.run_id, row.status); // Fully typed }); // Aggregation query with inline types const stats = await query.execute<{ status: string; count: number }>( "SELECT status, COUNT(*) as count FROM runs GROUP BY status", { scope: "project", period: "30d" } ); // CSV export const csv = await query.execute("SELECT run_id, status FROM runs", { format: "csv", period: "7d", }); console.log(csv.results); // Raw CSV string ``` ### Patch Changes - Add `maxDelay` option to debounce feature. This allows setting a maximum time limit for how long a debounced run can be delayed, ensuring execution happens within a specified window even with continuous triggers. ([#2984](#2984)) ```typescript await myTask.trigger(payload, { debounce: { key: "my-key", delay: "5s", maxDelay: "30m", // Execute within 30 minutes regardless of continuous triggers }, }); ``` - Aligned the SDK's `getRunIdForOptions` logic with the Core package to handle semantic targets (`root`, `parent`) in root tasks. ([#2874](#2874)) - Export `AnyOnStartAttemptHookFunction` type to allow defining `onStartAttempt` hooks for individual tasks. ([#2966](#2966)) - Fixed a minor issue in the deployment command on distinguishing between local builds for the cloud vs local builds for self-hosting setups. ([#3070](#3070)) - Updated dependencies: - `@trigger.dev/core@4.4.0` ## @trigger.dev/build@4.4.0 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.4.0` ## trigger.dev@4.4.0 ### Patch Changes - Fix runner getting stuck indefinitely when `execute()` is called on a dead child process. ([#2978](#2978)) - Add optional `timeoutInSeconds` parameter to the `wait_for_run_to_complete` MCP tool. Defaults to 60 seconds. If the run doesn't complete within the timeout, the current state of the run is returned instead of waiting indefinitely. ([#3035](#3035)) - Fixed a minor issue in the deployment command on distinguishing between local builds for the cloud vs local builds for self-hosting setups. ([#3070](#3070)) - Updated dependencies: - `@trigger.dev/core@4.4.0` - `@trigger.dev/build@4.4.0` - `@trigger.dev/schema-to-json@4.4.0` ## @trigger.dev/core@4.4.0 ### Patch Changes - Add `maxDelay` option to debounce feature. This allows setting a maximum time limit for how long a debounced run can be delayed, ensuring execution happens within a specified window even with continuous triggers. ([#2984](#2984)) ```typescript await myTask.trigger(payload, { debounce: { key: "my-key", delay: "5s", maxDelay: "30m", // Execute within 30 minutes regardless of continuous triggers }, }); ``` - Fixed a minor issue in the deployment command on distinguishing between local builds for the cloud vs local builds for self-hosting setups. ([#3070](#3070)) - fix: vendor superjson to fix ESM/CJS compatibility ([#2949](#2949)) Bundle superjson during build to avoid `ERR_REQUIRE_ESM` errors on Node.js versions that don't support `require(ESM)` by default (< 22.12.0) and AWS Lambda which intentionally disables it. - Add Vercel integration support to API schemas: `commitSHA` and `integrationDeployments` on deployment responses, and `source` field for environment variable imports. ([#2994](#2994)) ## @trigger.dev/python@4.4.0 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.4.0` - `@trigger.dev/sdk@4.4.0` - `@trigger.dev/build@4.4.0` ## @trigger.dev/react-hooks@4.4.0 ### Patch Changes - Fix `onComplete` callback firing prematurely when the realtime stream disconnects before the run finishes. ([#2929](#2929)) - Updated dependencies: - `@trigger.dev/core@4.4.0` ## @trigger.dev/redis-worker@4.4.0 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.4.0` ## @trigger.dev/rsc@4.4.0 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.4.0` ## @trigger.dev/schema-to-json@4.4.0 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.4.0` --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
3 participants
Summary
Fixes #2856 - The
onCompletecallback inuseRealtimeRunwas firing prematurely in self-hosted environments with reverse proxies like Traefik.Root Cause
The callback was triggered when the long-poll stream ended, regardless of whether the run had actually completed. Reverse proxies often close idle connections, causing the stream to end prematurely. In this case it was caused by fetch abort due to React strict mode.
Fix
Changed the condition from checking if
runexists to checking ifrun?.finishedAtexists, ensuringonCompleteonly fires when the run has reached a terminal state.