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. |
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.