-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: catch all socket errors #1752
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1752 +/- ##
==========================================
- Coverage 79.47% 79.45% -0.02%
==========================================
Files 23 23
Lines 1403 1402 -1
Branches 329 329
==========================================
- Hits 1115 1114 -1
Misses 209 209
Partials 79 79 ☔ View full report in Codecov by Sentry. |
Apart from the comment I added I didn't found any reason why we are filtering errors to emit. The errors that could happen on connect are a lot and we will always miss some, this PR just ignores all errors that comes from browser Websocket (like it happened before as Tests are working but I would like to know if someone has a reason why those errors should be filtered. Note that actually we always setup a client.on('error') listener so that would not throw unhandled rejections: MQTT.js/src/lib/connect/index.ts Lines 174 to 176 in 6d33a05
|
@mcollina Kindly ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Backport to version that supports NodeJS 14
Fixes #1751