[kamilmysliwiec]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

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?

The worker reads an endpoint once for each group of its deliveries in a batch (up to worker.batchSize, 50 by default) and uses that copy for every delivery in the group. A change made while the group is being sent doesn't apply to the rest of it: after update(id, { enabled: false }) or delete(id) the remaining deliveries are still sent, after update(id, { url }) they go to the old URL, and after rotateSecret(id, { overlap: 0 }) they're signed with the old secret only. The docs say pending deliveries of a disabled or deleted endpoint fail "when their turn comes", and WebhookEndpointStore's JSDoc says the worker reads it before each attempt.

A related detail: after a 410 Gone disables the endpoint mid-group, the worker patched its copy to enabled: false without the reason, so the rest of the group failed with "is disabled (manual)" instead of "(gone)".

Issue Number: #46

What is the new behavior?

The worker reads the endpoint before each attempt, after the lease check, so deliveries it releases cost no read. A disable, delete, URL change or rotation applies from the next attempt; an attempt already in flight finishes as it started. Since the store already records enabled, disabledReason and failingSince, the worker's local patches of its copy (added in #43) and the two Outcome fields that fed them are removed. The order of deliveries within a group is unchanged.

Thanks to @johnnyhuirilef for the report, the reproduction and the idea of reading the endpoint again for each delivery.

Two choices compared to the proposal in the issue:

• A read on every iteration rather than only for i > 0. The first attempt then uses the same code path as the others, and the local enabled/failingSince patches can go entirely instead of staying alongside a fresh read. The first read also moves after the stop/lease check. The cost is the same: one primary-key lookup per attempt, next to the store write each attempt already makes and the HTTP request itself.
• No revision check (updatedAt) instead of a full read. addEndpointSecret() doesn't bump updatedAt (in the in-memory and PostgreSQL stores), so a compare would miss a secret rotation. It would also need a read anyway, and a new store method that every custom store would have to implement.

Tests (tests/outgoing.e2e.spec.ts, "an endpoint changed while the worker sends a batch to it"): a disable, a delete, a URL change and a rotation with overlap: 0 during the first of three requests, plus the "(gone)" reason after a 410. All five fail on master.

Does this PR introduce a breaking change?

• Yes
• No

Other information

The worker now makes one endpoint read per attempt instead of one per group of deliveries.

[thekhegay]

Tried it with store whose getEndpoint() takes 2s (controllable clock like lease spec in worker.spec.ts), delivery.timeout: '10s', worker.lease: '14s', two deliveries to one endpoint:

lease left when each send starts: master 12000 and 11000 ms, PR 12000 and 9000 ms
second worker polling at lease end: master sends message 2 once, PR twice with same webhook-id
stop() during read: master sends 0 and releases 2, PR sends 1 and releases 1
50ms read, first response 3.93s: lease left at second send master 10020 ms, PR 9970 ms

With suggestion every send starts with 10s+ of lease left, message 2 goes out once, stop() sends nothing and full suite stays green, relased group just pays one extra read

[thekhegay]

read now sits between stop/lease check and send, so its latency eats into lease margin and attempt can start with less than timeoutMs left or after stop()

Suggested change

	// Never start an attempt that could outlive the lease: another worker could claim the delivery and send it too.
	if (this.stopping || Date.now() + this.config.timeoutMs > leaseUntil) {
	return this.release(group.slice(i), owner, result);
	}
	// Read before each attempt, not once per group: the endpoint may have been disabled, deleted, moved or had
	// its secret rotated (or the previous attempt changed its health) since.
	const endpoint = await this.endpoints.getEndpoint(endpointId);
	// Read before each attempt, not once per group: the endpoint may have been disabled, deleted, moved or had
	// its secret rotated (or the previous attempt changed its health) since.
	const endpoint = await this.endpoints.getEndpoint(endpointId);
	// Never start an attempt that could outlive the lease: another worker could claim the delivery and send it too.
	if (this.stopping || Date.now() + this.config.timeoutMs > leaseUntil) {
	return this.release(group.slice(i), owner, result);
	}