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

Solution -- with exceptions #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Solution -- with exceptions #2

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 18, 2014

Still not able to get child process to catch SIGINT and SIGTERM from
index.js at all. Some research leads me to believe this is due to
Windows not supporting sending/processing signals. Very curious to know
if this is the case or my implementation is wrong.

Still not able to get child process to catch SIGINT and SIGTERM from
index.js at all.  Some research leads me to believe this is due to
Windows not supporting sending/processing signals.  Very curious to know
if this is the case or my implementation is wrong.
@txase
Copy link
Owner

txase commented Sep 18, 2014

Hey Shaina,

So I have to confess that you were the first guinea pig for this challenge, but you did a good job of highlighting the deficiencies in it :). I was looking for your solution to work if you were to do something like:

var err = new Error('something went wrong!');

try {
  throw err;
} catch (e) {
  console.log(e.stack); // Should still print out the stack even when using the logger module
}

I forgot to write a test for this. But as I was trying to write a test for this, I found out that this may not be possible... I believe v8 will only add the stack property to a real Error object rather than any sort of wrapper object we create.

Of course, I also didn't think about the fact that UNIX signals aren't available on Windows either...

That said, everything else looks good and passes the tests!

I'll send you a follow up email :).

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.

1 participant