Skip to content

fix(destregistry): treat event format errors as failed deliveries, not DLQ#957

Merged
alexluong merged 4 commits into
mainfrom
fix/format-error-failed-attempt
Jun 16, 2026
Merged

fix(destregistry): treat event format errors as failed deliveries, not DLQ#957
alexluong merged 4 commits into
mainfrom
fix/format-error-failed-attempt

Conversation

@alexluong

@alexluong alexluong commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Problem

When a destination fails to format a specific event (e.g. an aws_s3 key_template referencing a field that event doesn't have), the pipeline treated it as a system error: the message was retried blindly and eventually dead-lettered. The failure was never recorded, was invisible to the customer, and paged us — instead of being handled like any other failed delivery.

What this changes

A formatting failure is now treated as a normal failed delivery: it's recorded as a failed attempt the customer can see, and it retries on the normal schedule until exhausted. No silent dead-lettering, no page.

Applied across all destination types that format events per-event.

Tests

Unit coverage plus an e2e regression (cmd/e2e/regression_format_error_test.go) that forces a format failure and asserts it's recorded as failed attempts and exhausts retries without being dead-lettered.

🤖 Generated with Claude Code

alexluong and others added 4 commits June 16, 2026 12:05
…t DLQ

A Format/key-template failure (e.g. an S3 key_template referencing a field
absent from the event) returned a nil delivery, which the registry turned into
a nil attempt and the deliverymq handler classified as a PreDeliveryError →
nack → Pub/Sub DLQ. The failure was never logged, invisible to the customer,
and paged us instead of surfacing as an actionable delivery error.

Add destregistry.NewFormatErrorDelivery, returning a non-nil failed Delivery
plus an ErrDestinationPublishAttempt, so the registry records a failed attempt,
acks the message, and retries via the scheduler. The customer-facing response
is a generic message; the raw Go error stays on the error for logs/telemetry
and is not persisted on the attempt.

Apply it across all providers with a Format step: s3, sqs, azure_servicebus,
gcp_pubsub, webhook, webhook_standard (previously `return nil, err`) and
kinesis, kafka (previously nil-delivery ErrDestinationPublishAttempt).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot DLQ

Standalone e2e test reproducing the production incident: an aws_s3 destination
whose key_template references a field missing from the event. Asserts the fixed
behavior end to end — nothing is written to S3, each delivery is recorded as a
failed attempt carrying the format error, and retries run on the normal schedule
and exhaust their budget rather than being nacked/dead-lettered.

Verified as a real guard: reverting the destawss3 fix makes this test fail
(0 attempts logged, message dead-lettered) instead of recording 3 attempts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The helper returns the (*Delivery, error) pair a publisher returns on a format
failure, not just a delivery — name it accordingly. Behavior unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alexluong alexluong merged commit f43a92f into main Jun 16, 2026
3 checks passed
@alexluong alexluong deleted the fix/format-error-failed-attempt branch June 16, 2026 17:53
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