From c22f17db53b9fb05cc869c3b0ba1e5bec43f91c0 Mon Sep 17 00:00:00 2001 From: martian56 Date: Thu, 9 Jul 2026 00:16:21 +0400 Subject: [PATCH 1/2] fix(plugins): surface malformed index errors --- CHANGELOG.md | 4 ++ rv.toml | 2 +- src/plugins/manage.rv | 8 +++- src/plugins/market.rv | 93 ++++++++++++++++++++++++++++---------- src/plugins/market_test.rv | 26 ++++++++++- 5 files changed, 104 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0794ff7..26d4a7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.3.17 + +- Surfaced malformed plugin marketplace index JSON instead of treating parse failures as an empty index. + ## 0.3.16 - Reported plugin removal failures when recursive deletion commands fail or the plugin directory remains on disk. diff --git a/rv.toml b/rv.toml index 8eacfdc..82c0e30 100644 --- a/rv.toml +++ b/rv.toml @@ -1,6 +1,6 @@ [package] name = "rook" -version = "0.3.16" +version = "0.3.17" edition = "v2" [dependencies] diff --git a/src/plugins/manage.rv b/src/plugins/manage.rv index ebfb3cc..a3359a8 100644 --- a/src/plugins/manage.rv +++ b/src/plugins/manage.rv @@ -9,7 +9,7 @@ import std/fs { is_dir, list_dir } import std/process { run, Output } import "./model" { Plugin, installed_plugins, plugin_path, valid_plugin_name } import "./install" { install_plugin, plugin_source } -import "./market" { browse_text, resolve_name } +import "./market" { browse_text, resolve_name_result } // Dispatch a /plugins invocation. The argument is what followed the command // ("" or "list", "add [tag]", "remove ", "update "). @@ -106,7 +106,11 @@ fun add(rest: String) -> String { // anything with a slash or scheme is treated as a git URL directly. let url = first if !_is_url(first) { - url = resolve_name(first) + let resolved = resolve_name_result(first) + if resolved.error != "" { + return resolved.error + } + url = resolved.url if url == "" { return "no plugin '${first}' in the index; give a git URL, or try /plugins browse" } diff --git a/src/plugins/market.rv b/src/plugins/market.rv index 9e9c59e..200c130 100644 --- a/src/plugins/market.rv +++ b/src/plugins/market.rv @@ -20,6 +20,16 @@ struct MarketEntry { url: String, } +struct MarketIndex { + entries: List, + error: String, +} + +struct MarketLookup { + url: String, + error: String, +} + // The index URL: ROOK_PLUGIN_INDEX when set, else the default. fun market_url() -> String { if has_env("ROOK_PLUGIN_INDEX") { @@ -39,7 +49,11 @@ fun browse_text(query: String) -> String { if resp.status_code >= 400 { "error: the plugin index returned ${resp.status_code}" } else { - let entries = filter_entries(parse_index(resp.body), query) + let parsed = parse_index_result(resp.body) + if parsed.error != "" { + return _index_error(parsed.error) + } + let entries = filter_entries(parsed.entries, query) if entries.len() == 0 { "no plugins found in the index${_suffix(query)}" } else { @@ -54,45 +68,70 @@ fun browse_text(query: String) -> String { // The git URL for a marketplace plugin by name, or "" when the index has no // such name or cannot be reached. Lets /plugins add take a bare name. fun resolve_name(name: String) -> String { + return resolve_name_result(name).url +} + +// The git URL for a marketplace plugin by name, plus an index parse error +// when the index response cannot be understood. +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: "" } } else { - resolve_in(parse_index(resp.body), name) + resolve_name_in_index(resp.body, name) } }, - Err(e) -> "", + Err(e) -> MarketLookup { url: "", error: "" }, + } +} + +fun resolve_name_in_index(text: String, name: String) -> MarketLookup { + let parsed = parse_index_result(text) + if parsed.error != "" { + return MarketLookup { url: "", error: _index_error(parsed.error) } } + return MarketLookup { url: resolve_in(parsed.entries, name), error: "" } } // Parse the index JSON into entries; malformed entries (no name or url) are -// dropped, and bad JSON yields an empty list. -fun parse_index(text: String) -> List { - let out: List = [] - match parse(text) { +// dropped. Bad JSON or a non-array document is reported in the result. +fun parse_index_result(text: String) -> MarketIndex { + let entries: List = [] + return match parse(text) { Ok(j) -> { - let arr = as_list(j) - let i = 0 - while i < arr.len() { - let name = as_str(field(arr[i], "name")) - let url = as_str(field(arr[i], "url")) - if name != "" && url != "" { - out.push( - MarketEntry { - name, - description: as_str(field(arr[i], "description")), - url, - }, - ) - } - i = i + 1 + match j { + Array(arr) -> { + let i = 0 + while i < arr.len() { + let name = as_str(field(arr[i], "name")) + let url = as_str(field(arr[i], "url")) + if name != "" && url != "" { + entries.push( + MarketEntry { + name, + description: as_str(field(arr[i], "description")), + url, + }, + ) + } + i = i + 1 + } + MarketIndex { entries, error: "" } + }, + _ -> MarketIndex { entries, error: "expected a JSON array" }, } }, - Err(e) -> {}, + Err(e) -> MarketIndex { entries, error: e.message }, } - return out +} + +// Backward-compatible entry parser for pure callers that only need entries. +// Bad JSON still returns an empty list; use parse_index_result when the caller +// needs to report malformed index data. +fun parse_index(text: String) -> List { + return parse_index_result(text).entries } // The entries whose name or description contains `query`, ignoring case. An @@ -153,3 +192,7 @@ fun _suffix(query: String) -> String { } return " for '${query.trim()}'" } + +fun _index_error(detail: String) -> String { + return "error: could not parse plugin index (${market_url()}): ${detail}" +} diff --git a/src/plugins/market_test.rv b/src/plugins/market_test.rv index f781c2f..5f1888a 100644 --- a/src/plugins/market_test.rv +++ b/src/plugins/market_test.rv @@ -1,6 +1,6 @@ import std/string import std/test { assert_eq_str, assert_eq_int, assert_true } -import "./market" { MarketEntry, parse_index, filter_entries, resolve_in, format_entries, market_url } +import "./market" { MarketEntry, parse_index, parse_index_result, filter_entries, resolve_in, resolve_name_in_index, format_entries, market_url } const INDEX: String = "[{\"name\": \"fmt\", \"description\": \"formats on save\", \"url\": \"github.com/u/rook-fmt\"}, {\"name\": \"lint\", \"description\": \"runs the linter\", \"url\": \"github.com/u/rook-lint\"}, {\"name\": \"bad\"}]" @@ -18,6 +18,20 @@ fun test_parse_index_tolerates_bad_json() { assert_eq_int(parse_index("[]").len(), 0) } +fun test_parse_index_result_reports_bad_documents() { + let bad = parse_index_result("not json") + assert_eq_int(bad.entries.len(), 0) + assert_true(bad.error != "") + + let non_array = parse_index_result("{\"plugins\": []}") + assert_eq_int(non_array.entries.len(), 0) + assert_true(non_array.error.contains("JSON array")) + + let empty = parse_index_result("[]") + assert_eq_int(empty.entries.len(), 0) + assert_eq_str(empty.error, "") +} + fun test_filter_by_name_or_description() { let entries = parse_index(INDEX) assert_eq_int(filter_entries(entries, "").len(), 2) @@ -34,6 +48,16 @@ fun test_resolve_name_returns_the_url() { assert_eq_str(resolve_in(entries, "missing"), "") } +fun test_resolve_name_in_index_reports_parse_errors() { + let bad = resolve_name_in_index("not json", "fmt") + assert_eq_str(bad.url, "") + assert_true(bad.error.starts_with("error: could not parse plugin index")) + + let empty = resolve_name_in_index("[]", "fmt") + assert_eq_str(empty.url, "") + assert_eq_str(empty.error, "") +} + fun test_format_shows_name_and_url() { let entries = parse_index(INDEX) let text = format_entries(entries) From 5fbbdeedda21852ff3a15222e127107394afea40 Mon Sep 17 00:00:00 2001 From: martian56 Date: Thu, 9 Jul 2026 00:26:32 +0400 Subject: [PATCH 2/2] fix(plugins): report index fetch errors --- CHANGELOG.md | 2 +- src/plugins/market.rv | 27 ++++++++++++++++++--------- src/plugins/market_test.rv | 12 +++++++++++- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26d4a7f..9e3b861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.3.17 -- Surfaced malformed plugin marketplace index JSON instead of treating parse failures as an empty index. +- Surfaced malformed plugin marketplace index JSON and fetch failures instead of treating them as an empty index. ## 0.3.16 diff --git a/src/plugins/market.rv b/src/plugins/market.rv index 200c130..9d02138 100644 --- a/src/plugins/market.rv +++ b/src/plugins/market.rv @@ -47,7 +47,7 @@ fun browse_text(query: String) -> String { return match request_with_timeout("GET", market_url(), "", headers, TIMEOUT_MS) { Ok(resp) -> { if resp.status_code >= 400 { - "error: the plugin index returned ${resp.status_code}" + _http_error(resp.status_code) } else { let parsed = parse_index_result(resp.body) if parsed.error != "" { @@ -76,15 +76,16 @@ fun resolve_name(name: String) -> String { 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: "" } - } else { - resolve_name_in_index(resp.body, name) - } - }, - Err(e) -> MarketLookup { url: "", error: "" }, + Ok(resp) -> resolve_name_from_response(resp.status_code, resp.body, name), + Err(e) -> MarketLookup { url: "", error: _reach_error(e.message) }, + } +} + +fun resolve_name_from_response(status_code: Int, body: String, name: String) -> MarketLookup { + if status_code >= 400 { + return MarketLookup { url: "", error: _http_error(status_code) } } + return resolve_name_in_index(body, name) } fun resolve_name_in_index(text: String, name: String) -> MarketLookup { @@ -196,3 +197,11 @@ fun _suffix(query: String) -> String { fun _index_error(detail: String) -> String { return "error: could not parse plugin index (${market_url()}): ${detail}" } + +fun _http_error(status_code: Int) -> String { + return "error: the plugin index returned ${status_code}" +} + +fun _reach_error(detail: String) -> String { + return "error: could not reach the plugin index (${market_url()}): ${detail}" +} diff --git a/src/plugins/market_test.rv b/src/plugins/market_test.rv index 5f1888a..3c4f588 100644 --- a/src/plugins/market_test.rv +++ b/src/plugins/market_test.rv @@ -1,6 +1,6 @@ import std/string import std/test { assert_eq_str, assert_eq_int, assert_true } -import "./market" { MarketEntry, parse_index, parse_index_result, filter_entries, resolve_in, resolve_name_in_index, format_entries, market_url } +import "./market" { MarketEntry, parse_index, parse_index_result, filter_entries, resolve_in, resolve_name_from_response, resolve_name_in_index, format_entries, market_url } const INDEX: String = "[{\"name\": \"fmt\", \"description\": \"formats on save\", \"url\": \"github.com/u/rook-fmt\"}, {\"name\": \"lint\", \"description\": \"runs the linter\", \"url\": \"github.com/u/rook-lint\"}, {\"name\": \"bad\"}]" @@ -58,6 +58,16 @@ fun test_resolve_name_in_index_reports_parse_errors() { assert_eq_str(empty.error, "") } +fun test_resolve_name_from_response_reports_http_errors() { + let missing_index = resolve_name_from_response(503, "temporarily unavailable", "fmt") + assert_eq_str(missing_index.url, "") + assert_eq_str(missing_index.error, "error: the plugin index returned 503") + + let bad = resolve_name_from_response(200, "not json", "fmt") + assert_eq_str(bad.url, "") + assert_true(bad.error.starts_with("error: could not parse plugin index")) +} + fun test_format_shows_name_and_url() { let entries = parse_index(INDEX) let text = format_entries(entries)