Surface malformed plugin index errors#197
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces structured error handling for malformed plugin marketplace index JSON. Adds ChangesMarketplace index parse error surfacing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ManageCmd as "/plugins add"
participant Market as market.rv
participant Index as "Marketplace Index"
User->>ManageCmd: /plugins add <name>
ManageCmd->>Market: resolve_name_result(name)
Market->>Index: fetch index text
Index-->>Market: raw response body
Market->>Market: parse_index_result(text)
alt JSON malformed
Market-->>ManageCmd: MarketLookup{url:"", error:"could not parse index"}
ManageCmd-->>User: return error message
else JSON valid
Market-->>ManageCmd: MarketLookup{url, error:""}
ManageCmd-->>User: proceed with url
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/market.rv (1)
76-88: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHTTP errors are silently swallowed in
resolve_name_result.Both the
status_code >= 400branch (line 81) and theErr(e)branch (line 86) returnMarketLookup { url: "", error: "" }, causing the caller inmanage.rvto report "no plugin '${first}' in the index" — misleading when the index was unreachable.browse_textsurfaces HTTP errors (line 64), so this is an inconsistency. Consider populatingerrorfor these cases as well, or at minimum for network failures.♻️ Suggested improvement
fun resolve_name_result(name: String) -> MarketLookup { let headers = "User-Agent: rook/1.0\nAccept: application/json" return match request_with_timeout("GET", market_url(), "", headers, TIMEOUT_MS) { Ok(resp) -> { if resp.status_code >= 400 { - MarketLookup { url: "", error: "" } + MarketLookup { url: "", error: "error: the plugin index returned ${resp.status_code}" } } else { resolve_name_in_index(resp.body, name) } }, - Err(e) -> MarketLookup { url: "", error: "" }, + Err(e) -> MarketLookup { url: "", error: "error: could not reach the plugin index (${market_url()}): ${e.message}" }, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/market.rv` around lines 76 - 88, resolve_name_result is swallowing HTTP and network failures by returning an empty MarketLookup in both the non-2xx and Err paths. Update resolve_name_result to populate the error field with the response status/message or the request failure details instead of leaving it blank, so callers like manage.rv can distinguish “not found” from “index unreachable”; keep the same handling consistent with browse_text and use request_with_timeout/resolve_name_in_index/MarketLookup as the main touchpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/plugins/market.rv`:
- Around line 76-88: resolve_name_result is swallowing HTTP and network failures
by returning an empty MarketLookup in both the non-2xx and Err paths. Update
resolve_name_result to populate the error field with the response status/message
or the request failure details instead of leaving it blank, so callers like
manage.rv can distinguish “not found” from “index unreachable”; keep the same
handling consistent with browse_text and use
request_with_timeout/resolve_name_in_index/MarketLookup as the main touchpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e2a52ac-bd00-406a-8a30-fcdc4a54be33
📒 Files selected for processing (5)
CHANGELOG.mdrv.tomlsrc/plugins/manage.rvsrc/plugins/market.rvsrc/plugins/market_test.rv
Fixes #178.
Summary:
/plugins browse./plugins add <name>separately from missing plugin names.no plugins foundand add pure parser and lookup tests.Checks:
Summary by CodeRabbit
Bug Fixes
Chores
0.3.17.