Repository navigation
Retry Cloudflare gateway timeouts (504, 520–524) (v1.16.1) - #179
Conversation
521 and 523 mean Cloudflare never delivered the request, so every method is retried like a 429. 504, 520, 522 and 524 leave it open whether the API acted, so like a 502 only safe methods are retried. Co-authored-by: Ibrahim Rahhal <ibrahim.rahhal3636@gmail.com>
Co-authored-by: Ibrahim Rahhal <ibrahim.rahhal3636@gmail.com>
corgea-security
left a comment
There was a problem hiding this comment.
Automated review risk: 2/5.
The change consistently classifies 502, 504, and Cloudflare 520–524 as transient gateway errors, retries only safe methods, and adds focused coverage. No actionable issues are evident from the supplied diff.
No critical or high-priority changes were found.
corgea-security
left a comment
There was a problem hiding this comment.
Approved by Dennis: high policy risk and automated risk 2/5.
Co-authored-by: Ibrahim Rahhal <ibrahim.rahhal3636@gmail.com>
There was a problem hiding this comment.
No merge blockers. I checked the retry policy against the call sites and the second commit.
should_retry still retries 429 for every method, and retries 502, 504, and Cloudflare 520–524 only when method.is_safe(). 500, 503, 525, 526, and 530 stay fail-fast. POST/PATCH uploads (start-scan, archive chunk, source, report) go through send_with_retries with the method they actually send, and RetryLoopGuard keeps that from stacking a second schedule, so a 524 on a write is still one attempt. is_transient_error now includes those statuses, so a source upload stops the rest of the walk instead of repeating the same answer per file.
The follow-up that treats 521 and 523 like the other gateway errors (reads only) matches that write rule. The tests cover the status×method matrix, a real GET that rides out two 524s, and POST 524/521 staying at one hit.
Sent by Cursor Automation: pr-flow


Summary
Pipelines are hitting
524 A Timeout Occurred, which the Corgea web server never sends. It's Cloudflare's own status: it delivered the request to the origin, then stopped waiting for an answer. Until now the shared retry policy insrc/utils/api.rsretried only429and502, so a 524 failed the command on the first attempt.This PR treats
504and Cloudflare's520–524as gateway errors, the same as502. They are retried for reads only (GET/HEAD). A POST or PATCH that gets one is sent exactly once, so this PR adds no new retries of writes.429502504,520,521,522,523,524Writes stay excluded because every write the CLI sends creates something. Re-sending
POST /start-scan,POST /scan-uploador the final archivePATCHafter a gateway error could start a second scan, and with a 524 the API was still working on the request when Cloudflare gave up.525,526and530(TLS/cert/DNS config errors) are still not retried, because they don't clear up within the 10s/30s/50s schedule.500and503are unchanged.Other changes:
is_transient_errornow covers the new statuses. When one hits a source file upload incorgea upload, the CLI stops and reports the platform as unavailable instead of re-sending that POST in its per-file loop.1.16.1(Cargo.toml/Cargo.lock).Testing
./harness check: clippy, fmt, strict clippy and all 931 tests pass (on stable Rust 1.98, matching CI'sdtolnay/rust-toolchain@stable).cloudflare_gateway_errors_are_retried_only_for_readscovers the status × method matrix.send_retries_a_cloudflare_timeout_on_a_readchecks a GET against a stub answering 524 twice, then 200 (3 hits, ends OK).send_leaves_a_post_at_one_attempt_on_a_cloudflare_timeoutchecks that a POST that gets a 524 is sent once.send_leaves_a_post_at_one_attempt_when_the_origin_is_downchecks that a POST that gets a 521 is sent once.