Repository navigation
fix(s3): limitar a leitura do corpo do GetObject pelo prazo da tentativa (#22) - #23
Merged
Merged
Conversation
For objects with a full-object checksum, the AWS SDK wraps the GetObject body in a validating stream that swallows the abort: when the per-attempt AbortSignal.timeout destroyed the socket mid-body, transformToByteArray() never settled, no retry ran, and the read hung forever. The pending promise also pinned the CachedStore in-flight entry and blocked clearCache(), hanging dataset teardown and re-open() of that id. Race the body read against the same per-attempt signal (destroying the body on abort) so `timeout` bounds request and body; the TimeoutError is retryable, so the normal retry policy runs. SDK checksum validation stays on. Closes #22
brandao-andre
requested review from
guidocerqueira
and
a balanced review from Copilot
October 7, 2026 17:16
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation addresses the reported failure mode with appropriate retry behavior and comprehensive regression coverage.
0 open findings
What changed in this PR
Fixes stalled S3 body reads by enforcing one deadline across each request and its body consumption.
Changes:
- Races body reads against the attempt’s abort signal and preserves retries.
- Adds unit and integration coverage for stalled checksummed streams.
- Updates timeout documentation and changelog.
| File | Description |
|---|---|
src/store/s3.ts |
Enforces the deadline while reading S3 response bodies. |
src/store/store.ts |
Clarifies timeout semantics. |
tests/unit/s3-store.test.ts |
Tests body destruction and listener cleanup. |
tests/integration/s3.test.ts |
Tests timeout and retry against a stalled checksummed response. |
CHANGELOG.md |
Documents the resolved hanging-read issue. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
guidocerqueira
approved these changes
Oct 7, 2026
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.
Closes #22
Problema
Para objetos com checksum de objeto inteiro (
x-amz-checksum-crc32,FULL_OBJECT), o SDK (responseChecksumValidation: "WHEN_SUPPORTED", padrão) embrulha o corpo doGetObjectnum stream de validação. Quando oAbortSignal.timeoutda tentativa destrói o socket no meio do corpo, esse stream não repassa o erro:transformToByteArray()nunca resolve nem rejeita, nenhuma retentativa acontece e o GET fica pendurado para sempre.Efeito em cascata: a promessa pendurada prende a entrada de requisição em voo do
CachedStoree bloqueiaclearCache(). Com isso, a limpeza de um dataset removido por LRU nunca termina, e oopen()desse mesmo id,clear()ewhenTornDown()ficam parados até reiniciar o processo (o "só voltou depois de reiniciar" da issue).Correção
S3Store.getObjectcria um únicoAbortSignal.timeoutpor tentativa e usa esse sinal tanto noclient.sendquanto na leitura do corpo (helperreadBody). Quando o prazo estoura, o body é destruído e a tentativa rejeita comTimeoutError, que já é retentável, então a política normal de retry segue. Otimeoutpassa a limitar requisição e corpo; a documentação da opção foi atualizada.A validação de checksum do SDK continua ligada. Avaliamos desligá-la (
WHEN_REQUIRED, sugestão da issue). Também resolve, mas não é necessário e tem pouco ganho. Medição local, objeto de 5 MB:WHEN_SUPPORTED(valida)WHEN_REQUIREDOs números de CPU incluem o servidor de teste, que roda no mesmo processo; só a diferença entre eles importa. São ~2 ms por chunk com
zlib.crc32nativo (Node ≥ 20.15/22.2), cerca de 1% do p50 de S3 dentro da AWS. Em troca, mantemos a detecção de corrupção, que importa sobretudo para chunks sem compressão.Testes
x-amz-checksum-crc32que para depois de 1 KB.onRetry.readBody.npm test(417 ok),npm run lint,npm run format:check,tsc --noEmit.Fora do escopo
HTTPStore: lê o corpo fora doexecuteWithRetry. Um corpo estancado ali falha no prazo (não trava), mas não é retentado. Merece uma issue separada.S3Store(sugestão 3 da issue).