From 868412bd31eb4feef47f9ef68ae96f75d3ec22f3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 30 Sep 2026 07:48:52 +0000 Subject: [PATCH 1/3] Retry Cloudflare gateway timeouts (504, 520-524) 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 --- README.md | 34 +++++---- src/scan.rs | 11 +-- src/utils/api.rs | 190 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 176 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 7dd3d6c..a97ceec 100644 --- a/README.md +++ b/README.md @@ -50,8 +50,9 @@ that exits 1 on a blocking rule still leaves its report behind to ingest. ### Rate limits and gateway errors are retried -Neither `429 Too Many Requests` nor `502 Bad Gateway` is Corgea rejecting a -request on its merits, so both are retried rather than failed: the CLI waits +A `429 Too Many Requests`, a `502 Bad Gateway` or `504 Gateway Timeout`, and +Cloudflare's `520`–`524` are not Corgea rejecting a request on its merits, so +all of them are retried rather than failed: the CLI waits 10s, then 30s, then 50s, and a pipeline rides out the blips a busy platform produces under parallel scans instead of failing on them. A request still answered the same way after those three retries fails the command in the usual @@ -61,21 +62,26 @@ three retries again. A `429` that names a `Retry-After` in seconds is honored, up to two minutes for any one pause, and never shortens the pause below the schedule. -Which requests get retried depends on which of the two it is: - -- A `429` retries everything, `POST` and `PATCH` included. The rate limiter - declines the request before the API sees it, so nothing was created and - sending it again finishes the same work. -- A `502` retries reads only. It comes from the proxy rather than from Corgea, - so it is equally the answer for "the request never arrived" and for "the - request was processed and the reply was lost coming back" — and every write - the CLI sends creates something, so re-sending one does not finish the first - scan, it starts a second. A write's 502 goes straight to the caller. +Which requests get retried depends on whether the request could have reached +Corgea: + +- A `429`, `521 Web Server Is Down` or `523 Origin Is Unreachable` retries + everything, `POST` and `PATCH` included. The rate limiter declined the + request, or Cloudflare could not connect to Corgea at all, so nothing was + created and sending it again finishes the same work. +- A `502`, `504`, `520`, `522` or `524 A Timeout Occurred` retries reads only. + Each comes from a proxy rather than from Corgea, so it is equally the answer + for "the request never arrived" and for "the request was processed and the + reply was lost coming back" — a `524` in particular means Corgea took the + request and was still working on it when Cloudflare stopped waiting. Every + write the CLI sends creates something, so re-sending one does not finish the + first scan, it starts a second. A write's gateway error goes straight to the + caller. Writes still retry network errors, where nothing reached Corgea at all. -`corgea upload` treats either status on a source upload — a 502, or a rate limit -that outlived its retries — as the platform being unavailable rather than one bad +`corgea upload` treats any of these on a source upload — a gateway error, or a +rate limit that outlived its retries — as the platform being unavailable rather than one bad file, and reports the remaining paths as unsent instead of collecting the same answer once per path. diff --git a/src/scan.rs b/src/scan.rs index 3823ff8..df3c88c 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -418,11 +418,12 @@ pub fn upload_scan( "Code upload failed with status: {}. Response body: {}", status, body )); - // A 502 or a rate limit that got this far is the - // platform being unavailable, not something wrong with - // this one file: the 502 because an upload is a write - // and so is never replayed, the 429 because its retries - // are already spent. Walking the remaining paths would + // A gateway error or a rate limit that got this far is + // the platform being unavailable, not something wrong + // with this one file: a gateway error such as a 502 or + // 524 because an upload is a write and so is never + // replayed, a 429 or 521 because its retries are + // already spent. Walking the remaining paths would // just collect the same answer once per file, so stop // uploading source files altogether. if utils::api::is_transient_error(status) { diff --git a/src/utils/api.rs b/src/utils/api.rs index a31b547..e2c7b62 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -202,45 +202,80 @@ impl DebugRequestBuilder { } } -/// Statuses the CLI answers with a retry rather than a failure. +/// Cloudflare's own statuses for failing to get an answer from the origin. +/// They have no `StatusCode` constants because no RFC defines them. +const CLOUDFLARE_UNKNOWN_ERROR: u16 = 520; +const CLOUDFLARE_WEB_SERVER_DOWN: u16 = 521; +const CLOUDFLARE_CONNECTION_TIMED_OUT: u16 = 522; +const CLOUDFLARE_ORIGIN_UNREACHABLE: u16 = 523; +const CLOUDFLARE_TIMEOUT_OCCURRED: u16 = 524; + +/// Statuses that say the request never reached the API, so nothing was +/// created and any method can be sent again. /// -/// A `429` is Corgea's rate limiter turning the request away and a `502` is the -/// proxy in front of Corgea failing to get an answer from the API. Neither is -/// the API rejecting the request on its merits, so under the parallel scanning -/// load that produces them a retry succeeds where failing the pipeline would -/// not. -pub fn is_transient_error(status: StatusCode) -> bool { +/// - `429 Too Many Requests` is Corgea's rate limiter declining the request. +/// - `521 Web Server Is Down` is Cloudflare having its connection to the +/// origin refused. +/// - `523 Origin Is Unreachable` is Cloudflare finding no route to the origin. +fn never_reached_api(status: StatusCode) -> bool { + status == StatusCode::TOO_MANY_REQUESTS + || matches!( + status.as_u16(), + CLOUDFLARE_WEB_SERVER_DOWN | CLOUDFLARE_ORIGIN_UNREACHABLE + ) +} + +/// Statuses where a proxy in front of the API gave up waiting for it, which +/// says nothing about whether the API acted: each is equally the answer for +/// "never arrived" and for "was processed, and the reply was lost". +/// +/// - `502 Bad Gateway` and `504 Gateway Timeout` from any proxy. +/// - `520 Unknown Error`: the origin reset the connection or sent back +/// something that was not an HTTP response. +/// - `522 Connection Timed Out`: covers the connection never opening, but also +/// the request going out and never being acknowledged. +/// - `524 A Timeout Occurred`: Cloudflare delivered the request and the origin +/// did not answer within the proxy read timeout (125s by default) — which +/// is the API still working on it, not the API never seeing it. +fn proxy_lost_answer(status: StatusCode) -> bool { matches!( status, - StatusCode::TOO_MANY_REQUESTS | StatusCode::BAD_GATEWAY + StatusCode::BAD_GATEWAY | StatusCode::GATEWAY_TIMEOUT + ) || matches!( + status.as_u16(), + CLOUDFLARE_UNKNOWN_ERROR | CLOUDFLARE_CONNECTION_TIMED_OUT | CLOUDFLARE_TIMEOUT_OCCURRED ) } +/// Statuses the CLI answers with a retry rather than a failure. +/// +/// None of them is the API rejecting the request on its merits — they are the +/// rate limiter, or the proxies between the CLI and the API failing to get an +/// answer — so under the parallel scanning load that produces them a retry +/// succeeds where failing the pipeline would not. +pub fn is_transient_error(status: StatusCode) -> bool { + never_reached_api(status) || proxy_lost_answer(status) +} + /// Whether `status` is worth sending this `method` again. /// -/// Both transient statuses are worth retrying, but they promise different -/// things about what happened to the request, and only one of them is safe to -/// answer by re-sending a write: +/// Every transient status is worth retrying, but they promise different things +/// about what happened to the request, and only some are safe to answer by +/// re-sending a write: /// -/// - `429 Too Many Requests` is the rate limiter declining the request before -/// the API sees it. Nothing was created, so any method can be sent again — -/// which matters most for the writes, since a rate-limited upload that failed -/// the command leaves a pipeline to be re-run by hand. -/// - `502 Bad Gateway` comes from the proxy, so it says nothing about whether -/// the API acted: it is equally the answer for "never arrived" and for "was -/// processed, and the reply was lost coming back". Only a request that changes -/// nothing is safe to send into that ambiguity, and every write the CLI sends -/// creates something. `POST /start-scan` mints a transfer, `POST /scan-upload` -/// takes a whole report, and the archive `PATCH` that fills the last of -/// `Upload-Length` is the one that answers with `scan_id` — on an archive -/// under `CHUNK_SIZE`, the only chunk. Re-sending one of those does not finish -/// the first scan, it starts a second. +/// - A status from `never_reached_api` means nothing was created, so any +/// method can be sent again — which matters most for the writes, since an +/// upload that failed the command leaves a pipeline to be re-run by hand. +/// - A status from `proxy_lost_answer` leaves it open whether the API acted. +/// Only a request that changes nothing is safe to send into that ambiguity, +/// and every write the CLI sends creates something. `POST /start-scan` mints +/// a transfer, `POST /scan-upload` takes a whole report, and the archive +/// `PATCH` that fills the last of `Upload-Length` is the one that answers +/// with `scan_id` — on an archive under `CHUNK_SIZE`, the only chunk. +/// Re-sending one of those does not finish the first scan, it starts a +/// second. fn should_retry(status: StatusCode, method: &Method) -> bool { - match status { - StatusCode::TOO_MANY_REQUESTS => true, - StatusCode::BAD_GATEWAY => method.is_safe(), - _ => false, - } + never_reached_api(status) || (proxy_lost_answer(status) && method.is_safe()) } /// Pauses before each retry: a transient error has to survive four attempts @@ -2982,22 +3017,23 @@ mod tests { Duration::from_secs(50) ] ); - assert!(is_transient_error(StatusCode::BAD_GATEWAY)); - assert!(is_transient_error(StatusCode::TOO_MANY_REQUESTS)); + for code in [429, 502, 504, 520, 521, 522, 523, 524] { + assert!(is_transient_error(status(code)), "{code}"); + } // Everything else is the API answering for itself, including the other // 5xx: those are not what the pipelines are hitting, and replaying an - // upload the server did read is not free. - for status in [ - StatusCode::OK, - StatusCode::UNAUTHORIZED, - StatusCode::NOT_FOUND, - StatusCode::INTERNAL_SERVER_ERROR, - StatusCode::SERVICE_UNAVAILABLE, - ] { - assert!(!is_transient_error(status), "{status}"); + // upload the server did read is not free. 525, 526 and 530 are + // Cloudflare too, but a broken certificate or DNS record does not fix + // itself within the schedule. + for code in [200, 401, 404, 500, 503, 525, 526, 530] { + assert!(!is_transient_error(status(code)), "{code}"); } } + fn status(code: u16) -> StatusCode { + StatusCode::from_u16(code).unwrap() + } + #[test] fn retry_delay_override_falls_back_to_the_schedule_when_unusable() { assert_eq!(parse_retry_delays(None), TRANSIENT_RETRY_DELAYS.to_vec()); @@ -3105,6 +3141,80 @@ mod tests { } } + #[test] + fn cloudflare_retries_writes_only_when_the_origin_never_saw_them() { + // Refused (521) or unroutable (523): the request never left Cloudflare. + for code in [521, 523] { + for method in [Method::GET, Method::POST, Method::PATCH] { + assert!(should_retry(status(code), &method), "{code} {method}"); + } + } + // A gateway timeout, an origin reset (520), a request never + // acknowledged (522), or an origin that took the request and outran the + // read timeout (524): the API may have acted, so only reads go again. + for code in [504, 520, 522, 524] { + for method in [Method::GET, Method::HEAD] { + assert!(should_retry(status(code), &method), "{code} {method}"); + } + for method in [Method::POST, Method::PUT, Method::PATCH, Method::DELETE] { + assert!( + !should_retry(status(code), &method), + "{code} {method} must be sent once" + ); + } + } + for code in [525, 526, 530] { + assert!(!should_retry(status(code), &Method::GET), "{code}"); + } + } + + #[test] + fn send_retries_a_cloudflare_timeout_on_a_read() { + let (base, hits) = spawn_failing_stub( + 2, + "524 A Timeout Occurred", + "", + "error code: 524", + ); + + let response = http_client().get(&base).send().expect("send"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(hits.load(Ordering::SeqCst), 3, "two 524s, then the answer"); + } + + #[test] + fn send_leaves_a_post_at_one_attempt_on_a_cloudflare_timeout() { + // The origin took the request and was still working on it when + // Cloudflare gave up; sending it again would start a second scan. + let (base, hits) = spawn_failing_stub( + usize::MAX, + "524 A Timeout Occurred", + "", + "error code: 524", + ); + + let response = http_client().post(&base).body("{}").send().expect("send"); + + assert_eq!(response.status().as_u16(), 524); + assert_eq!(hits.load(Ordering::SeqCst), 1); + } + + #[test] + fn send_retries_a_post_the_origin_refused() { + let (base, hits) = spawn_failing_stub( + 2, + "521 Web Server Is Down", + "", + "error code: 521", + ); + + let response = http_client().post(&base).body("{}").send().expect("send"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(hits.load(Ordering::SeqCst), 3); + } + #[test] fn the_schedule_stops_after_its_retries_and_any_other_answer_resets_it() { let mut retries = TransientRetries::new("GET /api/v1/scan/s1"); From 4448262016b5c694f87a13ca67d8cdbd07f1fad1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 30 Sep 2026 08:35:56 +0000 Subject: [PATCH 2/3] Retry 521 and 523 for reads only, like the other gateway errors Co-authored-by: Ibrahim Rahhal --- README.md | 27 +++++++-------- src/scan.rs | 4 +-- src/utils/api.rs | 89 ++++++++++++++++++++---------------------------- 3 files changed, 50 insertions(+), 70 deletions(-) diff --git a/README.md b/README.md index a97ceec..4dc95d1 100644 --- a/README.md +++ b/README.md @@ -62,21 +62,18 @@ three retries again. A `429` that names a `Retry-After` in seconds is honored, up to two minutes for any one pause, and never shortens the pause below the schedule. -Which requests get retried depends on whether the request could have reached -Corgea: - -- A `429`, `521 Web Server Is Down` or `523 Origin Is Unreachable` retries - everything, `POST` and `PATCH` included. The rate limiter declined the - request, or Cloudflare could not connect to Corgea at all, so nothing was - created and sending it again finishes the same work. -- A `502`, `504`, `520`, `522` or `524 A Timeout Occurred` retries reads only. - Each comes from a proxy rather than from Corgea, so it is equally the answer - for "the request never arrived" and for "the request was processed and the - reply was lost coming back" — a `524` in particular means Corgea took the - request and was still working on it when Cloudflare stopped waiting. Every - write the CLI sends creates something, so re-sending one does not finish the - first scan, it starts a second. A write's gateway error goes straight to the - caller. +Which requests get retried depends on which it is: + +- A `429` retries everything, `POST` and `PATCH` included. The rate limiter + declines the request before the API sees it, so nothing was created and + sending it again finishes the same work. +- A gateway error — `502`, `504`, or Cloudflare's `520`–`524` — retries reads + only. It comes from a proxy rather than from Corgea, so it is not Corgea's + word on what happened to the request: a `524` in particular means Corgea took + the request and was still working on it when Cloudflare stopped waiting. + Every write the CLI sends creates something, so re-sending one does not + finish the first scan, it starts a second. A write's gateway error goes + straight to the caller. Writes still retry network errors, where nothing reached Corgea at all. diff --git a/src/scan.rs b/src/scan.rs index df3c88c..1e7b0e3 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -422,8 +422,8 @@ pub fn upload_scan( // the platform being unavailable, not something wrong // with this one file: a gateway error such as a 502 or // 524 because an upload is a write and so is never - // replayed, a 429 or 521 because its retries are - // already spent. Walking the remaining paths would + // replayed, the 429 because its retries are already + // spent. Walking the remaining paths would // just collect the same answer once per file, so stop // uploading source files altogether. if utils::api::is_transient_error(status) { diff --git a/src/utils/api.rs b/src/utils/api.rs index e2c7b62..f971130 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -210,72 +210,63 @@ const CLOUDFLARE_CONNECTION_TIMED_OUT: u16 = 522; const CLOUDFLARE_ORIGIN_UNREACHABLE: u16 = 523; const CLOUDFLARE_TIMEOUT_OCCURRED: u16 = 524; -/// Statuses that say the request never reached the API, so nothing was -/// created and any method can be sent again. -/// -/// - `429 Too Many Requests` is Corgea's rate limiter declining the request. -/// - `521 Web Server Is Down` is Cloudflare having its connection to the -/// origin refused. -/// - `523 Origin Is Unreachable` is Cloudflare finding no route to the origin. -fn never_reached_api(status: StatusCode) -> bool { - status == StatusCode::TOO_MANY_REQUESTS - || matches!( - status.as_u16(), - CLOUDFLARE_WEB_SERVER_DOWN | CLOUDFLARE_ORIGIN_UNREACHABLE - ) -} - -/// Statuses where a proxy in front of the API gave up waiting for it, which -/// says nothing about whether the API acted: each is equally the answer for -/// "never arrived" and for "was processed, and the reply was lost". +/// Statuses where a proxy in front of the API failed to get an answer from it. /// /// - `502 Bad Gateway` and `504 Gateway Timeout` from any proxy. /// - `520 Unknown Error`: the origin reset the connection or sent back /// something that was not an HTTP response. +/// - `521 Web Server Is Down`: the origin refused Cloudflare's connection. /// - `522 Connection Timed Out`: covers the connection never opening, but also /// the request going out and never being acknowledged. +/// - `523 Origin Is Unreachable`: Cloudflare found no route to the origin. /// - `524 A Timeout Occurred`: Cloudflare delivered the request and the origin /// did not answer within the proxy read timeout (125s by default) — which /// is the API still working on it, not the API never seeing it. -fn proxy_lost_answer(status: StatusCode) -> bool { +fn is_gateway_error(status: StatusCode) -> bool { matches!( status, StatusCode::BAD_GATEWAY | StatusCode::GATEWAY_TIMEOUT ) || matches!( status.as_u16(), - CLOUDFLARE_UNKNOWN_ERROR | CLOUDFLARE_CONNECTION_TIMED_OUT | CLOUDFLARE_TIMEOUT_OCCURRED + CLOUDFLARE_UNKNOWN_ERROR + | CLOUDFLARE_WEB_SERVER_DOWN + | CLOUDFLARE_CONNECTION_TIMED_OUT + | CLOUDFLARE_ORIGIN_UNREACHABLE + | CLOUDFLARE_TIMEOUT_OCCURRED ) } /// Statuses the CLI answers with a retry rather than a failure. /// -/// None of them is the API rejecting the request on its merits — they are the -/// rate limiter, or the proxies between the CLI and the API failing to get an -/// answer — so under the parallel scanning load that produces them a retry -/// succeeds where failing the pipeline would not. +/// A `429` is Corgea's rate limiter turning the request away and a gateway +/// error is a proxy in front of Corgea failing to get an answer from the API. +/// Neither is the API rejecting the request on its merits, so under the +/// parallel scanning load that produces them a retry succeeds where failing the +/// pipeline would not. pub fn is_transient_error(status: StatusCode) -> bool { - never_reached_api(status) || proxy_lost_answer(status) + status == StatusCode::TOO_MANY_REQUESTS || is_gateway_error(status) } /// Whether `status` is worth sending this `method` again. /// /// Every transient status is worth retrying, but they promise different things -/// about what happened to the request, and only some are safe to answer by +/// about what happened to the request, and only one is safe to answer by /// re-sending a write: /// -/// - A status from `never_reached_api` means nothing was created, so any -/// method can be sent again — which matters most for the writes, since an -/// upload that failed the command leaves a pipeline to be re-run by hand. -/// - A status from `proxy_lost_answer` leaves it open whether the API acted. -/// Only a request that changes nothing is safe to send into that ambiguity, -/// and every write the CLI sends creates something. `POST /start-scan` mints -/// a transfer, `POST /scan-upload` takes a whole report, and the archive -/// `PATCH` that fills the last of `Upload-Length` is the one that answers -/// with `scan_id` — on an archive under `CHUNK_SIZE`, the only chunk. -/// Re-sending one of those does not finish the first scan, it starts a -/// second. +/// - `429 Too Many Requests` is the rate limiter declining the request before +/// the API sees it. Nothing was created, so any method can be sent again — +/// which matters most for the writes, since a rate-limited upload that failed +/// the command leaves a pipeline to be re-run by hand. +/// - A gateway error comes from a proxy, so it is not the API's word on what +/// happened to the request. Only a request that changes nothing is sent +/// again, because every write the CLI sends creates something. +/// `POST /start-scan` mints a transfer, `POST /scan-upload` takes a whole +/// report, and the archive `PATCH` that fills the last of `Upload-Length` is +/// the one that answers with `scan_id` — on an archive under `CHUNK_SIZE`, +/// the only chunk. Re-sending one of those does not finish the first scan, +/// it starts a second. fn should_retry(status: StatusCode, method: &Method) -> bool { - never_reached_api(status) || (proxy_lost_answer(status) && method.is_safe()) + status == StatusCode::TOO_MANY_REQUESTS || (is_gateway_error(status) && method.is_safe()) } /// Pauses before each retry: a transient error has to survive four attempts @@ -3142,17 +3133,9 @@ mod tests { } #[test] - fn cloudflare_retries_writes_only_when_the_origin_never_saw_them() { - // Refused (521) or unroutable (523): the request never left Cloudflare. - for code in [521, 523] { - for method in [Method::GET, Method::POST, Method::PATCH] { - assert!(should_retry(status(code), &method), "{code} {method}"); - } - } - // A gateway timeout, an origin reset (520), a request never - // acknowledged (522), or an origin that took the request and outran the - // read timeout (524): the API may have acted, so only reads go again. - for code in [504, 520, 522, 524] { + fn cloudflare_gateway_errors_are_retried_only_for_reads() { + // A proxy's word on the request, not the API's: only reads go again. + for code in [504, 520, 521, 522, 523, 524] { for method in [Method::GET, Method::HEAD] { assert!(should_retry(status(code), &method), "{code} {method}"); } @@ -3201,9 +3184,9 @@ mod tests { } #[test] - fn send_retries_a_post_the_origin_refused() { + fn send_leaves_a_post_at_one_attempt_when_the_origin_is_down() { let (base, hits) = spawn_failing_stub( - 2, + usize::MAX, "521 Web Server Is Down", "", "error code: 521", @@ -3211,8 +3194,8 @@ mod tests { let response = http_client().post(&base).body("{}").send().expect("send"); - assert_eq!(response.status(), StatusCode::OK); - assert_eq!(hits.load(Ordering::SeqCst), 3); + assert_eq!(response.status().as_u16(), 521); + assert_eq!(hits.load(Ordering::SeqCst), 1); } #[test] From 3ef718e80377664e086e029a99d3b86cd85859b8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 30 Sep 2026 08:54:33 +0000 Subject: [PATCH 3/3] Bump version to 1.16.1 Co-authored-by: Ibrahim Rahhal --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8298322..6fca48f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -369,7 +369,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "corgea" -version = "1.16.0" +version = "1.16.1" dependencies = [ "chrono", "clap", diff --git a/Cargo.toml b/Cargo.toml index 37d77a3..55a82d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "corgea" -version = "1.16.0" +version = "1.16.1" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html