Return repository_not_found for unknown ids in all websocket commands - #5382
Open
frenck wants to merge 1 commit into
Open
Return repository_not_found for unknown ids in all websocket commands#5382frenck wants to merge 1 commit into
frenck wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Standardizes websocket error handling when a client references a repository ID that no longer exists, preventing AttributeError tracebacks from surfacing as generic “Unknown error” responses and avoiding noisy Home Assistant logs.
Changes:
- Introduces a shared
resolve_repository()helper that returns a consistentrepository_not_foundwebsocket error for unknown repository IDs. - Refactors all repository-related websocket commands to use the shared resolver (including
infoandignorewhich previously had bespoke guards). - Adds a parametrized websocket test covering all 12 relevant commands and records API-usage snapshots for each parameterization.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| custom_components/hacs/websocket/helpers.py | Adds resolve_repository() to centralize repository lookup and structured not-found error sending. |
| custom_components/hacs/websocket/repository.py | Updates all single-repository websocket commands to use resolve_repository() and early-return on unknown IDs. |
| custom_components/hacs/websocket/repositories.py | Updates multi-repository websocket commands that accept a repository ID to use resolve_repository(). |
| tests/test_websocket.py | Adds a parametrized test asserting consistent repository_not_found responses across all affected websocket commands. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-info.json | Records API-usage snapshot for the repository/info unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-ignore.json | Records API-usage snapshot for the repository/ignore unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-state.json | Records API-usage snapshot for the repository/state unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-version.json | Records API-usage snapshot for the repository/version unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-beta.json | Records API-usage snapshot for the repository/beta unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-download.json | Records API-usage snapshot for the repository/download unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-remove.json | Records API-usage snapshot for the repository/remove unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-refresh.json | Records API-usage snapshot for the repository/refresh unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-release-notes.json | Records API-usage snapshot for the repository/release_notes unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repository-releases.json | Records API-usage snapshot for the repository/releases unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repositories-clear-new.json | Records API-usage snapshot for the repositories/clear_new unknown-ID case. |
| tests/snapshots/api-usage/tests/test_websockettest-unknown-repository-returns-not-found-repositories-remove.json | Records API-usage snapshot for the repositories/remove unknown-ID case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MartinHjelmare
approved these changes
Aug 25, 2026
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Suggested change
| } | |
| } | |
Comment on lines
+2
to
+8
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repositories-clear_new-]": { | ||
| "https://api.github.com/repos/hacs/integration": 1, | ||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||
| } |
Contributor
There was a problem hiding this comment.
Side note: It's not clear to me what these snapshots show. They seem to repeat a lot of data, which makes me think they're not testing something distinct.
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.
Proposed change
Ten websocket handlers dereferenced
repositories.get_by_id()without aNonecheck (state,version,beta,download,remove,refresh,release_notes,releases,repositories/clear_new, andrepositories/remove). A stale frontend referencing a repository that no longer exists (removed, or renamed) got anAttributeErrorsurfaced as "Unknown error", with a traceback in the Home Assistant log.This adds a shared
resolve_repository()helper that sends a properrepository_not_founderror and makes all repository commands use it, includinginfoandignorewhich each had their own copy of the guard. Error behavior is now identical across all commands.Added a parametrized test that sends all 12 commands with an unknown repository id and asserts the structured error response. 10 of the 12 fail on the previous code.
Type of change
Checklist