-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
fix: prevent enqueuing into closed ReadableStream #1781
base: main
Are you sure you want to change the base?
fix: prevent enqueuing into closed ReadableStream #1781
Conversation
|
WalkthroughThe changes add a private boolean property Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as ReadableShapeStream
participant U as Unsubscribe
participant EH as ErrorHandler
C->>S: stop()
S->>S: set #isStreamClosed = true
S->>U: call unsubscribe function
C->>S: cancel()
S->>S: set #isStreamClosed = true
S->>U: call unsubscribe function
S->>S: transform(message)
alt Stream Open
S->>S: Process message
else Stream Closed
S->>S: Skip processing
end
S->>S: Error in transform
alt Error Caught
S->>EH: invoke error handler
S->>U: call unsubscribe function
end
Poem
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
Scope: all 2 workspace projects 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (8)
✨ 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. 🪧 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 (
|
🧩 Analysis chainI'll review this PR for you. Let me gather more information about the changes first. 🏁 Script executed:
Length of output: 209 I need to first check out the PR to properly review the changes. Let me try again. 🏁 Script executed:
Length of output: 272 🏁 Script executed:
Length of output: 6943 @CodeMan62: Yes, this PR looks excellent. The changes properly address the issue of enqueueing data into closed ReadableStreams by:
These changes will effectively prevent the "Cannot enqueue a chunk into a closed readable stream" error that was occurring after 5-15 minutes of operation. ✅ Actions performedComments resolved. |
@ericallam can you check this PR and tell me if there is any problem in it |
Closes #1751
✅ Checklist
Testing
pnpm run test --filter @trigger.dev/core
Changelog
#isStreamClosed
flag to track stream statecancel
handlerstop()
methodScreenshots
N/A - This is a backend fix for stream handling, no UI changes.
💯
Summary by CodeRabbit
Bug Fixes
Refactor