Clarify maintained app download timeout errors - #50104
Conversation
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Improves the user-facing error experience when adding Fleet-maintained apps by translating large-installer download timeouts/cancellations into clearer, actionable messages (especially for proxy/load balancer timeouts).
Changes:
- Translate
context.DeadlineExceededandcontext.Cancelederrors from Fleet-maintained app downloads into dedicated Gateway Timeout messages. - Add backend and frontend tests to ensure timeout/cancel scenarios show the friendly timeout copy (including 499 handling on the frontend).
- Refresh the shared frontend “request timed out” copy to better reference proxies/load balancers and large installer transfers.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/service/maintained_apps.go | Adds error translation for timeout/canceled download failures when adding Fleet-maintained apps. |
| server/service/maintained_apps_test.go | Adds unit test coverage for the endpoint error translation behavior. |
| server/fleet/errors.go | Introduces backend constants for clearer maintained-app timeout/cancel messages. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppDetailsPage/helpers.tsx | Treats 499/408/504 uniformly as timeout/cancel and shows friendly timeout copy. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppDetailsPage/helpers.tests.tsx | Adds tests asserting friendly timeout messaging for 504/499/408 paths. |
| frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx | Updates the shared timeout message text shown in the UI. |
| changes/48416-maintained-app-timeout-error-message | User-visible changelog entry (content not available in this review). |
Files excluded by content exclusion policy (1)
- changes/48416-maintained-app-timeout-error-message
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50104 +/- ##
==========================================
+ Coverage 67.98% 68.06% +0.08%
==========================================
Files 3923 3932 +9
Lines 250102 250550 +448
Branches 13361 13276 -85
==========================================
+ Hits 170022 170543 +521
+ Misses 64779 64694 -85
- Partials 15301 15313 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/48416-maintained-app-timeout-error-message
Comments suppressed due to low confidence (2)
frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx:5
- Grammar: the subject is plural ("your server, and any proxy or load balancer"), so the verb should be "allow" instead of "allows".
export const REQUEST_TIMEOUT_ERROR_MESSAGE = `${ADD_SOFTWARE_ERROR_PREFIX} The request timed out. Make sure your server, and any proxy or load balancer in front of Fleet, allows enough time to transfer large installers.`;
frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppDetailsPage/helpers.tsx:62
- The inline comment says 499 is an upstream proxy/LB cancel, but Fleet uses 499 to mean the client disconnected (nginx "Client Closed Request" convention). Clarifying the comment will avoid confusion when debugging.
const isTimeout =
responseStatus === 504 || responseStatus === 408 || responseStatus === 499; // upstream proxy/LB canceled the request
const reason = getErrorReason(err);
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
WalkthroughUpdated Fleet-maintained app installation error handling for installer download timeouts and cancellations. The server now returns distinct gateway timeout messages for deadline exceeded and canceled contexts. The frontend recognizes status codes 504, 499, and 408, and displays a more specific timeout message referencing server, proxy, and load balancer transfer timeouts. Tests cover the new server mappings and frontend responses. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.45.0)server/service/integration_enterprise_test.goast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Related issue: Resolves #48416
When adding a Fleet-maintained app, a large-installer download that's canceled or times out now returns a clear message pointing at the likely proxy/load-balancer timeout, instead of a raw
context canceled.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit