The handler looked defensive. That was the problem.
@PostMapping("/webhooks/payments")
ResponseEntity<Void> onPaymentEvent(@RequestBody String payload) {
try {
paymentEvents.apply(parse(payload));
} catch (Exception e) {
log.error("Failed to process payment event", e);
}
return ResponseEntity.ok().build(); // always
}Somebody wrote that to stop a bad event taking the endpoint down. It is a reasonable instinct and it produced three weeks of silent data loss.
What 200 means to the sender
Every webhook sender treats a 2xx as an acknowledgement: received, understood, do not send again. The event leaves their retry queue permanently.
That acknowledgement is the entire delivery guarantee. A sender that retries with exponential backoff for a day is offering you a day's worth of protection against your own downtime, deploys and bugs — and a blanket 200 hands all of it back.
The catch block turned a recoverable failure into an unrecoverable one. Every
event that hit an exception was logged, acknowledged, and gone.
Why it took three weeks
The failure was in parsing a field that only appeared on one event type, which happened rarely. So:
- The error rate was near zero, because the endpoint returned 200.
- The sender's dashboard showed 100% successful delivery.
- The log line existed, but
log.errorin a service that produces plenty of log volume is not an alert, and nobody was reading it. - The data was not obviously missing. Records were absent rather than wrong, and absence does not show up in a spot check.
I found it because a customer asked about something that should have been there. That is the worst possible detection mechanism, and it was the only one I had.
The rule
Return 2xx only after the event is durably recorded. Anything that fails before that returns 5xx, so the sender retries.
Which sounds obvious, and is, and is still worth writing down because the defensive-catch version looks more careful.
What the handler should do
Three things, then stop.
@PostMapping("/webhooks/payments")
ResponseEntity<Void> onPaymentEvent(@RequestBody String payload,
@RequestHeader("Signature") String signature) {
// 1. Authenticate. A bad signature is not retryable — 400, not 500.
if (!verifier.isValid(payload, signature)) {
return ResponseEntity.badRequest().build();
}
var event = parse(payload);
// 2. Record durably. Idempotent on the sender's own event id.
boolean stored = inbox.recordIfAbsent(event.id(), payload);
// 3. Hand off. Nothing else happens on this thread.
if (stored) {
processor.enqueue(event.id());
}
return ResponseEntity.ok().build();
}If recordIfAbsent throws, the exception propagates, Spring returns 500, and the
sender retries. That is the correct behaviour, and it required deleting code
rather than adding it.
Note that a duplicate still returns 200. A repeat delivery is the guarantee working, not an error — the same reasoning as any at-least-once consumer, which I have written up in more depth in idempotency keys are not enough.
Which failures are retryable
Not everything deserves a 5xx. Getting this wrong in the other direction gives you an event the sender retries forever.
| Failure | Status | Why |
|---|---|---|
| Invalid signature | 400 | Retrying will not fix it |
| Malformed body | 400 | Same payload, same result |
| Unknown event type | 200 | Record it and move on; not an error |
| Database unavailable | 500 | Transient, retry is exactly right |
| Downstream service down | 200 | Already recorded — the worker retries, not the sender |
| Unexpected exception | 500 | Unknown means assume transient |
The "downstream service down" row is the one worth pausing on. Once the event is in your inbox table, the sender has done its job. Failures after that point are yours to retry internally, and pulling the sender back into your recovery makes the failure harder to reason about, not easier.
What I changed beyond the handler
The code fix took ten minutes. The rest took longer and mattered more.
An alert on inbox rows that never reached a terminal state. A row recorded but not processed within a window is the thing I actually care about, and it is observable without knowing which bug caused it.
Reconciliation against the sender. Once a day, count events on their side against events on mine for the same period. A gap is a bug regardless of what the logs say. This is the check that would have caught it on day one, and it is about twenty lines.
No blanket catch in a webhook handler. If a specific exception is genuinely
non-retryable, catch that exception specifically and say why in a comment. A bare
catch (Exception e) at a delivery boundary is a decision to discard data, and
it should read like one.
The general shape
This is the same class of bug as an alert that only fires on errors: the system was failing in a way that produced no signal, and the absence of a signal was read as health. I hit the identical pattern from a different direction with the free-tier quota that took my product down for two days — there the signal was consumption rather than acknowledgement, but the lesson is the same one.
Monitor the thing you actually promised, not the thing that is easy to measure. Delivery rate was easy. Events applied was the promise.