Report download failures, add more fields to report - #36
Conversation
e99e7d0 to
07f7771
Compare
07f7771 to
d500bca
Compare
ofalvai
changed the title
| console.log("Application downloading the package."); | ||
| res.download(exports.updatePackagePath); | ||
| }); | ||
| app.post("/v0.1/public/codepush/report_status/download", function (req, res) { |
There was a problem hiding this comment.
Added this handler so that the requests show up in test logs.
There was a problem hiding this comment.
Pull request overview
Adds download outcome telemetry, including failures and additional package metrics.
Changes:
- Adds download status types and report fields.
- Reports successful and failed downloads with duration.
- Updates SDK fixtures and the test server endpoint.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/acquisition-sdk/types.ts |
Extends download report types. |
src/acquisition-sdk/acquisition-sdk.ts |
Builds enriched download reports. |
src/acquisition-sdk/__tests__/acquisition-sdk.test.ts |
Supplies required status values. |
package-mixins.js |
Reports download outcomes and duration. |
code-push-plugin-testing-framework/script/serverUtil.js |
Accepts download status reports. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const downloadStartTime = Date.now(); | ||
| const reportDownloadStatus = async (status) => { | ||
| if (!reportStatusDownload) return; | ||
| const downloadDurationMs = Date.now() - downloadStartTime; |
There was a problem hiding this comment.
download_duration_ms gets sent on both outcomes, but it means two different things:
On success → how long a complete download took.
On failure → how long until it broke (which could be a fraction of a second if it failed instantly, or a long time if it timed out).
Both values land in the same download_duration_ms field. So if someone later charts "average download duration" without first filtering by status, they'd be averaging complete-download times together with time-to-failure — two unrelated things — and get a misleading number.
There was a problem hiding this comment.
This wasn't intentional, thanks for catching it. I'll remove it from the failure case to avoid confusion, we can re-add this later if needed.
d500bca to
63eb0f2
Compare
Why
report_status/downloadis only called after a successful download at the moment. It's important to track download failures as well, not just deployment failures.What
Call
report_status/downloadafter failures, and introduce astatusfield.Also add a few more useful fields to the report body that's easily available.