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

beforeShutdown in example should return new Promise #61

Closed
peterkuiper opened this issue Jul 4, 2018 · 4 comments
Closed

beforeShutdown in example should return new Promise #61

peterkuiper opened this issue Jul 4, 2018 · 4 comments

Comments

@peterkuiper
Copy link
Contributor

Should be:

function beforeShutdown () {
  // given your readiness probes run every 5 second
  // may be worth using a bigger number so you won't
  // run into any race conditions
  return new Promise(resolve => {
    setTimeout(resolve, 5000)
  })
}
terminus(server, {
  beforeShutdown
})

or ES6:

const beforeShutdown = () => new Promise(resolve => setTimeout(resolve, 15000));
@peterkuiper
Copy link
Contributor Author

peterkuiper commented Jul 4, 2018

I'm also trying to figure out where beforeShutdown() fits in. I have beforeShutdown, onSignal and onShutdown set. beforeShutdown is run before onSignal. I would expect:

onSignal -> beforeShutdown -> onShutdown

onSignal would allow me to the actual logging (without a delay) and pre-cleanup steps that wouldn't interfere with serving traffic. I would then do my cleanup in beforeShutdown so onShutdown can be safely called.

Does this make sense?

@gergelyke
Copy link
Collaborator

@peterkuiper can you send a PR to fix the docs?

@peterkuiper
Copy link
Contributor Author

@gergelyke I will send the PR on monday. I'll also take a look at the examples, I think those also miss the 'new Promise'.

@gergelyke
Copy link
Collaborator

Cool, thanks a lot @peterkuiper!

peterkuiper added a commit to peterkuiper/terminus that referenced this issue Jul 9, 2018
peterkuiper added a commit to peterkuiper/terminus that referenced this issue Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants