Fix mod update checks failing on non-JSON responses (#931)

The mod update and Find Mods feeds feed the raw HTTP body straight to Json.decode. When the endpoint hands back something that is not JSON (an HTML error page, a proxy/captive prompt, or a plain-text outage message like "Exceeded secondary rate limit" -- usually still HTTP 200), the decoder's "unexpected character 'E'" assert escaped through the pcall and became the error message, blaming the parser instead of the response. Add Json.describeUnexpected() as a pre-decode content-type guard: it returns nil for body shapes the endpoints actually publish (JSON object or array) and otherwise a short message naming what the server sent (HTML page / plain text / empty, with a preview). Wire it into ModUpdate.parseReleases and ModIndex.parse, so both the sync and async update-check paths surface the real answer instead of the parse error. HTTP status was already checked upstream by HostShell.httpGet (non-2xx becomes "HTTP <code> from <url> (...)"); this closes the remaining "2xx but not JSON" gap everywhere, including bridge platforms that expose no status or headers. Add regression tests for plain-text, HTML, and empty bodies; strengthen the ModIndex HTML soft-fail test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-12 00:10:56 +02:00 · 2026-08-07 10:49:10 +01:00
parent 112120e8fe
commit d59a9522ee
5 changed files with 58 additions and 3 deletions
@@ -171,4 +171,26 @@ function Json.decode(s)
  return nil, v
 end

+-- For an HTTP response that was meant to carry JSON but did not.  Returns nil
+-- when `s` starts like a JSON object or array (the only shapes the update and
+-- index endpoints publish), otherwise a short message naming what the server
+-- actually sent -- so callers surface "the response was an HTML page/plain
+-- text, not JSON (it starts with ...)" instead of leaking the decoder's
+-- low-level "unexpected character 'E'" assert at the first byte of an error
+-- page or plain-text outage message.
+function Json.describeUnexpected(s)
+  if type(s) ~= "string" then
+    return "the response had no body to decode"
+  end
+  local first = s:match("^%s*(.)")
+  if first == "{" or first == "[" then return nil end
+  local preview = s:gsub("%s+", " "):gsub("^%s+", ""):gsub("%s+$", "")
+  if preview == "" then
+    return "the response was empty, not JSON"
+  end
+  if #preview > 60 then preview = preview:sub(1, 57) .. "..." end
+  local kind = (first == "<") and "an HTML page" or "plain text"
+  return ("the response was %s, not JSON (it starts with %q)"):format(kind, preview)
+end
+
 return Json
@@ -191,8 +191,10 @@ end
 -- Never throws: a truncated download, an HTML error page, or a feed from a
 -- future schema all come back as a message the panel can print.
 function ModIndex.parse(jsonText, Json)
+  Json = Json or require("src.link.Json")
+  local notJson = Json.describeUnexpected(jsonText)
+  if notJson then return nil, notJson end
  local ok, result, err = pcall(function()
-    Json = Json or require("src.link.Json")
    local doc, decodeErr = Json.decode(jsonText)
    if type(doc) ~= "table" then
      return nil, decodeErr or "index.json is not an object"
@@ -140,8 +140,10 @@ end
 -- Decode a releases array (GET /repos/.../releases) into a sorted list
 -- (newest first). Releases without a .zip asset are dropped. Never throws.
 function ModUpdate.parseReleases(jsonText, modId, Json)
+  Json = Json or require("src.link.Json")
+  local notJson = Json.describeUnexpected(jsonText)
+  if notJson then return nil, notJson end
  local ok, result, err = pcall(function()
-    Json = Json or require("src.link.Json")
    local doc, decodeErr = Json.decode(jsonText)
    if type(doc) ~= "table" then
      return nil, decodeErr or "releases json is not an array"
@@ -142,7 +142,11 @@ do
  index, err = ModIndex.parse(Json.encode({ mods = { NUZLOCKE } }))
  check(index == nil and err ~= nil, "a feed with no schema_version is refused")
  index, err = ModIndex.parse("<!DOCTYPE html><html>404</html>")
-  check(index == nil and err ~= nil, "an HTML error page soft-fails")
+  check(index == nil and tostring(err):find("HTML", 1, true) ~= nil,
+    "an HTML error page is named, not blamed on the parser")
+  index, err = ModIndex.parse("Error: upstream unavailable")
+  check(index == nil and tostring(err):find("not JSON", 1, true) ~= nil,
+    "a plain-text error names the response")
  index, err = ModIndex.parse('{"schema_version":1}')
  check(index == nil and err ~= nil, "a feed with no mods array soft-fails")
 end
@@ -76,6 +76,31 @@ do
  check(path == nil and dlErr ~= nil, "empty url soft-fails")
 end

+-- the reported bug: a non-JSON answer (plain-text error, proxy/captive
+-- prompt, outage message) used to leak the decoder's "unexpected character"
+-- assert at the first byte of the body. The guard must name what the server
+-- actually sent and never let that assert surface.
+do
+  local list, err = ModUpdate.parseReleases("Error: API rate limit exceeded", "demo")
+  check(list == nil and err ~= nil, "plain-text error soft-fails")
+  check(tostring(err):find("not JSON", 1, true) ~= nil
+      and tostring(err):find("Error: API", 1, true) ~= nil,
+    "plain-text error names the response and previews what it said")
+  list, err = ModUpdate.parseReleases("<!DOCTYPE html><html>502 Bad Gateway</html>", "demo")
+  check(list == nil and tostring(err):find("HTML", 1, true) ~= nil,
+    "an HTML error page is named as such")
+  list, err = ModUpdate.parseReleases("", "demo")
+  check(list == nil and tostring(err):find("empty", 1, true) ~= nil,
+    "an empty response is named")
+  check(tostring(err):find("unexpected character", 1, true) == nil,
+    "the decoder's assert never leaks into the message")
+  list = ModUpdate.parseReleases(Json.encode({
+    { tag_name = "v1.0.0", assets = {
+        { name = "demo-1.0.0.zip", browser_download_url = "https://x/d.zip" } } },
+  }), "demo")
+  eq(#list, 1, "the guard lets real JSON through")
+end
+
 do
  local body = Json.encode({
    tag_name = "v2.0.0",