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: do not throw if child process no longer exist #108

Merged
merged 2 commits into from
Oct 23, 2018

Conversation

mucsi96
Copy link
Contributor

@mucsi96 mucsi96 commented Oct 7, 2018

Fixes #46

child process kill wrapped in try catch

Fixes bahmutov#46
@bahmutov
Copy link
Owner

bahmutov commented Oct 8, 2018

@mucsi96 thank you for submitting this feature, is there are any chance a test could be added, something similar to "demo" scripts?

@mucsi96
Copy link
Contributor Author

mucsi96 commented Oct 8, 2018

Sure. Probably with mocking I can do that. Will add it Today. Also I am wandering do we want to explicitly ignore this error type. Or it’s fine like this?

@bahmutov
Copy link
Owner

bahmutov commented Oct 8, 2018 via email

@mucsi96
Copy link
Contributor Author

mucsi96 commented Oct 8, 2018

I tried to come up with test you suggested without too much luck. Unfortunately it's not good if the server just immediately exits as it produces different exception. The child process should run until the psTree call and exit immediately after it - which is quite tricky without doing some complex mocking :)

@mucsi96
Copy link
Contributor Author

mucsi96 commented Oct 11, 2018

Do you have any idea how can we write a test for this? Or can we merge this without a test?

@wlsf82
Copy link

wlsf82 commented Oct 23, 2018

@mucsi96, thanks for the fix. I'm using your fork for now.
Please notify when merging to the original repo so that I can go back to the official one.

@bahmutov
Copy link
Owner

Ok, I will merge the PR, and hope it fixes issue and does not introduce any new ones

@bahmutov bahmutov changed the title Do not throw if child process no longer exist fix: do not throw if child process no longer exist Oct 23, 2018
@bahmutov bahmutov merged commit c623c62 into bahmutov:master Oct 23, 2018
@bahmutov
Copy link
Owner

🎉 This PR is included in version 1.7.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mucsi96
Copy link
Contributor Author

mucsi96 commented Oct 23, 2018

Thx!

Copy link

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thank you!!!! 💙💚💛💜

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

Successfully merging this pull request may close these issues.

4 participants