feat: add job retrying and failed:fatal events#227
Conversation
| @@ -545,9 +544,8 @@ class Queue extends Emitter { | |||
| } | |||
|
|
|||
| _finishJob(err, data, job) { | |||
There was a problem hiding this comment.
Refactored this method and moved some code to Job#computeDelay so we can compute the final status early.
| // Workaround for #184: emit failed event for backwards | ||
| // compatibility while affording for a separate event that | ||
| // identifies the final failure. | ||
| const emitExtra = | ||
| status === 'retrying' | ||
| ? 'failed' | ||
| : status === 'failed' | ||
| ? 'failed:fatal' | ||
| : null; | ||
| if (emitExtra) this.emit(emitExtra, job, result); |
|
@skeggse this needs a rebase, are you able to take care of that? |
|
Ah, fair enough then. I don't recall these discussions or this PR well enough to green-light anything, but I'll try and find some time to rebase over the next few days. |
7916cef to
9f4d4e2
Compare
Adds the documented but missing `retrying` event, and adds a `failed:fatal` event that signals that a job has failed permanently. The `failed` event remains as it was, signaling job execution failures without signaling the outcome of the job. Co-authored-by: Hugh Secker-Walker <hsw@hodain.net>
9f4d4e2 to
f78ad0f
Compare
|
Rebased, largely to reconcile against the changes in #134. Unfortunately, it seems the tests are flaky. I don't know if this is a new upstream regression, or something that changed in this branch, but I don't have the bandwidth to diagnose that. It's especially difficult to debug in this project because ava doesn't report which test(s) were responsible for the timeout (or at least, that used to be its behavior, and I don't see any obvious change in the output that would suggest otherwise). |
Could you describe your setup? I'm getting intermittent and inconsistent test failures when I run the tests locally on my M1 MacBook Pro with Redis 6.2 and Node 16.17.1. Strangely, they pass consistently when run in CI. Not sure what's up with that, more data may suggest a solution. |
|
@jorenvandeweyer please add your review! Thanks :) |
Ah, this is in CI. It (seemed to) pass consistently in my local setup. Just a |
@compwright I'm seeing the same inconsistent test failures on my M1 Pro (I'm using DBngin). I suggest you run the tests using Docker for now. |
## [1.7.0](v1.6.0...v1.7.0) (2023-11-06) ### Features * improve job failure events ([#227](#227)) ([15d02c2](15d02c2))
|
🎉 This PR is included in version 1.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds the documented but missing
retryingevent, and adds afailed:fatalevent that signals thata job has failed permanently. The
failedevent remains as it was, signaling job executionfailures without signaling the outcome of the job.
Fixes #184; see #186 for more discussion and an alternate proposal.