Skip to content

Fixes #207: Synchronously log error on exit #208

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

Merged
merged 1 commit into from
Mar 15, 2018
Merged

Conversation

Hilzu
Copy link
Contributor

@Hilzu Hilzu commented Mar 14, 2018

Calling process.exit(1) exits the process immediately and might prevent writes from completing as explained in process.exit documentation. Setting the exitCode property instead lets pending events in the event loop run (like writing to stderr) but prevents any additional work from being scheduled. This is the recommended way to exit Node processes.

@Hilzu
Copy link
Contributor Author

Hilzu commented Mar 14, 2018

It seems that setting the exitCode property doesn't have the effect I was hoping for (it doesn't actually exit). Let me rewrite the printErrorAndExit function to use fs.writeSync and exiting with process.exit(1)

@Hilzu Hilzu changed the title Fixes #207: let pending events flush when exiting Fixes #207: Synchronously log error on exit Mar 14, 2018
@Hilzu
Copy link
Contributor Author

Hilzu commented Mar 14, 2018

According to the fs.writeSync documentation the version of the function that takes a string instead of buffer was only added in v0.11.5 but it seems to work according to my manual testing with v0.10.48 and the tests pass. This should be good now.

Calling `process.exit(1)` exits the process immediately and might prevent writes from completing as explained in [process.exit documentation](https://nodejs.org/api/process.html#process_process_exit_code). Using `fs.writeSync` ensures that the writes are done before exiting.
@LinusU
Copy link
Collaborator

LinusU commented Mar 15, 2018

Thank you!

@LinusU LinusU merged commit eaa6fdd into evanw:master Mar 15, 2018
@LinusU
Copy link
Collaborator

LinusU commented Mar 15, 2018

Released as 0.5.4, than you for your contribution 🚀

@mohd-akram
Copy link
Contributor

This PR seems to have broken error messages when using a debugger such as Chromium. This can be seen by creating a file test.js with the following:

const fs = require('fs');
fs.writeSync(2, 'error\n');

And running node --inspect-brk test.ts then debugging in Chromium. You'll see that the output is printed in your terminal, but not in the Chromium console.

What was wrong with setting process.exitCode? I've tried it and it seems to work.

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.

3 participants