fix(transport-webrtc): abort stream when datachannel send throws#3469
Open
tabcat wants to merge 5 commits intolibp2p:mainfrom
Open
fix(transport-webrtc): abort stream when datachannel send throws#3469tabcat wants to merge 5 commits intolibp2p:mainfrom
tabcat wants to merge 5 commits intolibp2p:mainfrom
Conversation
…gence Triggers the JS-vs-native state divergence in node-datachannel polyfill by closing the peer connection synchronously before send(), so the cached readyState is still 'open' while the native channel is closed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
channel.send can throw synchronously when the native libdatachannel state diverges from the polyfill's cached readyState. Wrap the call and abort the stream so the error surfaces as a transport failure instead of going uncaught. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The JS-vs-native state divergence is specific to the node-datachannel polyfill; native browser WebRTC does not reproduce it. Also add libdatachannel to the spell-check dictionary for the new fix comment. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Description
The
node-datachannelpolyfill cachesRTCDataChannel.readyStateand updates it viasetImmediatewhen the native channel closes. This creates a window where the cachedreadyStatestill reports'open'while the underlying native channel is already closed. In that window,channel.send()passes thereadyState !== 'open'guard inWebRTCStream._sendMessageand calls into the native binding, which throws"DataChannel is closed"synchronously.The throw was uncaught because
_sendMessagedid not wrap thechannel.sendcalls, surfacing as an unhandled error in CI (observed in https://github.com/libp2p/js-libp2p/actions/runs/24605305629/job/71950609115?pr=3459).This PR:
send()aborts the stream cleanly.channel.sendin a try/catch and callsthis.abort(err)on synchronous throw, so the error surfaces as a transport failure on the stream rather than an uncaught exception.Related #3459.
Notes & open questions
The first commit (test only) is expected to fail CI to demonstrate the error path. The fix commit will be pushed next.
Change checklist