Repository navigation
Conversation
Registered text/byte stream handlers were invoked without awaiting or catching their result. An async handler that iterated its reader without a try/catch therefore produced an unhandled promise rejection whenever the stream ended abnormally (sender or local room disconnecting mid stream), and a synchronous throw escaped out of packet handling. Invoke handlers through a wrapper that catches both: AbnormalEnd DataStreamErrors are logged at debug, anything else at error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d4f2950 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
size-limit report 📦
|
…ects Rather than catching every handler error, re-throw anything that happens while the room is connected (or reconnecting) so it still surfaces as an unhandled rejection, e.g. a lost chunk or a remote participant leaving mid stream. Only errors that land after the room has disconnected are logged as a warning, since the session is over and the failure is out of the application's hands. A dedicated flag, set by clearControllers() and cleared by setConnected(true), tracks the disconnect so a reconnect isn't mistaken for one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
I'm not convinced this is a good idea, but I think this could be one way to address a limited subset of the issue from livekit/components-js#1426.
Summary
IncomingDataStreamManagercalls registered text/byte stream handlers without awaiting them. Most handlers areasyncfunctions that read the stream with notry/catch, so any error they don't catch becomes an unhandled promise rejection.That is mostly the right default: a lost chunk (
Incomplete), a sender dropping mid stream, or a bug in the handler should fail loudly so apps notice. The exception is the local room disconnecting. Since #2125,clearControllers()errors every in-flight stream withRoom disconnected in the middle of receiving data, so hanging up while (for example) an agent transcription is still streaming produces an unhandled rejection that the app can't do anything about. The session is over at that point.Downstream report: livekit/components-js#1426.
Fix
Handlers are now invoked through
invokeStreamHandler, which looks at a handler's rejection:Whether the room has disconnected is tracked by a new
hasDisconnectedflag.clearControllers()sets it (it only runs fromhandleDisconnect), andsetConnected(true)clears it. The existingisConnectedflag isn't used because it's alsofalsewhile reconnecting, and errors during a reconnect the room recovers from should stay loud.The check happens when the handler's error arrives, not when the stream started. Any handler error that lands after the disconnect is downgraded, even one unrelated to the disconnect (an app bug, or a lost chunk just before hangup). That's intended: once the room is closed, failures are out of the app's hands, and they're still logged.
TextStreamHandler/ByteStreamHandlerare widened to returnvoid | Promise<void>to document that async handlers are supported. This doesn't break existing handlers.Unchanged (still unhandled rejections)
AbnormalEndfromvalidateParticipantHasNoActiveDataStreams)Incomplete)Known edge case
If an app disconnects and reconnects the same
Roombefore a slow handler from the previous session fails, that error arrives aftersetConnected(true)has cleared the flag and stays loud. That seemed rare enough not to be worth tracking per session.Tests
New
Stream handler errorsblock inIncomingDataStreamManager.test.ts:Incomplete)The first two fail on
main, and the other four pass onmain(they check that existing behaviour is kept). Full suite,tsc, eslint and prettier pass.🤖 Generated with Claude Code