Repository navigation
fix(deploy): improve build-watch timeout and cancellation error reporting - #94
Conversation
AI Adoption Report
💰 Cost Breakdown
Total Estimated Cost: $0.0010
Per-commit breakdown (5 commits)
Warning The following commits have no Git AI authorship notes — AI adoption cannot be tracked for these changes.
Powered by Git AI Standard v3.0.0 — authorship data from |
There was a problem hiding this comment.
🟡 Changes recommended
The new ctx.Done() error path in WatchDeployment mislabels cancellations as timeouts and the new behavior isn’t covered by unit tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts the CLI’s global timeout behavior and deployment build-watch error reporting so long-running (10–15m) builds don’t fail client-side before the server-side build deadline is reached.
Changes:
- Increased the root command default timeout from 10m to 16m to exceed the server-side build cap.
- Updated
--timeouthelp text to clarify it is a whole-command deadline (including build watching). - Replaced the generic build-watch timeout error with an actionable, wrapped error that includes
ctx.Err().
File summaries
| File | Description |
|---|---|
vcr/root/root.go |
Raises the global default timeout and clarifies the --timeout flag’s semantics. |
pkg/api/deployment.go |
Improves the error returned when the build watch exits due to ctx.Done(). |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The CLI's global command deadline defaulted to 10m while deployment-api caps builds via BUILDER_ACTIVE_DEADLINE_SECONDS (default 15m). Builds taking 10-15m tripped the client-side deadline and surfaced as "failed to watch deployment ... context exceeds deadline" even though the build was still progressing server-side. - Raise defaultTimeout to 16m so the server-side build limit is always reached first and the CLI reports the real build outcome. - Replace the bare "context exceeds deadline" error with an actionable message that names the package, notes the build may still be running, and wraps ctx.Err(). - Clarify the --timeout flag help text.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The build-wait timeout error path (ctx.Done() in WatchDeployment) is user-visible but was previously untested. Add a table-driven test that drives the watch with a short context deadline and with a cancellation, asserting the exact error message and that it wraps context.DeadlineExceeded and context.Canceled respectively (via errors.Is).
007fb78 to
da4e8ba
Compare
…imeout Address PR #94 review feedback: - Keep the global default --timeout at 10m; users raise it with -t for longer builds. - Remove internal platform details from the timeout rationale and the user-facing error message (public repo). - Distinguish context cancellation from deadline-exceeded in WatchDeployment so cancellations are not mislabelled as timeouts. - Update WatchDeployment ctx.Done() tests to match the new messages.
Replace the generic "context exceeds deadline" error in WatchDeployment with
actionable messages that include the package ID, suggest --timeout, and wrap the
underlying context error. Cancellations and deadline-exceeded are now reported
with distinct messages so users are not misled.
Clarify the --timeout flag help text to explain it is the overall command
deadline (including build time), and document the default in the timeout
constant comment.
Changes:
WatchDeployment, returning a specific message for each case.
waiting for a build to finish (e.g. 20m)".
paths in WatchDeployment.