Skip to content

fix(serverust-events): SqsBroker não faz ack silencioso sem handler/ARN#12

Merged
JaimeJunr merged 1 commit into
mainfrom
cursor/critical-correctness-bugs-f340
May 20, 2026
Merged

fix(serverust-events): SqsBroker não faz ack silencioso sem handler/ARN#12
JaimeJunr merged 1 commit into
mainfrom
cursor/critical-correctness-bugs-f340

Conversation

@cursor
Copy link
Copy Markdown
Contributor

@cursor cursor Bot commented May 19, 2026

Problema

Com Lambda ESM e ReportBatchItemFailures, qualquer registro SQS ausente de batchItemFailures é tratado como sucesso e removido da fila pelo runtime.

SqsBroker::handle_sqs_event ignorava mensagens sem event_source_arn parseável ou sem handler para o nome da fila extraído do ARN, devolvendo batchItemFailures vazio para esses registros. Cenário concreto: typo no subscribe("orders", …) vs fila real order, ou payload sem ARN; a função retornava sucesso e as mensagens eram descartadas sem processamento (perda de dados do ponto de vista da aplicação).

Correção

  • Sem ARN válido ou sem handlers para a fila: se existir messageId, adiciona entrada em batchItemFailures para retry/DLQ.
  • Sem messageId: mantém o limite anterior (só warn); a Lambda pode ainda fazer ack — documentado.

Validação

  • cargo +stable test -p serverust-events --features sqs
  • cargo +stable clippy -p serverust-events --features sqs --all-targets -- -D warnings
  • cargo +stable check --workspace

Changelog

Entrada em [Unreleased] em CHANGELOG.md.

Open in Web View Automation 

… ou ARN

Com ReportBatchItemFailures, ausência em batchItemFailures faz a Lambda
remover a mensagem como sucesso. Registros sem subscriber ou sem ARN
parseável agora entram em batchItemFailures quando há messageId.

Tests atualizados; CHANGELOG [Unreleased].

Co-authored-by: Jaime Basso <JaimeJunr@users.noreply.github.com>
@JaimeJunr JaimeJunr marked this pull request as ready for review May 20, 2026 19:31
Copilot AI review requested due to automatic review settings May 20, 2026 19:32
@JaimeJunr JaimeJunr merged commit d555e9f into main May 20, 2026
15 of 16 checks passed
Copy link
Copy Markdown
Contributor Author

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Risk assessment (automation)

Verdict: Medium–High risk (evidence from diff only).

Why

  • Surface: SqsBroker::handle_sqs_event in serverust-events — Lambda SQS ESM + ReportBatchItemFailures batch handling.
  • Behavior change: Records without a parseable event_source_arn or without a registered handler previously produced no batch_item_failures (implicit success → Lambda could remove messages). They now add batch_item_failures when message_id is present, changing retry/DLQ vs silent drop for misconfiguration paths.
  • Blast radius: Any deployment using this broker with partial batch failure reporting; operational semantics of “success” vs “failure” for edge cases shift (intended correctness fix, still pipeline-level).
  • Evidence: consumer.rs control-flow rewrite + sqs_consumer.rs expectations + CHANGELOG.md.

Review policy: Under the automation rules, Medium+ would require human review (up to 2 reviewers) and would not be eligible for bot self-approval at Medium–High.

Merge state: This pull request is already merged (merged_at present). No reviewer assignment or approval action taken here.

CODEOWNERS: No CODEOWNERS file found in the repository snapshot used for this run.

Open in Web View Automation 

Sent by Cursor Automation: Assign PR reviewers

@cursor cursor Bot review requested due to automatic review settings May 20, 2026 19:54
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