Skip to content

fix: stop a malformed SQS body from poisoning the whole batch - #5215

Merged
edersonbrilhante merged 1 commit into
github-aws-runners:mainfrom
vegardx:fix/malformed-sqs-body-poisons-batch
Jul 21, 2026
Merged

fix: stop a malformed SQS body from poisoning the whole batch#5215
edersonbrilhante merged 1 commit into
github-aws-runners:mainfrom
vegardx:fix/malformed-sqs-body-poisons-batch

Conversation

@vegardx

@vegardx vegardx commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

JSON.parse(body) runs in the record loop at the top of scaleUpHandler, outside the try/catch further down. An unparseable message body therefore throws straight out of the handler.

Lambda treats a thrown exception as a complete batch failure, so nothing is deleted and the entire batch returns to the queue after the visibility timeout. The body is malformed deterministically, so every redelivery reproduces the same SyntaxError. scaleUp is never reached, which means valid messages batched alongside the malformed one are never processed either — they are blocked as collateral damage.

Two things make it worse:

Verified empirically against main: a batch of two valid records plus one malformed one rejects with SyntaxError and scaleUp is called zero times.

Change

Parse each record defensively. A malformed body is logged and reported as an individual batch item failure; the rest of the batch proceeds to scaleUp as normal.

The malformed message then exhausts maxReceiveCount on its own rather than taking the batch with it. It cannot be acknowledged and discarded through the partial-failure mechanism — reporting it is the only way to single it out — so with the default configuration it expires by retention rather than moving to a DLQ. Operators who want these captured should enable redrive_build_queue.

Tests

Four tests covering: the invocation no longer fails, only the malformed message is reported, valid messages still reach scaleUp, and malformed and rejected messages combine correctly. All four fail against the current implementation.

Note on scope

This is distinct from #5129. That PR addresses the catch block, where a non-ScaleError returns an empty batchItemFailures — which AWS treats as complete success, so the batch is deleted. Opposite failure mode, different path, and worth keeping separate. I originally conflated the two in a comment on #5129 and have corrected it there.

Touches the same file as #5129, so whichever lands second will need a trivial rebase.

JSON.parse runs at the top of scaleUpHandler, outside the try/catch further
down, so an unparseable message body threw straight out of the handler. Lambda
treats a thrown exception as a complete batch failure, so nothing is deleted
and the entire batch returns to the queue after the visibility timeout — and
because the body is malformed deterministically, every redelivery reproduces
it. scaleUp is never reached, so valid messages batched alongside it are never
processed either.

ReportBatchItemFailures also disables Lambda's scale-down of polling on failed
invocations, so the usual backoff cushion does not apply here.

Parse each record defensively and report only the malformed message as a batch
item failure. The rest of the batch proceeds; the malformed message exhausts
maxReceiveCount on its own. It cannot be acknowledged and discarded through
that mechanism, so with the default redrive_build_queue disabled it expires by
retention (24h default) rather than moving to a DLQ.
@vegardx
vegardx requested a review from a team as a code owner July 21, 2026 09:54
Copilot AI review requested due to automatic review settings July 21, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@edersonbrilhante edersonbrilhante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR

@edersonbrilhante
edersonbrilhante merged commit 69e06b1 into github-aws-runners:main Jul 21, 2026
8 checks passed
Copilot AI pushed a commit that referenced this pull request Jul 22, 2026
### Problem

`JSON.parse(body)` runs in the record loop at the top of
`scaleUpHandler`, outside the `try`/`catch` further down. An unparseable
message body therefore throws straight out of the handler.

Lambda treats a thrown exception as a complete batch failure, so
**nothing is deleted and the entire batch returns to the queue** after
the visibility timeout. The body is malformed deterministically, so
every redelivery reproduces the same `SyntaxError`. `scaleUp` is never
reached, which means valid messages batched alongside the malformed one
are never processed either — they are blocked as collateral damage.

Two things make it worse:

- `ReportBatchItemFailures` [disables Lambda's scale-down of message
polling when invocations
fail](https://docs.aws.amazon.com/lambda/latest/dg/services-sqs-errorhandling.html),
so the usual concurrency backoff does not apply.
- `redrive_build_queue` defaults to `enabled = false`, so there is no
`maxReceiveCount` cap. The only backstop is
`job_queue_retention_in_seconds`, default 24h.

Verified empirically against `main`: a batch of two valid records plus
one malformed one rejects with `SyntaxError` and `scaleUp` is called
zero times.

### Change

Parse each record defensively. A malformed body is logged and reported
as an individual batch item failure; the rest of the batch proceeds to
`scaleUp` as normal.

The malformed message then exhausts `maxReceiveCount` on its own rather
than taking the batch with it. It cannot be acknowledged and discarded
through the partial-failure mechanism — reporting it is the only way to
single it out — so with the default configuration it expires by
retention rather than moving to a DLQ. Operators who want these captured
should enable `redrive_build_queue`.

### Tests

Four tests covering: the invocation no longer fails, only the malformed
message is reported, valid messages still reach `scaleUp`, and malformed
and rejected messages combine correctly. All four fail against the
current implementation.

### Note on scope

This is distinct from #5129. That PR addresses the `catch` block, where
a non-`ScaleError` returns an empty `batchItemFailures` — which AWS
treats as complete success, so the batch is **deleted**. Opposite
failure mode, different path, and worth keeping separate. I originally
conflated the two in a comment on #5129 and have corrected it there.

Touches the same file as #5129, so whichever lands second will need a
trivial rebase.
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.

3 participants