Don't count canceled sends as webhook endpoint failures; set prod failure threshold to 10#130
Merged
Merged
Conversation
… failure threshold to 10 A trigger is permanently disabled (status='failed') once its consecutive delivery-failure count reaches MAX_WEBHOOK_FAILURE_COUNT. Two problems surfaced during the 2026-06-10 consumer-lag incident: - The threshold was unset in prod, defaulting to 1: a single bad delivery permanently silenced a trigger (108 triggers in prod are in this state). - Cancellation of our own context (pod shutdown or rebalance mid-POST) surfaced as a client.Do error and was classified as a webhook failure, counting against the customer's endpoint. SendWebhook now returns a plain error (no WebhookFailureCode) when ctx is canceled, so shutdowns no longer increment failure counts. Client-side timeouts leave ctx.Err() nil and still count, as do connection errors and >=400 responses. Prod threshold is set to 10 so transient endpoint blips survive while dead endpoints still get cut off quickly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reset was gated on the in-memory cached trigger's FailureCount, which can be up to a cache-refresh interval (~5 min) stale. A failure burst that incremented the DB counter was typically invisible to the cached copy, so subsequent successful deliveries never triggered the reset and the counter ratcheted up across unrelated incidents until the trigger was permanently disabled. Call ResetTriggerFailureCount unconditionally on success; it already fetches fresh state and no-ops when the count is zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resetting on every successful delivery previously meant a SELECT ... FOR UPDATE transaction per send, with concurrent deliveries of the same trigger serializing on the row lock even when there was nothing to reset. The conditional UPDATE matches no rows (and takes no lock) in the common zero-count case, preserving the cheap fast path the old cached-count gate was aiming for without trusting stale data. Behavior is unchanged: count resets only when > 0, and a failed trigger is re-enabled as part of the reset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
During the 2026-06-10 incident (consumer group stuck with corrupt committed offsets, perpetually replaying the signals topic), we found 108 triggers in prod with
status = 'failed'— permanently disabled and never attempted again. Two contributing causes, both fixed here:Changes
1.
MAX_WEBHOOK_FAILURE_COUNTwas unset in prod and defaulted to 1. A single failed delivery permanently disabled a trigger. Now set to10invalues-prod.yaml: a receiver that fails ten consecutive deliveries is genuinely down, while a deploy-window blip of 5xxs survives.2. Our own shutdowns counted as the customer's failures.
SendWebhookuses the consumer's context, so a pod shutdown or rebalance mid-POST surfaced as aclient.Doerror carryingWebhookFailureCode— incrementing the trigger's failure count even though the endpoint was healthy. With a threshold of 1, every unlucky in-flight send during a restart permanently killed a trigger.SendWebhooknow checkserrors.Is(ctx.Err(), context.Canceled)after a failed POST and returns a plain (non-failure-coded) error in that case. The distinction is deliberate:ctx.Err()is nil)Updated the existing
context cancellationtest, which asserted the old behavior.After merge
Once this image ships, the 108
failedtriggers can be re-enabled (UPDATE triggers SET status = 'enabled', failure_count = 0 WHERE status = 'failed') without immediately re-dying from a transient blip.🤖 Generated with Claude Code
Follow-up commit: reset failure count on every success
The success-path reset was gated on the cached trigger's
FailureCount(refreshed ~every 5 min), so a failure burst that bumped the DB counter was usually invisible to the gate and successful deliveries never reset it. The counter ratcheted up across unrelated incidents until the trigger died — meaning the threshold was effectively "10 lifetime failures," not "10 consecutive." The reset is now unconditional;ResetTriggerFailureCountalready fetches fresh state and no-ops at zero.