Skip to content

fix(serverust-events): evitar panic no backoff exponencial do EventRouter#13

Merged
JaimeJunr merged 2 commits into
mainfrom
cursor/critical-correctness-bugs-0b52
May 25, 2026
Merged

fix(serverust-events): evitar panic no backoff exponencial do EventRouter#13
JaimeJunr merged 2 commits into
mainfrom
cursor/critical-correctness-bugs-0b52

Conversation

@cursor
Copy link
Copy Markdown
Contributor

@cursor cursor Bot commented May 20, 2026

Bug e impacto

Com RetryPolicy::Exponential em EventRouter::attach, o atraso entre tentativas era base_delay * 2^exponent. Em Rust, Duration * u32 panic quando o produto excede o máximo representável. Com base_delay grande e expoente até 31 (após muitas falhas), o processo (Lambda/worker) podia ser encerrado de forma abrupta.

Causa raiz

Uso de multiplicação não saturante de std::time::Duration por 2u32.pow(exponent) em wrap_with_retry (serverust-events/src/router.rs).

Correção e validação

  • Introduz exponential_backoff_delay usando Duration::saturating_mul.
  • Testes unitários: saturação em overflow e casos pequenos (1s × 2^0, 1s × 2^2).

Validação: rustup run 1.91 cargo test -p serverust-events e rustup run 1.91 cargo clippy -p serverust-events -- -D warnings.

Open in Web View Automation 

…uter

O cálculo `base_delay * 2^exponent` usava `Duration * u32`, que em overflow
dispara panic. Com `RetryPolicy::Exponential`, `base_delay` grande e
muitas tentativas (expoente até 31), o produto pode exceder o máximo de
`Duration` e derrubar o processo.

Usa `Duration::saturating_mul` via helper e testes de regressão.

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:31
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.

Stale comment

Avaliação de risco (automação)

Nível: Baixo

Evidência (diff): Um ficheiro (serverust-events/src/router.rs): exponential_backoff_delay com saturating_mul para evitar panic por overflow no backoff; testes para saturação e valores pequenos. Sem infra, auth ou alterações alargadas de API.

Revisão obrigatória por política Medium+: Não aplicável.

Aprovação automática: Não foi possível — a API do GitHub recusa aprovação neste contexto (Review Can not approve your own pull request). Risco continua Baixo: um mantenedor pode aprovar/mergear após verificação habitual.

Revisores adicionais: Não solicitados (já existe revisor atribuído; regra Low não exige fila extra).

Open in Web View Automation 

Sent by Cursor Automation: Assign PR reviewers

@claude
Copy link
Copy Markdown

claude Bot commented May 20, 2026

Claude finished @cursor[bot]'s task —— View job


Revisão do PR: fix(serverust-events): evitar panic no backoff exponencial

Tarefas:

  • Ler arquivo modificado (serverust-events/src/router.rs)
  • Verificar aderência às invariantes do CLAUDE.md
  • Analisar correção lógica e edge cases
  • Revisar aspectos de segurança
  • Avaliar cobertura de testes
  • Fornecer comentários inline e resumo final

Análise detalhada

Correção implementada:
A mudança substitui o cálculo direto *base_delay * 2u32.pow(exponent) por uma função helper exponential_backoff_delay que usa Duration::saturating_mul. Isso resolve o panic que ocorria quando o produto excedia os limites de Duration.

Mudanças específicas:

  1. Nova função: exponential_backoff_delay (linhas 66-74) com documentação clara
  2. Uso no retry loop: Substituição na linha 98-99 do cálculo inline pelo helper
  3. Testes: Dois cenários - saturação em overflow e valores pequenos (linhas 352-378)

Comentários inline

📍 serverust-events/src/router.rs:71-73

fn exponential_backoff_delay(base_delay: Duration, attempt_after_first: u32) -> Duration {
    let exponent = attempt_after_first.min(31);
    base_delay.saturating_mul(2u32.pow(exponent))
}

Boa implementação: Limitação do expoente a 31 é correta (2^32 causaria overflow em pow). O uso de saturating_mul resolve o problema original de panic.

📍 serverust-events/src/router.rs:98-100

let delay = exponential_backoff_delay(*base_delay, attempt.saturating_sub(1));

Melhoria: O attempt.saturating_sub(1) é mais seguro que o attempt - 1 original, embora attempt > 0 seja garantido pelo contexto.

📍 serverust-events/src/router.rs:358-364

fn saturates_instead_of_panic_when_product_would_overflow_duration() {
    let base = Duration::new(u64::MAX / 4, 0);
    assert_eq!(
        exponential_backoff_delay(base, 31),
        Duration::MAX,
        "produto base * 2^31 não cabe em Duration; deve saturar"
    );
}

Teste crítico: Valida comportamento em cenário extremo que causaria o panic original.

Verificação de invariantes CLAUDE.md

Sem impacto nas invariantes:

  • Mudança isolada em serverust-events (não afeta serverust-core)
  • Não adiciona dependências externas
  • Não impacta cold start ou tamanho binário do hello-world
  • Melhora robustez sem afetar performance normal

Segurança (OWASP)

Aspectos de segurança:

  • Availability: Fix resolve DoS por panic em cenário de alta carga/retry
  • Resource exhaustion: Saturação previne consumo excessivo de memória
  • No secrets/injection: Mudança puramente matemática, sem dados externos

Cobertura de testes

Adequada mas básica:

  • Teste de saturação em overflow: ✅
  • Teste de valores pequenos: ✅
  • Ausente: Teste de valores intermediários que não saturam

Resumo final

✅ Sem blockers - Aprovação recomendada

Pontos positivos:

  • ✅ Correção cirúrgica e bem fundamentada do panic crítico
  • ✅ Implementação defensiva com limitação de expoente
  • ✅ Testes de regressão para cenário problemático
  • ✅ Aderência completa às invariantes do projeto
  • ✅ Sem impacto em segurança ou performance

⚠️ Sugestões menores:

  • Adicionar teste intermediário (ex: base=10ms, attempt=1010.24s)
  • Considerar log/métrica quando saturação ocorre para observabilidade

📋 Verificações finais recomendadas:

  • cargo test -p serverust-events deve passar
  • cargo clippy -p serverust-events sem warnings

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Corrige um caso de panic por overflow ao calcular o atraso do backoff exponencial no EventRouter (crate serverust-events), evitando encerramento abrupto do processo (ex.: Lambda/worker) em cenários de muitas falhas com base_delay alto.

Changes:

  • Introduz helper exponential_backoff_delay que calcula base * 2^n usando Duration::saturating_mul e limitando o expoente.
  • Atualiza wrap_with_retry para usar o helper na política RetryPolicy::Exponential.
  • Adiciona testes unitários cobrindo saturação em overflow e casos pequenos.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread serverust-events/src/router.rs
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.

Avaliação de risco (automação)

Nível: Baixo

Evidência (diff): Apenas serverust-events/src/router.rs: função exponential_backoff_delay com expoente limitado a 31 e Duration::saturating_mul para evitar panic por overflow no produto base * 2^n; integração no loop de retry existente; testes unitários para saturação e casos pequenos. Sem infra, auth, billing ou mudanças amplas de superfície.

CODEOWNERS: Não aplicável (sem ficheiro no repositório).

Revisores: Não solicitados — política de Medium+ não se aplica.

Aprovação automática: A API do GitHub recusa (Review Can not approve your own pull request). O risco permanece Baixo; um mantenedor humano pode aprovar após verificação habitual.

Open in Web View Automation 

Sent by Cursor Automation: Assign PR reviewers

@JaimeJunr JaimeJunr merged commit 92cdd3a into main May 25, 2026
16 checks passed
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