Skip to content

fix(webtransport-websys): handle exceptions in WebTransport::close#5390

Merged
mergify[bot] merged 7 commits intolibp2p:masterfrom
zvolin:fix/handle-webtransport-close-exceptions
May 17, 2024
Merged

fix(webtransport-websys): handle exceptions in WebTransport::close#5390
mergify[bot] merged 7 commits intolibp2p:masterfrom
zvolin:fix/handle-webtransport-close-exceptions

Conversation

@zvolin
Copy link
Contributor

@zvolin zvolin commented May 15, 2024

Description

Calls to close can result in exceptions being thrown eg. when the connection is not yet fully initialized (in connecting state). Currently webtransport-websys doesn't catch those resulting in errors like:

Uncaught (in promise)
WebTransportError { source: "session", streamErrorCode: null, name: "WebTransportError", message: "remote WebTransport close", code: 0, result: 0, filename: "", lineNumber: 0, columnNumber: 0, data: null }

Those cannot be caught even by wrapping .close() in the try { ... } catch () {...} blocks. It's rather poorly documented, indirectly with just an example here, but the way to catch those is through the promise created by .closed.

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@zvolin zvolin force-pushed the fix/handle-webtransport-close-exceptions branch from 885050a to ae56647 Compare May 15, 2024 18:13
jxs
jxs previously approved these changes May 17, 2024
@jxs jxs added the send-it label May 17, 2024
@jxs
Copy link
Member

jxs commented May 17, 2024

Thanks for the fix! And thanks @oblique for the review ❤️

@mergify mergify bot dismissed jxs’s stale review May 17, 2024 00:49

Approvals have been dismissed because the PR was updated after the send-it label was applied.

mergify bot pushed a commit that referenced this pull request May 17, 2024
libc 0.2.154 is version is not published in crates.io and it breaks the build. The CI of #5390 fails because of this.

Pull-Request: #5395.
@mergify mergify bot merged commit 80f06d8 into libp2p:master May 17, 2024
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
libc 0.2.154 is version is not published in crates.io and it breaks the build. The CI of libp2p#5390 fails because of this.

Pull-Request: libp2p#5395.
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
Calls to `close` can result in exceptions being thrown eg. when the connection is not yet fully initialized (in `connecting` state). Currently webtransport-websys doesn't catch those resulting in errors like:
```
Uncaught (in promise)
WebTransportError { source: "session", streamErrorCode: null, name: "WebTransportError", message: "remote WebTransport close", code: 0, result: 0, filename: "", lineNumber: 0, columnNumber: 0, data: null }
```

Those cannot be caught even by wrapping `.close()` in the `try { ... } catch () {...}` blocks. It's rather poorly documented, indirectly with just an example [here](https://developer.mozilla.org/en-US/docs/Web/API/WebTransport/closed), but the way to catch those is through the promise created by `.closed`.

Pull-Request: libp2p#5390.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants