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.

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

Improve websocket error handling and fix #315 #320

Merged
merged 14 commits into from
Dec 18, 2017
Merged

Improve websocket error handling and fix #315 #320

merged 14 commits into from
Dec 18, 2017

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Dec 17, 2017

Logs errors accuring while sending data instead of throwing them as unhandled errors.
Also added an ignore to ECONNRESET, because this gets triggered every time html reload is triggered.
Closes #315

TODO:

@FalkoJoseph
Copy link

@DeMoorJasper Does this fix the issue where Parcel crashes on refreshing the page? (using parcel-plugin-vue)

@DeMoorJasper
Copy link
Member Author

@FalkoJoseph Yup it does

@FalkoJoseph
Copy link

@DeMoorJasper Nice, can't wait! Also cool to see a Howest student contributing to this project 👍

@DeMoorJasper
Copy link
Member Author

@devongovett could we get this PR approved and released as a bugfix, lot's of people are being affected with clean install #315

return;
}
// TODO: Use logger to print errors
console.log(prettyError(err));
Copy link
Member

Choose a reason for hiding this comment

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

have you seen any other errors actually happen? should we just ignore them?

Copy link
Member Author

@DeMoorJasper DeMoorJasper Dec 18, 2017

Choose a reason for hiding this comment

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

This was just for future error handling.
Normally ECONNRESET is the only error that accures due to chrome not properly closing the connection

Copy link

Choose a reason for hiding this comment

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

Other errors might be parser errors, like an invalid frame, an inflate error if you use permessage-deflate, a frame with a payload bigger than the maxPayload option or other net.Socket errors like write EPIPE, write ECONNABORTED, etc.

@devongovett devongovett merged commit 9f27e15 into parcel-bundler:master Dec 18, 2017
@devongovett
Copy link
Member

Published in v1.2.1.

if (this.unresolvedError) {
ws.send(JSON.stringify(this.unresolvedError));
}
});

this.wss.on('error', this.handleSocketError);
Copy link

Choose a reason for hiding this comment

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

This is probably not needed as it adds the listener after the server is bound. errors are forwarded from the internal http.Server and most of them (all?) are emitted when listening.

@DeMoorJasper DeMoorJasper deleted the feature/fix-websocket-error branch January 1, 2018 09:43
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* run prettier

* add proper error handling to sending data

* replace hacky bugfix with gracefull connection closing on client side

* improve error handling to catch all errors not just when sending

* fix tests

* code cleanup
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* run prettier

* add proper error handling to sending data

* replace hacky bugfix with gracefull connection closing on client side

* improve error handling to catch all errors not just when sending

* fix tests

* code cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants