Retry 502 Bad Gateway on every API call instead of failing the pipeline

[Ibrahimrahhal]

Why

A customer running many parallel scans intermittently gets 502 Bad Gateway from the proxy in front of Corgea, and the CLI turned each one straight into a failed command — sometimes as an opaque parse error, since a gateway's HTML body is not the JSON envelope the endpoints read. Their workaround, wrapping the CLI in shell retries on non-zero exit, cannot tell a load blip from a scan that genuinely failed. They asked for the CLI itself to absorb the 502 and to know when to give up.

What changed

Every Corgea API request now replays itself when the answer is 502, waiting 10s, then 30s, then 50s. A request still answered 502 on the fourth attempt fails the command exactly as it does today, so a real outage still exits non-zero. The retry count belongs to a single request: any successful call starts the next one with the full three retries again.

The retry sits in the API service's send path (DebugRequestBuilder::send), so it covers every endpoint rather than a hand-picked list. Details worth review:

• Streamed multipart bodies (the BLAST archive chunks, the source and report uploads) cannot be replayed from a built request, so those call sites go through send_with_retries, which rebuilds the form per attempt. A thread-local guard keeps the inner and outer loops from stacking into 16 attempts and six minutes of sleeping.
• Only 502 is replayed. The other 5xx are the API answering for itself, and replaying an upload the server did read is not free — extending the set is a one-line change to is_gateway_error.
• The issue listing now reads the status before the body, so an exhausted 502 reports Request failed with status: 502 Bad Gateway instead of Failed to parse response: expected value.
• DEBUG_CORGEA_OVERRIDE_RETRY_DELAYS_MS compresses the schedule to milliseconds for the e2e tests; an unusable value falls back to the real schedule rather than dropping the retries.

corgea upload stops walking source files once the gateway is gone

corgea upload uploads each source path referenced by the report, with its own three-attempt loop per file. An exhausted gateway retry now breaks out of the whole walk, not just that file's inner loop: a 502 that outlived 90 seconds of retries is the platform being unavailable, not one bad file, so the remaining paths would each have spent another full schedule. With three referenced files that was 12 uploads and about four and a half minutes; it is now four uploads and 90 seconds.

The paths that were never attempted are counted as unsent, so the closing summary cannot report a single failure for a whole skipped tree. The command still exits non-zero here because no file was uploaded at all.

Scan status is polled every 3 seconds, not every second

If these 502s are load related, the wait loop is a plausible contributor: wait_for_scan re-read the scan status on a fixed 1-second sleep, so one waiting scan issued about 60 requests a minute, and against the default 10-hour budget a single wait could reach roughly 36,000 requests to /api/v1/scan/<id>. A pipeline scanning in parallel multiplies that by every concurrent wait.

The sleep is now a named SCAN_POLL_INTERVAL of 3 seconds, cutting that volume to a third. Scans run for minutes, so the cost is at most a couple of seconds of extra latency on noticing the final status. There is no backoff or jitter, as before — the interval is a plain sleep, so the real cadence is 3 seconds plus the round trip.

Two existing wait tests used a 3-second timeout budget, which now exactly equals one poll interval — they would have expired before issuing a single poll and passed without exercising the loop they were written for. Their budgets are raised (to 7s and 5s), and both now assert that more than one scan read reached the stub, so this class of silent decoupling fails loudly instead.

On retrying non-idempotent writes

The automated security review flags that a 502 can be produced after the upstream committed a request, so replaying a POST could duplicate it. Worth recording where the exposure actually is, since most of these writes already carry a stable key:

• /scan-upload, /code-upload and /git-config-upload all carry a client-generated run_id (a UUID minted once per corgea upload run) that is identical across retries, and /code-upload also carries the file path. The server has everything it needs to dedupe a replay.
• BLAST archive chunks are PATCH /start-scan/<transfer_id>/ with Upload-Offset and Upload-Length, so replaying a chunk at the same offset is idempotent by construction.
• Scan status polling and issue listing are GET.
• The one unkeyed create is POST /start-scan, which mints a transfer_id. The scan itself is created by the final chunk PATCH, not by this call, so a duplicated POST leaves an unused transfer rather than a duplicate scan or duplicate findings.

So the CLI-side fix the review suggests — restrict retries to idempotent methods — would remove exactly the resilience this PR was asked for, and the residual risk is narrow. What is genuinely open is server-side: whether the backend dedupes on run_id, and whether orphaned transfers are collected. Both are platform questions rather than CLI ones, and neither is a reason to hold this change.

Version

Cargo.toml goes from 1.13.0 to 1.14.0 — a minor bump, since this changes how every command behaves against a flaky gateway without breaking any existing usage. Per RELEASING.md that manifest is the only manual edit: PyPI reads it through maturin's dynamic = ["version"], and npm takes its version from the release tag, so package.json stays on its 0.0.0 placeholder. Tag the release v1.14.0 so all three agree.

Evidence

The real CLI against a stub gateway, on the production schedule with no overrides. A gateway that recovers after two 502s completes the scan at t+40s and exits 0; one that never recovers gives up at t+90s and exits 1 after exactly four attempts.

gateway_502_retry_demo.log

Reset-on-success, same schedule: four 502s in one command — three of them against one request — and the command still exits 0, because the successful poll in the middle put the full three retries back.

gateway_502_retry_reset_after_success.log

The source-upload walk stopping, on the production schedule with a report referencing three files: four uploads against the first file, 90 seconds, 3 of 3 files were not sent, exit 1.

source_upload_stops_on_gateway_outage.log

The poll cadence, measured through the real binary: gaps of 3.002s against an instant stub and 3.253s once the stub takes 250ms to answer, which shows the interval is a plain sleep added to the round trip rather than a fixed rate.

scan_poll_interval_3s.log

13 new tests: 5 end-to-end through the real binary against the ordered API stub (which rejects unexpected requests, so they pin the exact attempt count and not just the outcome), 7 unit tests on the retry schedule, and 1 on the poll interval. Each was checked against the unfixed code and fails there.

gateway_502_retry_tests.log

The bumped manifest, and the built binary reporting the new version:

version_bump.log

./harness ci is green: strict clippy, format check, dep audit, 826 tests and the coverage gate.

_{To show artifacts inline, enable in settings.}

  

[juangaitanv]

🚀

[corgea-security]

Automated review risk: 4/5.

Retries improve resilience but can duplicate non-idempotent operations when the upstream processes a request before the proxy returns 502.

Critical or high-priority changes must be addressed.

Automatic approval was not submitted: automated review found critical or high-priority findings.

[cursor]

Stale comment

The 502 schedule is wired through send / send_with_retries correctly for cloneable bodies, multipart rebuilds, and the inner per-file attempt loop. One production hole remains: after those retries are spent, source upload still starts a fresh 90s schedule for every remaining path in the report.

  

_{Sent by Cursor Automation: pr-flow}

[corgea-security]

high: Retrying the start-scan POST can create duplicate scans

A 502 does not guarantee the upstream server rejected the request; it may have committed the operation before the gateway failed to relay its response. Retrying POST /start-scan without an idempotency key can therefore create multiple scans or upload sessions. The tests only model a gateway rejecting the request before processing it, so they do not cover this ambiguity. Restrict automatic retries to idempotent operations or provide a stable idempotency key understood by the API.

Proof or reproduction:

Server behavior demonstrating the problem:

attempt 1: create_scan(); return 502;
attempt 2: create_scan(); return 200;
assert_eq!(created_scans, 2);

The new call retries this POST through send_with_retries:
let response_object = send_with_retries("the scan start request", || {
    client.post(format!("{}{}/start-scan", url, API_BASE))
        .multipart(form)
        .send()
});

[corgea-security]

high: Gateway backoff ignores the scan timeout budget

wait_for_scan limits each HTTP request to its remaining WaitBudget, but DebugRequestBuilder::send performs 10s, 30s, and 50s sleeps internally after 502 responses. Those sleeps are outside the request timeout and receive no remaining-budget value. For example, a scan with CORGEA_SCAN_TIMEOUT_SECONDS=5 can enter a request after the 3-second poll delay and then sleep for 10 seconds on its first 502, already exceeding the configured deadline; persistent failures can add the full 90-second backoff. Retry delays must be capped by or charged against the caller's deadline.

Proof or reproduction:

// A 5-second scan budget has roughly 2 seconds left after polling sleeps.
// An immediate 502 then executes an unconditional 10-second retry delay:
if retries.wait_for_retry(response.status()) {
    // wait_before_retry(Duration::from_secs(10))
}
// elapsed > configured 5-second timeout

[corgea-security]

Automated review risk: 4/5.

The retry mechanism can duplicate non-idempotent operations and can exceed user-configured scan deadlines.

Critical or high-priority changes must be addressed.

Automatic approval was not submitted: automated review found critical or high-priority findings.

[cursor]

No remaining merge blockers. The source-walk hole is closed: upload_scan labels the file loop, sets gateway_gave_up, and break 'files after the 502 schedule is spent, then counts every skipped path as unsent. an_exhausted_gateway_stops_the_whole_source_upload_walk pins that with two referenced sources and exactly four /code-upload hits.

Re-checked the rest of the retry surface against main at 6cff32b:

• send replays cloneable 502s; streamed multipart returns immediately and is rebuilt through send_with_retries. The thread-local guard keeps the two loops from stacking (unit test still expects four attempts, not sixteen).
• Source, report, git-config, /start-scan, and archive-chunk uploads all go through send_with_retries. The inner while attempts < 3 does not re-spend the schedule on a 502.
• After the walk stops they still try the report upload once. A persistent 502 there sets upload_failed and exits 1; a recovered report with missing sources is the same skip-and-warn as any other per-file failure.
• Issue listing reads status before the body, so an exhausted 502 is Request failed with status: 502 Bad Gateway, not a JSON parse error.
• wait_for_scan sleeps SCAN_POLL_INTERVAL (3s). The two wait tests that used a 3s budget now use 7s/5s and assert more than one scan read, so they cannot pass without entering the poll loop. corgea wait still does the initial get_scan before that loop, so the stalling-read case still reaches the second GET.
• e2e covers: wait rides out two 502s; wait exits 1 after four; one-file source upload stops at four; BLAST chunk PATCH is rebuilt after one 502. ARCHIVE_UPLOAD = 4 matches blast_upload_plan (verify + two baselines + start-scan + chunk).

The security notes on POST /start-scan (orphaned transfer_id; scan is created by the final PATCH) and retry sleeps sitting outside a short CORGEA_SCAN_TIMEOUT_SECONDS are residual and not reasons to hold this.

  

_{Sent by Cursor Automation: pr-flow}