Handle errors thrown by requests in Realtime react hooks #1599
Conversation
🦋 Changeset detectedLatest commit: f6cb399 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
|
Warning Rate limit exceeded@ericallam has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughThis pull request introduces comprehensive error handling and improved state management for real-time hooks in the Trigger.dev React library. The changes focus on enhancing the Changes
Sequence DiagramsequenceDiagram
participant User as User Code
participant Hook as useRealtimeRun
participant ApiClient as API Client
participant ErrorHandler as Error Callback
User->>Hook: Trigger realtime run
Hook->>ApiClient: Subscribe to run
ApiClient-->>Hook: Stream run data
alt Error occurs
ApiClient->>ErrorHandler: Invoke onFetchError
ErrorHandler->>User: Handle error
end
Hook->>Hook: Update run completion state
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ericallam
force-pushed
the
ea-branch-5
branch
from
6e58a21 to
f6cb399
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
references/nextjs-realtime/src/app/actions.ts (1)
Line range hint
31-52: Maintain consistency in token generation approachThe
batchTriggerExampleTaskfunction still uses the old token generation approach (handle.publicAccessToken), whiletriggerExampleTaskhas been updated to useauth.createPublicToken. This inconsistency could lead to different security models between single and batch operations.Apply the same token generation pattern to maintain consistency:
console.log("Batch trigger example task"); const handle = await tasks.batchTrigger<typeof exampleTask>("example", [ { payload: { id: randomUUID() } }, // ... other entries ... ]); - console.log("Setting the run JWT in a cookie", handle.publicAccessToken); + const publicToken = await auth.createPublicToken({ + scopes: { + read: { + runs: handle.runIds, // Assuming handle provides runIds for batch operations + }, + }, + expirationTime: "2s", + }); + + console.log("Setting the run JWT in a cookie", publicToken); // Set JWT in a secure, HTTP-only cookie - cookies().set("run_token", handle.publicAccessToken); + cookies().set("run_token", publicToken, { + httpOnly: true, + secure: true, + sameSite: "strict" + });
🧹 Nitpick comments (1)
.changeset/many-panthers-relax.md (1)
1-5: Expand the changeset description to cover all changes.While the current description mentions fixing the
onCompletehook timing, it doesn't reflect the significant error handling improvements that are part of this patch. Consider expanding the description to include:
- Addition of
onErrorcallbacks for improved error handling- Updates to
processRealtimeBatch,processRealtimeRunsWithTag, etc.- Integration with
apiClienterror handling capabilitiesThis will help users better understand the full scope of changes in this patch.
Here's a suggested expanded description:
--- "@trigger.dev/react-hooks": patch --- -Make sure useRealtimeRun onComplete hook fires at the correct time +Improve real-time hooks reliability: +- Fix useRealtimeRun onComplete hook timing +- Add comprehensive error handling with onError callbacks +- Enhance error management in real-time batch processing +- Integrate with apiClient error handling capabilities
🛑 Comments failed to post (2)
references/nextjs-realtime/src/app/actions.ts (2)
26-26: 🛠️ Refactor suggestion
Enhance cookie security attributes
While using HTTP-only cookies is good, consider adding additional security attributes:
- cookies().set("run_token", publicToken); + cookies().set("run_token", publicToken, { + httpOnly: true, + secure: true, + sameSite: "strict" + });📝 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.cookies().set("run_token", publicToken, { httpOnly: true, secure: true, sameSite: "strict" });
14-21: 💡 Codebase verification
Increase token expiration time to 30 seconds
The current 2-second expiration is too aggressive for real-time subscriptions. A 30-second window provides:
- Sufficient time for connection setup (DNS, TCP, TLS, WebSocket/SSE)
- Better tolerance for network latency and clock skew
- Still maintains security with a reasonably short lifetime
🔗 Analysis chain
Consider increasing the token expiration time
The 2-second expiration time might be too short for slower network conditions or when the client and server have clock differences. Consider increasing it to provide a better buffer while maintaining security.
To help determine an appropriate expiration time, we can analyze the typical request patterns:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for timing patterns in network requests rg -A 5 "useRealtimeRun|subscribeToRun" --type tsLength of output: 20071
@trigger.dev/build
@trigger.dev/react-hooks
@trigger.dev/rsc
@trigger.dev/sdk
@trigger.dev/core
trigger.dev
commit: |
Summary by CodeRabbit
Bug Fixes
useRealtimeRunhook'sonCompletecallbackNew Features
Chores
@trigger.dev/react-hooksand@trigger.dev/core