Skip to content

beforeExit does not exist in node 0.10 - fixes #14 #22

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

Closed
wants to merge 6 commits into from
Closed

beforeExit does not exist in node 0.10 - fixes #14 #22

wants to merge 6 commits into from

Conversation

warang580
Copy link
Contributor

Tests were failing because it couldn't move global generators (rename directories) into stimpak generators/ folder because they already were there. After some search, I found that generators were not cleaned because it never catches the beforeExit event, because it doesn't exist in Node v.0.10 - so global generators were never moved back after use.

You don't need to process.exit() when listening to exit (as it was for beforeExit) because it's already too late, the process is about to stop and you can't prevent it.

@dcrockwell
Copy link
Contributor

While the tests pass, it's because there is no coverage for the copying system (because it's a hack, not actual behavior of stimpak).

process.on("exit") does not allow asynchronous tasks to happen, so all callbacks are ignored, meaning that the cleanup task never actually happens (and we don't see that in the tests because there isn't a test to check if the cleanup actually occurred, again because this is hacky behavior and I intend to replace it with the absolute minimal solution, then codify it as a spec.

Try it out yourself, and see how the generator gets moved to /generators, then never gets put back.

@dcrockwell
Copy link
Contributor

I'm not sure if there's a good way to reliably cleanup with node 0.10, considering a Ctrl-C can short circuit the whole process and exit before cleanup.

@dcrockwell
Copy link
Contributor

We could try process.on("SIGINT"

@dcrockwell
Copy link
Contributor

Looks like there's a bug that affects all libraries that utilizes readline (like inquirer.js) that prevents us from using the SIGINT hook: nodejs/node#4758

Until this bug is fixed, the entire cleanup sequence must be made synchronous to avoid the issue where the event loop quits taking new events, or we simply cannot support Node 0.10.

@dcrockwell dcrockwell closed this Jul 7, 2016
@warang580 warang580 deleted the fix/clean-exit branch July 7, 2016 11:52
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.

2 participants