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

Added ability to reset the timeout #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bugs181
Copy link

@bugs181 bugs181 commented May 31, 2020

Example use:

q.on('timeout', (next, job, resetTimeout) => {
  if (job.stillAlive) {
    console.log('Job said it was still working on something, giving it more time to finish')
    job.stillAlive = false
    resetTimeout()
    return
  }

  processJobError(job, 'Job timed out')
  next()
})

@jessetane
Copy link
Owner

Another contributor added this feature recently, does it cover your use case?

321fe6e

@bugs181
Copy link
Author

bugs181 commented Jun 7, 2020

Another contributor added this feature recently, does it cover your use case?

321fe6e

Unfortunately, the linked feature is an opt-out of timeouts. What I needed was a way to extend the timeout based on some event. I still want timeouts just in case the "job" fails to execute or whatever.

As a concrete example, using the job.stillAlive check. In my particular application, stillAlive is set every time new data comes in from a download event handler. We still want timeouts just in case the network is interrupted or some other event happens.

@jessetane
Copy link
Owner

Well, to be fair, that feature allows you to set custom timeouts per job, not just opt out.

Anyhow your reset idea is cool, but I wonder, should we fix this first?

#66

@bugs181
Copy link
Author

bugs181 commented Jun 12, 2020

should we fix this first?

Oof! Good catch. I'll see if I can implement a PR.

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