in_s3: stop retrying SQS messages that cannot be decoded - #484
Draft
Watson1978 wants to merge 1 commit into
Draft
Conversation
Watson1978
marked this pull request as draft
August 17, 2026 06:03
Watson1978
force-pushed
the
in_s3-validate-sqs-message-structure
branch
3 times, most recently
from
August 17, 2026 08:26
fdc9195 to
22c6239
Compare
is_valid_queue only checked that the top-level Records or detail key existed, so a message missing the nested s3.object.key reached get_raw_key and raised. The rescue in run turned that into :skip_delete, the message was never removed, and SQS redelivered it after the visibility timeout to fail the same way until the retention period expired. skip_delete is false by default and anyone who can put a message on the queue can trigger it, a body of "hello" included. - Make get_raw_key total and move decoding, validation and match_regexp into object_key_from, where a permanent failure skips the message - Keep object_key_from inside the per-message rescue, so anything unforeseen still falls back to :skip_delete rather than escaping QueuePoller#poll and halting ingestion for retry_error_interval - Report every skipped message through one helper that truncates and scrubs what it logs, since an object key can be megabytes long and can hold invalid UTF-8 that JSON.generate rejects for the log event record - Leave failures after the key is known, such as a failed get or a corrupt archive, on :skip_delete, because the object exists and a redrive policy with a dead letter queue is the right tool there - Redeliver the same message in tests to prove a skipped message is handled once, and assert the warn each malformed shape produces Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Watson1978
force-pushed
the
in_s3-validate-sqs-message-structure
branch
from
August 17, 2026 12:21
195d9d1 to
8ea2375
Compare
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
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.
The problem
Posting
helloto the queue is enough to stall anin_s3source indefinitely.is_valid_queueonly checks that the top-levelRecords(ordetail) key exists, so a body without the nesteds3.object.keygets through it and raises inget_raw_key. Therescueinrunturns every per-message error into:skip_delete, which tellsQueuePollerto leave the message in the queue. SQS redelivers it after the visibility timeout, it fails identically, and this repeats until the retention period expires — up to 14 days of CPU, log volume and SQS requests, with a backtrace logged on every pass and nothing ever ingested.skip_deleteisfalseby default, so no configuration is needed to reach this, and anything that can put a message on the queue can trigger it.The change
get, a corrupt archive, a parse error — on:skip_delete. The object exists there, a misconfigureds3_bucketwould otherwise drain the whole backlog, and a redrive policy with a dead letter queue is the right tool for itrescue, so an unforeseen error still falls back to:skip_deleterather than escapingQueuePoller#polland halting ingestion forretry_error_intervalsecondsevent_bridge_modethat does not match the queue, used to be dropped in silence and looked exactly like an idle queueJSON.generaterejects when the log event record is builtNotes
processnow takes the object key rather than the parsed body. Tests go from 108 to 143, and they redeliver the same message to prove a skipped one is handled exactly once while a failed fetch is still retried. Two pre-existing limitations are unchanged and left for separate changes: onlyRecords[0]is examined, andadd_object_metadatareports the URL-escaped key.