Stop waiting forever on scans that already ended - #144
Conversation
There was a problem hiding this comment.
Automated review risk: 4/5.
The core failure-reporting path breaks when the API returns scan_errors as null. The scan-details fallback URL also mishandles project names containing slashes.
Critical or high-priority changes must be addressed.
Automatic approval was not submitted: automated review found critical or high-priority findings.
There was a problem hiding this comment.
if we check for all scan status why do we need CORGEA_SCAN_TIMEOUT_SECONDS? The issue with this is that we have scans that can take 10 hours or more for big clients. maybe we can set it as 10hours for now and document that clients can override that in the docs
done, it's set to 10 hours, and documentation updates in https://github.com/Corgea/docs/pull/303 |
There was a problem hiding this comment.
Automated review risk: 2/5.
The diff addresses the existing timeout, null deserialization, and URL-encoding comments with targeted tests. No actionable critical, high-priority, or nitpick findings remain in the supplied diff.
No critical or high-priority changes were found.
Automatic approval was not submitted: changes have been requested.
|
|
||
| let result = loop { | ||
| thread::sleep(Duration::from_secs(1)); | ||
| match utils::api::get_scan(&config.get_url(), scan_id) { |
There was a problem hiding this comment.
a stalled status read can exceed CORGEA_SCAN_TIMEOUT_SECONDS because requests use the 150-second client timeout; could we start one deadline before the initial read and cap requests to its remaining time?
corgea scan polled until
status == "complete", so a scan that ended as incomplete was polled forever — the CI job hung until its own timeout with no indication anything had failed.completed/failed/running, withincomplete,failed,error, andcancelledall terminal, matched case-insensitively (corgea wait compared against "Complete" and never matched).CORGEA_SCAN_TIMEOUT_SECONDSoverrides) rather than burning a job's whole budget on a scan that never reaches a terminal state.corgea scan,corgea wait, andcorgea upload --waitall share this path.failed_reasonandscan_errorsare optional, so responses from servers without the doghouse change still parse and behave as before.Merge order: doghouse first (the API fields the CLI reads), then fusion (writes the messages doghouse sanitizes), then cli. The CLI tolerates servers without the new fields, so the order is about when the output becomes useful, not about breakage.
Linear ticket
Test results
Before fix
After