Surface malformed plugin index errors

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.3.17
+
+- Surfaced malformed plugin marketplace index JSON and fetch failures instead of treating them as an empty index.
+
 ## 0.3.16
 
 - Reported plugin removal failures when recursive deletion commands fail or the plugin directory remains on disk.

rv.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rook"
-version = "0.3.16"
+version = "0.3.17"
 edition = "v2"
 
 [dependencies]

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 <url> [tag]", "remove <name>", "update <name>").
@@ -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"
         }

src/plugins/market.rv

@@ -20,6 +20,16 @@ struct MarketEntry {
     url: String,
 }
 
+struct MarketIndex {
+    entries: List<MarketEntry>,
+    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") {
@@ -37,9 +47,13 @@ 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 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,71 @@ 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 {
-                ""
-            } else {
-                resolve_in(parse_index(resp.body), name)
-            }
-        },
-        Err(e) -> "",
+        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 {
+    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<MarketEntry> {
-    let out: List<MarketEntry> = []
-    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<MarketEntry> = []
+    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<MarketEntry> {
+    return parse_index_result(text).entries
 }
 
 // The entries whose name or description contains `query`, ignoring case. An
@@ -153,3 +193,15 @@ 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}"
+}
+
+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}"
+}

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_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\"}]"
 
@@ -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,26 @@ 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_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)