Skip to content

Commit

Permalink
Improve websocket error handling and fix #315 (#320)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Jasper De Moor authored and devongovett committed Dec 18, 2017
1 parent c008bb0 commit 9f27e15
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 29 deletions.
25 changes: 18 additions & 7 deletions .github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Search open/closed issues before submitting since someone might have asked the s
<!--- Provide a general summary of the issue in the title above -->

### 🎛 Configuration (.babelrc, package.json, cli command)

<!--- If describing a bug, tell us what your babel configuration looks like -->

```js
Expand All @@ -18,31 +19,41 @@ Search open/closed issues before submitting since someone might have asked the s
```

### 🤔 Expected Behavior

<!--- If you're describing a bug, tell us what should happen -->

<!--- If you're suggesting a change/improvement, tell us how it should work -->

### 😯 Current Behavior

<!--- If describing a bug, tell us what happens instead of the expected behavior -->

<!--- If you are seeing an error, please include the full error message and stack trace -->

<!--- If suggesting a change/improvement, explain the difference from current behavior -->

### 💁 Possible Solution

<!--- Not obligatory, but suggest a fix/reason for the bug, -->

<!--- or ideas how to implement the addition or change -->

### 🔦 Context

<!--- How has this issue affected you? What are you trying to accomplish? -->

<!--- Providing context helps us come up with a solution that is most useful in the real world -->

### 🌍 Your Environment

<!--- Include as many relevant details about the environment you experienced the bug in -->

| Software | Version(s)
| ---------------- | ----------
| Parcel |
| Node |
| npm/Yarn |
| Operating System |
| Software | Version(s) |
| ---------------- | ---------- |
| Parcel |
| Node |
| npm/Yarn |
| Operating System |

<!-- Love parcel? Please consider supporting our collective:
👉 https://opencollective.com/parcel/donate -->
👉 https://opencollective.com/parcel/donate -->
7 changes: 1 addition & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,24 @@ yarn test
[node]: https://nodejs.org/
[yarn]: https://yarnpkg.com/


## Financial contributions

We also welcome financial contributions in full transparency on our [open collective](https://opencollective.com/parcel).
Anyone can file an expense. If the expense makes sense for the development of the community, it will be "merged" in the ledger of our open collective by the core contributors and the person who filed the expense will be reimbursed.


## Credits


### Contributors

Thank you to all the people who have already contributed to parcel!
<a href="graphs/contributors"><img src="https://opencollective.com/parcel/contributors.svg?width=890" /></a>


### Backers

Thank you to all our backers! [[Become a backer](https://opencollective.com/parcel#backer)]

<a href="https://opencollective.com/parcel#backers" target="_blank"><img src="https://opencollective.com/parcel/backers.svg?width=890"></a>


### Sponsors

Thank you to all our sponsors! (please ask your company to also support this open source project by [becoming a sponsor](https://opencollective.com/parcel#sponsor))
Expand All @@ -71,4 +66,4 @@ Thank you to all our sponsors! (please ask your company to also support this ope
<a href="https://opencollective.com/parcel/sponsor/6/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/6/avatar.svg"></a>
<a href="https://opencollective.com/parcel/sponsor/7/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/7/avatar.svg"></a>
<a href="https://opencollective.com/parcel/sponsor/8/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/8/avatar.svg"></a>
<a href="https://opencollective.com/parcel/sponsor/9/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/9/avatar.svg"></a>
<a href="https://opencollective.com/parcel/sponsor/9/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/9/avatar.svg"></a>
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
```shell
yarn global add parcel-bundler
```

or with npm:

```shell
npm install -g parcel-bundler
```
Expand Down Expand Up @@ -97,14 +99,12 @@ All feedback and suggestions are welcome!
This project exists thanks to all the people who contribute. [[Contribute]](CONTRIBUTING.md).
<a href="graphs/contributors"><img src="https://opencollective.com/parcel/contributors.svg?width=890" /></a>


## Backers

Thank you to all our backers! 🙏 [[Become a backer](https://opencollective.com/parcel#backer)]

<a href="https://opencollective.com/parcel#backers" target="_blank"><img src="https://opencollective.com/parcel/backers.svg?width=890"></a>


## Sponsors

Support this project by becoming a sponsor. Your logo will show up here with a link to your website. [[Become a sponsor](https://opencollective.com/parcel#sponsor)]
Expand All @@ -120,8 +120,6 @@ Support this project by becoming a sponsor. Your logo will show up here with a l
<a href="https://opencollective.com/parcel/sponsor/8/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/8/avatar.svg"></a>
<a href="https://opencollective.com/parcel/sponsor/9/website" target="_blank"><img src="https://opencollective.com/parcel/sponsor/9/avatar.svg"></a>



## License

MIT
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"uglify-es": "^3.2.1",
"v8-compile-cache": "^1.1.0",
"worker-farm": "^1.4.1",
"ws": "^3.3.2"
"ws": "^3.3.3"
},
"devDependencies": {
"babel-cli": "^6.26.0",
Expand Down
12 changes: 12 additions & 0 deletions src/HMRServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ class HMRServer {
});

this.wss.on('connection', ws => {
ws.onerror = this.handleSocketError;
if (this.unresolvedError) {
ws.send(JSON.stringify(this.unresolvedError));
}
});

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

return this.wss._server.address().port;
}

Expand Down Expand Up @@ -69,6 +72,15 @@ class HMRServer {
}
}

handleSocketError(err) {
if (err.code === 'ECONNRESET') {
// This gets triggered on page refresh, ignore this
return;
}
// TODO: Use logger to print errors
console.log(prettyError(err));
}

broadcast(msg) {
const json = JSON.stringify(msg);
for (let ws of this.wss.clients) {
Expand Down
5 changes: 4 additions & 1 deletion src/builtins/hmr-runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ if (!module.bundle.parent) {
}

if (data.type === 'reload') {
window.location.reload();
ws.close();
ws.onclose = () => {
window.location.reload();
}
}

if (data.type === 'error-resolved') {
Expand Down
21 changes: 15 additions & 6 deletions test/hmr.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,24 @@ describe('hmr', function() {
rimraf.sync(__dirname + '/input');
});

afterEach(function() {
if (b) {
b.stop();
b = null;
}
afterEach(function(done) {
let finalise = () => {
if (b) {
b.stop();
b = null;

done();
}
};

if (ws) {
ws.close();
ws = null;
ws.onclose = () => {
ws = null;
finalise();
};
} else {
finalise();
}
});

Expand Down
4 changes: 3 additions & 1 deletion test/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ describe('plugins', function() {
});

it('should load package.json from parent tree', async function() {
let b = await bundle(__dirname + '/integration/plugins/sub-folder/index.js');
let b = await bundle(
__dirname + '/integration/plugins/sub-folder/index.js'
);

assertBundleTree(b, {
name: 'index.js',
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4934,9 +4934,9 @@ write-file-atomic@^1.1.4:
imurmurhash "^0.1.4"
slide "^1.1.5"

ws@^3.3.2:
version "3.3.2"
resolved "https://registry.yarnpkg.com/ws/-/ws-3.3.2.tgz#96c1d08b3fefda1d5c1e33700d3bfaa9be2d5608"
ws@^3.3.3:
version "3.3.3"
resolved "https://registry.yarnpkg.com/ws/-/ws-3.3.3.tgz#f1cf84fe2d5e901ebce94efaece785f187a228f2"
dependencies:
async-limiter "~1.0.0"
safe-buffer "~5.1.0"
Expand Down

0 comments on commit 9f27e15

Please sign in to comment.