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.

Sign up for GitHub

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

Jump to bottom

Timeout not working #155

Closed
ltagliamonte-dd opened this issue Jul 21, 2020 · 3 comments
Closed

Timeout not working #155

ltagliamonte-dd opened this issue Jul 21, 2020 · 3 comments

Comments

@ltagliamonte-dd
Copy link

the timeout option of the library seem not working, here is a simple example code:

const http = require('http');
const { createTerminus } = require('@godaddy/terminus');

function onSignal () {
  console.log('server is starting cleanup');
}

function onShutdown () {
  console.log('cleanup finished, server is shutting down');
}

const server = http.createServer((request, response) => {
  response.end(
    `<html>
      <body>
        <h1>Hello, World!</h1>
       </body>
     </html>`
   );
})

const options = {
  timeout: 30000,
  onSignal: onSignal,                       
  onShutdown: onShutdown,          
};

createTerminus(server,options);

server.listen(8080);

From my understanding of the documentation the timeout option should delay the shutdown, but if i ran:
node app.js
and then kill -TERM <PID>
the logs shows:

node app.js
server is starting cleanup
cleanup finished, server is shutting down
Terminated: 15

and there is not "sleep" of 30s.
Can someone please advise?
@ltagliamonte-dd
Copy link
Author

as per doc using beforeShutdown hook i can wait but i don't understand the timeout option at this point

const http = require('http');
const { createTerminus } = require('@godaddy/terminus');

function onSignal () {
  console.log('server is starting cleanup');
}

function beforeShutdown () {
  console.log('waiting');
  return new Promise(resolve => {
    setTimeout(resolve, 30000)
  })
}

function onShutdown () {
  console.log('cleanup finished, server is shutting down');
}

const server = http.createServer((request, response) => {
  response.end(
    `<html>
      <body>
        <h1>Hello, World!</h1>
       </body>
     </html>`
   );
})

const options = {
  timeout: 30000,
  onSignal: onSignal,                       
  onShutdown: onShutdown,
  beforeShutdown: beforeShutdown          
};

createTerminus(server,options);

server.listen(8080);

@MichaelSitter
Copy link

I forked the repo to do a little experimentation with the timeout handling. afaik timeout does work, I don't think your test is exercising the feature correctly b/c you aren't creating any connections before stopping the server. It might be helpful if there was a test case to verify timeout is respected. You can see my findings here:
MichaelSitter#1

@ltagliamonte-dd
Copy link
Author

thank you @MichaelSitter I confirm that with requests flowing the following snippet works:

const http = require('http');
const { createTerminus } = require('@godaddy/terminus');

function onSignal () {
  console.log('server is starting cleanup');
}

function beforeShutdown () {
  console.log('waiting for k8s environment to completely drain this service');
  return new Promise(resolve => {
    setTimeout(resolve, 10000)
  })
}

function onShutdown () {
  console.log('cleanup finished, server is shutting down');
}

const server = http.createServer(
  (req, res) => {
    setTimeout(() => {
      res.end('hello')
    }, 5000)
  })

const options = {
  timeout: 10000,
  onSignal: onSignal,                       
  onShutdown: onShutdown,
  beforeShutdown: beforeShutdown       
};

createTerminus(server,options);

server.listen(8080);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@MichaelSitter @ltagliamonte-dd