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

[gatsby-cli] lib/reporter hard exits on panic #13179

Closed
amclin opened this issue Apr 6, 2019 · 3 comments
Closed

[gatsby-cli] lib/reporter hard exits on panic #13179

amclin opened this issue Apr 6, 2019 · 3 comments
Labels
stale? Issue that may be closed soon due to the original author not responding any more.

Comments

@amclin
Copy link

amclin commented Apr 6, 2019

Summary

When encountering an error, the Gatsby CLI reporter forces a hard-exit using process.exit()

This is explicitly discouraged by NodeJS because it prevents other asynchronous tasks from completing.

In most situations, it is not actually necessary to call process.exit() explicitly. The Node.js process will exit on its own if there is no additional work pending in the event loop. The process.exitCode property can be set to tell the process which exit code to use when the process exits gracefully.
...
The reason this is problematic is because writes to process.stdout in Node.js are sometimes asynchronous and may occur over multiple ticks of the Node.js event loop. Calling process.exit(), however, forces the process to exit before those additional writes to stdout can be performed.

Basic example

replace with:

process.exitCode = 1

Motivation

A real use case I'm trying to solve: I'm capturing log events in order to post to a remote monitoring service. But the most important events to capture are errors, and I cannot capture them because the entire Node process exits.

@sidharthachatterjee
Copy link
Contributor

This is interesting.

In most situations, it is not actually necessary to call process.exit() explicitly. The Node.js process will exit on its own if there is no additional work pending in the event loop.

I've seen instances when a panic logs multiple times in an endless loop (don't remember where) so I'm not sure if process.exitCode = 1 will suffice in situations where we would want to hard exit.

From the linked documentation

If it is necessary to terminate the Node.js process due to an error condition, throwing an uncaught error and allowing the process to terminate accordingly is safer than calling process.exit().

This does seem like a cleaner way to do this. @pieh thoughts in case I'm missing something here?

@gatsbot
Copy link

gatsbot bot commented May 1, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label May 1, 2019
@gatsbot
Copy link

gatsbot bot commented May 12, 2019

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale? Issue that may be closed soon due to the original author not responding any more.
Projects
None yet
Development

No branches or pull requests

2 participants