Skip to content

fix(worker): reset the failure clock after a failure in the same batch - #43

Merged
kamilmysliwiec merged 1 commit into
nestjs:masterfrom
johnnyhuirilef:fix/success-resets-failure-clock
Oct 22, 2026
Merged

kamilmysliwiec merged 1 commit into
nestjs:masterfrom
johnnyhuirilef:fix/success-resets-failure-clock

Conversation

@johnnyhuirilef

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Hi! 👋 When one worker batch has a failed delivery and then a successful delivery to the same endpoint, the success does not reset the failure clock. The worker reads the endpoint once for each group, and the failure does not update that copy. So the success sees failingSince: null and skips recordEndpointSuccess(). The docs say "A success at any point resets the clock".

Issue Number: #42

What is the new behavior?

  • After a failure that the store recorded, the worker also sets failingSince on its copy of the endpoint. A later success in the same group then calls recordEndpointSuccess(), and the store resets the clock.
  • If recordEndpointFailure() itself fails, the copy does not change, because the store did not set failingSince either.
  • A delivery that is not sent (the endpoint is deleted or disabled) does not change the copy.

I ran the repro of the issue against a packed build of this branch:

RESULT: FIXED: the endpoint stays enabled in all 2 scenarios.

Does this PR introduce a breaking change?

  • Yes
  • No

A success in a separate batch, and a success after no failure, work as before.

Other information

The new test in tests/outgoing.e2e.spec.ts sends a delivery that fails and then a delivery that succeeds to one endpoint in one batch. It runs twice: once with retry: false, so the failure ends the delivery (failed), and once with the default retry, so the failure schedules a retry (retried). It checks that failingSince is null after the batch, and that one failure 2 hours later does not disable the endpoint (with disableEndpointAfter: '1h'). Without the fix, it fails with expected 1792587247650 to be null. If I remove the fix from only one of the two return paths, the case for that path fails.

The test above it, "disables an endpoint after disableEndpointAfter without a success, and a success resets the clock", sends each delivery in its own batch, so it did not catch this.

npx vitest run: 42 files passed, 581 tests passed, 386 skipped. npm run lint and npm run build pass.

@kamilmysliwiec
kamilmysliwiec merged commit 7450731 into nestjs:master Oct 22, 2026
7 checks passed
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