Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: when using AbortController, errors on resulting stream must be caught #539

Merged
merged 5 commits into from
Nov 29, 2024
Merged

fix: when using AbortController, errors on resulting stream must be caught #539

merged 5 commits into from
Nov 29, 2024

Conversation

uriva
Copy link
Contributor

@uriva uriva commented Nov 9, 2024

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix
[ ] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

Bugfix, according to nodejs/undici#3353 (comment)

Which issue (if any) does this pull request address?

webtorrent/webtorrent#2874

Is there anything you'd like reviewers to focus on?

@uriva
Copy link
Contributor Author

uriva commented Nov 11, 2024 •
edited
Loading

What I saw is that when I add this line and remove the fetch polyfill the problem disappears.

According to what you said the polyfill resolves to native fetch on newer node versions, so this means its removal is not essential.

However when I kept the fetch polyfill, some tests failed, I assume due to some older versions not returning a response with body which is a stream, so I added a check to validate the output for different versions.

According to my logic this should solve the bug and not regress anything, but please let me know if I missed something.

SilentBot1
SilentBot1 approved these changes Nov 29, 2024
View reviewed changes
Copy link
Member

@SilentBot1 SilentBot1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SilentBot1 SilentBot1 merged commit e45516d into webtorrent:master Nov 29, 2024
3 checks passed
@ThaUnknown ThaUnknown mentioned this pull request Nov 29, 2024
Closed
@SilentBot1 SilentBot1 changed the title Bugfix - when using AbortController, errors on resulting stream must be caught fix: when using AbortController, errors on resulting stream must be caught Dec 28, 2024
SilentBot1 added a commit that referenced this pull request Dec 28, 2024
@SilentBot1
feat: release #539 and #535
518299f
@SilentBot1 SilentBot1 mentioned this pull request Dec 28, 2024
Merged
SilentBot1 added a commit that referenced this pull request Dec 28, 2024
@SilentBot1
feat: release #539 and #535 (#544)
e7de90c
webtorrent-bot pushed a commit that referenced this pull request Dec 28, 2024
@semantic-release-bot
chore(release): 11.2.0
9809195 
# [11.2.0](v11.1.2...v11.2.0) (2024-12-28)

### Features

* release [#539](#539) and [#535](#535) ([#544](#544)) ([e7de90c](e7de90c))
@webtorrent-bot
Copy link

🎉 This PR is included in version 11.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SilentBot1 SilentBot1 SilentBot1 approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@uriva @webtorrent-bot @SilentBot1