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

Implement clean shutdown code via bedrock.exit() #60

Closed
dlongley opened this issue May 13, 2020 · 4 comments
Closed

Implement clean shutdown code via bedrock.exit() #60

dlongley opened this issue May 13, 2020 · 4 comments
Assignees
Labels
enhancement Priority 2 Important but not critical

Comments

@dlongley
Copy link
Member

We need to implement cleaner shutdown code via bedrock.exit(). When bedrock.exit() is called, a bedrock.exit event should be emitted that is awaited on all workers -- and new workers should be prevented from starting up. Once all workers have exited, we can call cluster kill. A flag can be provided to bedrock.exit() with an optional timeout that will forcibly call cluster kill after the timeout.

@aljones15
Copy link
Contributor

aljones15 commented May 13, 2020

this sounds good. I will work on this when time allows.

Just so there is more context for this issue.
We are having issues with bedrock-test, mocha, and fire and forget promises that result in an obscure node error: ERR_INTERNAL_ASSERTION

Node issue is here

The problem is we are not cleaning up promises when bedrock shuts down resulting in Node throwing and then Mocha throwing an uncaught Exception after the test has run.

@mattcollier mattcollier added the Priority 2 Important but not critical label Jun 30, 2020
@aljones15
Copy link
Contributor

aljones15 commented Oct 11, 2020

one idea is to make a new function in bedrock.utils cancellable:

bedrock.utils.cancellable = ({events = [], action}) => new Promise(async (resolve, reject) => {
  for(const event of events) {
    // we could make this configurable to also call on resolve
    bedrock.events.on(event, () => reject(new Error(`Action cancelled by event ${event}`)));
  }
  const result = await action();
  // FIXME unsubscribe from events here.
  resolve(result);
});

This is meta-code and does not handle situations such as wanting to cancel with a reject for some events and a resolve for others, but it would allow:

const {cancellable} = bedrock.utls;
const FireForgetCancel = await cancellable({events: ['exit'], action: watchMongo});

This would allow async functions that might be running on exit to cancel and remove themselves from the async que.

@aljones15
Copy link
Contributor

aljones15 commented Oct 23, 2020

AbortController which is experimental in node 15 could also be used to handle cancellation:

https://developer.mozilla.org/en-US/docs/Web/API/AbortController

Node Docs:
https://nodejs.org/docs/latest-v15.x/api/globals.html#globals_class_abortcontroller

@dlongley
Copy link
Member Author

dlongley commented Jul 2, 2021

We now emit the bedrock.exit event and apps can handle it and gracefully exit.

@dlongley dlongley closed this as completed Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Priority 2 Important but not critical
Projects
None yet
Development

No branches or pull requests

4 participants