Skip to content

fix(canary): give the dry-run a timeout a real deploy can finish inside - #122

Merged
sgardoll merged 1 commit into
mainfrom
fix/canary-timeout
Oct 4, 2026
Merged

sgardoll merged 1 commit into
mainfrom
fix/canary-timeout

Conversation

@sgardoll

@sgardoll sgardoll commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

The bug

The canary bounded its request to the runner at 60s. A real deploy takes 103-215s on the production runner, and the canary drives the same path — so every scheduled run would have failed with request ... failed: timed out.

A daily job that always fails is worse than no job: it trains you to ignore it, and the real failure it exists to catch looks identical.

Verified against the live runner

CANARY_TIMEOUT_MS=60000 (the old default)  -> FAIL: request ... failed: timed out
CANARY_TIMEOUT_MS=400000                   -> OK: dry-run accepted (success:true)
                                              health: HTTP 200 {"sha":"9906bfb",...}

The fix

Default is now 300s — above the slowest observed deploy (215s), below Cloud Run's 900s request cap. CANARY_TIMEOUT_MS still overrides it.

Found by actually running the canary against production rather than trusting that it had been reviewed.


Devin Review

The canary bounded its request at 60s. A real deploy takes 103-215s, and the
canary drives that same path, so every scheduled run would have failed with
"timed out" - a daily false alarm, which is worse than no canary because it
trains you to ignore it.

300s sits above the slowest observed deploy and below Cloud Run's 900s request
cap. Verified against the live runner: at 60s it fails, at 400s it reports OK
and reads the health route.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@sgardoll
sgardoll merged commit e5ace5e into main Oct 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant