search

pkrumins / node-tree-kill

kill trees of processes
MIT License
335 stars 37 forks source link

Update index.js #15

Closed Unitech closed 7 years ago

billiegoose commented 7 years ago

Hi! It's really exciting that you contributed a pull request! I'm a little unsure that simply catching and ignoring the error is the proper thing to do here though. Which OS were you having errors on? Maybe the 'ps' command needs fixing.

billiegoose commented 7 years ago

Looking at this again, a little older and wiser, I'm reading this comment: "// Catch error msg else treekill will throw an uncaught Exception" (emphasis mine) As in an uncatchable exception? Not simply tree-kill throwing an error, but node crashing. If this is the case, I definitely think handling stream errors is a wise addition, however I ask that the error be passed along to the callback rather than silently ignored.

billiegoose commented 7 years ago

I don't really have enough information to act on this... but if it's throwing an error that's a symptom of a deeper problem that should be addressed. I don't think I should merge in a PR that silently ignores the error.