Repository navigation
fix(skill-store): bound discovery requests with an HTTP timeout - #459
Open
polylane[bot] wants to merge 1 commit into
Open
polylane[bot] wants to merge 1 commit into
polylane[bot] wants to merge 1 commit into
Conversation
Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
Contributor
Author
|
Warning Polylane could not verify the production impact of this pull request. The added 8 s bound matches the 8 s bound this crate already puts on the same upstream's background fetches, and 72 h of errors and spans for the four skill-studio Sentry projects returned empty. I found no regression path. Also considered · 2 refuted
Analysed against 1 integration and 1 repository
Polylane has no cloud accounts connected for this workspace, so it cannot see what this repository deploys to. Connect a cloud and every pull request will be checked against your production resources. Polylane analysed Did this help? React 👍 or 👎 so the next review is sharper. |
Contributor
Author
Suggested observability improvements
|
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.
Fixes: Skill Store discovery requests have no HTTP timeout
Skill Store search, the popular list, and skill details load over an HTTP client that had no request timeout. When the upstream accepts the connection and then never answers, the request never returns, so those screens stay busy and show no error. After this change each request gives up after eight seconds and reports a normal connection error, so the store can recover.
Detection evidence
Affected:
int_0c37b2126001vkbz84c78wfnWhy this fix
The client behind every Skill Store discovery request was built with no timeout, while the same crate bounds the identical background fetch at eight seconds for the stated reason that a hung connection must not hold a lookup open. A server that accepts a connection and never replies reproduced the hang: without the change the request never returns, with it the request fails at eight seconds. The eight-second bound matches the existing background fetches, and those responses are short JSON, so it does not cut off a healthy response.
Fix walkthrough
Watch the fix explained (1:06). Recorded for commit
482f88e7555242e64ed24afa3798e9f0222ea98c.Out of scope
github_skill_listing.rs#HttpGithubApi::get_jsonalso builds a client with no timeout, but it serves the GitHub-skill listing flow, not Skill Store discovery. Recorded as a follow-up on the issue.skill_install_countsfetches already carry their own bound, so they were left as they are.Causal chain
1 file changed (+64/-2)
apps/desktop/src-tauri/src/skills/api.rs: modified, +64/-2Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.