fix: surface unmapped platform combinations instead of dropping silently - #72
Merged
Conversation
filenameToPlatform's composite-id lookup returned undefined for discrete parts with no modeled id (a 'universal' token on linux or non-msix windows assets), so ingestion skipped those assets without logging anything and the function violated its declared return type. Throw a descriptive error instead: ingestion already logs detection errors per asset, and the /download/:tag/:filename route maps detection failures to a 400 (previously a 500 for unparseable filenames). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM
There was a problem hiding this comment.
Pull request overview
This PR tightens platform parsing by making unsupported filename-derived platform combinations explicit errors (instead of returning undefined), and then translating those parse failures into visible drops during ingestion and a client-facing 400 on the download-by-filename route.
Changes:
filenameToPlatformnow throwsUnrecognized platform combination (...)when discrete parts don’t map to a modeled compositePlatform./download/:tag/:filenamemaps filename platform-parse failures toBadRequestError(400) instead of surfacing as a 500.- Adds unit coverage for unmapped combinations and an integration test asserting the 400 behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/unit/platforms.spec.ts | Adds assertions that unmapped discrete parts now throw a descriptive error. |
| test/integration/download.spec.ts | Verifies invalid filename-to-platform parsing returns HTTP 400 on download-by-filename route. |
| src/utils/platforms.ts | Converts missing composite platform lookup from undefined to a thrown, descriptive error. |
| src/http/downloads.ts | Wraps filename-derived platform parsing errors as BadRequestError to return 400. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
filenameToPlatform's composite-id lookup returnedundefinedfor discrete parts that don't combine into a modeled id — e.g. auniversaltoken on a linux asset (myapp-universal.tar.gz→linux_universal) or a non-msix windows asset (myapp-win64-universal.zip→windows_universal). Ingestion then skipped those assets without logging anything, and the function violated its declaredPlatformreturn type.Changes:
filenameToPlatformnow throws a descriptiveUnrecognized platform combination (...)error for unmapped combos;normalizeRelease's existing per-asset try/catch logs the drop instead of skipping silently./download/:tag/:filename, filename detection failures are mapped to a 400 (BadRequestError) — previously an unparseable filename surfaced as a 500.Includes unit tests for the throwing combos and an integration test for the 400.
From the 2.0 release review pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM
Generated by Claude Code