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

Fix for ipc closed error in tests #591

Merged
merged 2 commits into from
Jan 26, 2018
Merged

Fix for ipc closed error in tests #591

merged 2 commits into from
Jan 26, 2018

Conversation

DeMoorJasper
Copy link
Member

In the tests there sometimes accurs an ipc closed while sending data error, this is due to the main thread being killed before the children died.
This should force-kill the children, preventing this from happening. Another solution is awaiting the super.end using a promise or callback but this caused many tests to fail and in theory this fix would be just as solid and will not wait for unused data to be received.

@codecov-io
Copy link

codecov-io commented Jan 19, 2018

Codecov Report

Merging #591 into master will decrease coverage by 0.26%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #591      +/-   ##
==========================================
- Coverage   88.95%   88.69%   -0.27%     
==========================================
  Files          61       61              
  Lines        1902     1910       +8     
==========================================
+ Hits         1692     1694       +2     
- Misses        210      216       +6
Impacted Files Coverage Δ
src/WorkerFarm.js 94.44% <100%> (+0.56%) ⬆️
src/worker.js 90.9% <33.33%> (-9.1%) ⬇️
src/Logger.js 32.72% <0%> (-5.46%) ⬇️
src/assets/RustAsset.js 89.28% <0%> (-1.2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d7339a...4e11093. Read the comment docs.

@fathyb
Copy link
Contributor

fathyb commented Jan 19, 2018

Fixes the issue for me, good job!

@devongovett
Copy link
Member

The error still occurred for me, but in the child worker process not in the master. I added something to worker.js to handle the promise rejections and just ignore that error.

@devongovett devongovett merged commit aee8510 into parcel-bundler:master Jan 26, 2018
@DeMoorJasper DeMoorJasper deleted the feature/fix-for-ipcClosed-error branch January 31, 2018 09:17
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* fix

* Upgrade worker-farm and handle ERR_IPC_CHANNEL_CLOSED on the worker side
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* fix

* Upgrade worker-farm and handle ERR_IPC_CHANNEL_CLOSED on the worker side
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