Replace process.exit() with process.exitCode

[kozhevnikov]

Would it be possible to replace process.exit() calls (a couple in run.js is all I could find) with process.exitCode as per Node.js recommendation linked below. It my case Cucumber.js terminates Node before async file logger has a chance to flush, among other possible pending operations in the event loop.

https://nodejs.org/api/process.html#process_process_exit_code

It is important to note that calling process.exit() will force the process to exit as quickly as possible even if there are still asynchronous operations pending that have not yet completed fully, including I/O operations to process.stdout and process.stderr. 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.

[charlierudolph]

Can you wait for the logger to finish in a hook / step definition? I remember warly one there were many cases where cucumber-js wouldn't exit as it was waiting for everything to exit the event loop

[kozhevnikov]

The current workaround I'm using is an arbitrary setTimeout in AfterFeatures, but after learning earlier about exitCode and revisiting the hack due to upcoming registerHandler depreciation I've edited local run.js and it seems to work great. It's definitely worth revisiting due to 1) Node official recommendation, 2) per-process delay that dummy setTimeout introduces when running in parallel and 3) when executing Cucumber by other Node process.

I thought about it hanging forever, but wouldn't timeout on individual steps take care of anything infinite? There must be a graceful way to request an exit with timeout in Node without terminating immediately.

registerHandler('AfterFeatures', function (features, callback) {
  setTimeout(callback, 1000);
});

[charlierudolph]

I'm hesitant to do this as I think other users may experience issues (I'm mainly thinking about those using it for browser testing which is where I remember having it hang). Also that node recommendation only exists on version 6 and 8. It does not exist on version 4 which will be supported until April 2018 (when node drops support). I would be good with adding a CLI option for this though. Something like --drain-event-loop which will use process.exitCode instead of process.exit.

[adamlacoste]

Just want to add that this issue presents a real problem for running Cucumber.js as a child process. I'm doing that with --format json, and if my output grows beyond the 8K chunk size of the stdout stream, I only get the first chunk back before Cucumber's process.exit prematurely terminates the child process, truncating the output. I removed the process.exit and the problem was solved.

I'll try to submit a pull request for this soon.

[adamlacoste]

I put forward a pull request, although it's incomplete per @charlierudolph's preferred solution; I didn't do the CLI flag part.

I have a workaround for the specific issue I was running into, which was that running Cucumber.js in a child process with the JSON formatter was truncating output after the first stdout stream chunk, because process.exit is such a hard exit that child process doesn't wait for the remainder. The solution was just to add | cat to my child process command, and that allowed me to receive the full output in the parent process.

Anybody would be welcome to add onto the above PR, or it may be appropriate to merge it as-is in a few months when version 4 sunsets.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.