chore: make system more stable with invalid json responses from AI
This commit is contained in:
10
CODESMELL.md
10
CODESMELL.md
@@ -99,11 +99,9 @@ _None._ All modules previously on the queue have been split; refresh the queue i
|
||||
|
||||
## 9. `Jason.decode!/1` on External HTTP Responses
|
||||
|
||||
**Status:** open.
|
||||
**Status:** ✅ done (2026-05-01). The 2 scoped HTTP response decodes in `BDS.AI.OpenAICompatibleRuntime` now use `Jason.decode/1` and return `{:error, %{kind: :invalid_json_response, reason: reason}}` for malformed model-list or chat-completion response bodies. The error shape propagates through endpoint model listing, chat, and one-shot orchestration via the existing `with` chains. Remaining `Jason.decode!/1` sites in `BDS.AI.Catalog` decode model-catalog HTTP/cache payloads or trusted on-disk JSON and are out of scope for this section.
|
||||
|
||||
**Scope:** only the 2 sites in `BDS.AI` / `BDS.AI.OpenAICompatibleRuntime` that decode HTTP response bodies. The remaining 12 `Jason.decode!/1` sites decode our own on-disk files (metadata, embeddings index, generation hashes) and are not in scope.
|
||||
|
||||
**Plan:** switch to `Jason.decode/1` and propagate `{:error, reason}` from the runtime through to the chat / one-shot orchestration layer in `BDS.AI.Chat` and `BDS.AI.OneShot`.
|
||||
**Rule:** OpenAI-compatible runtime HTTP responses must not be decoded with bang functions; malformed provider JSON is a recoverable runtime error.
|
||||
|
||||
---
|
||||
|
||||
@@ -161,6 +159,10 @@ Most tests share the SQLite repo and named GenServers (`BDS.Tasks`, `BDS.Search`
|
||||
|
||||
## Changelog
|
||||
|
||||
### 2026-05-01
|
||||
|
||||
- **`Jason.decode!/1` on external HTTP responses**: replaced the 2 scoped OpenAI-compatible runtime response decodes with `Jason.decode/1` and tagged `{:error, %{kind: :invalid_json_response, reason: reason}}` propagation for malformed `/models` and `/chat/completions` bodies. Added regressions covering endpoint model listing through a fake HTTP client and generation through a local Bandit server. Section 9 is closed.
|
||||
|
||||
### 2026-05-10
|
||||
|
||||
- **Duplicate helpers across contexts**: added `BDS.MapUtils` (`attr/2`, `maybe_put/3`, `blank_to_nil/1`) and expanded `BDS.ProgressReporter` (`callback/1`, `scaled/3`, `report_count_started/4`, `report_count_progress/5`, `report_rebuild_started/3`, `report_rebuild_progress/4`, `report_phase/3`). Replaced copy-pasted helpers in posts, post translations, post rebuild, media file ops, search, generation progress, maintenance progress, embeddings/index progress, publishing, MCP util, scripts, and templates. Domain-specific modules now express their wording/ranges as options to the shared reporter, preserving existing user-facing progress messages while sharing one implementation. Added focused tests for both shared modules. Section 6 is closed.
|
||||
|
||||
@@ -47,23 +47,24 @@ defmodule BDS.AI.OpenAICompatibleRuntime do
|
||||
end
|
||||
|
||||
defp normalize_response(body) do
|
||||
payload = Jason.decode!(body)
|
||||
message = get_in(payload, ["choices", Access.at(0), "message"]) || %{}
|
||||
content = normalize_content(message["content"])
|
||||
tool_calls = normalize_tool_calls(message["tool_calls"] || [])
|
||||
usage = normalize_usage(payload["usage"] || %{})
|
||||
with {:ok, payload} <- decode_json_body(body) do
|
||||
message = get_in(payload, ["choices", Access.at(0), "message"]) || %{}
|
||||
content = normalize_content(message["content"])
|
||||
tool_calls = normalize_tool_calls(message["tool_calls"] || [])
|
||||
usage = normalize_usage(payload["usage"] || %{})
|
||||
|
||||
json =
|
||||
case content do
|
||||
nil -> nil
|
||||
value when is_binary(value) ->
|
||||
case Jason.decode(value) do
|
||||
{:ok, decoded} when is_map(decoded) -> decoded
|
||||
_other -> nil
|
||||
end
|
||||
end
|
||||
json =
|
||||
case content do
|
||||
nil -> nil
|
||||
value when is_binary(value) ->
|
||||
case Jason.decode(value) do
|
||||
{:ok, decoded} when is_map(decoded) -> decoded
|
||||
_other -> nil
|
||||
end
|
||||
end
|
||||
|
||||
{:ok, %{content: content, json: json, tool_calls: tool_calls, usage: usage}}
|
||||
{:ok, %{content: content, json: json, tool_calls: tool_calls, usage: usage}}
|
||||
end
|
||||
end
|
||||
|
||||
defp completions_url(url) do
|
||||
@@ -84,24 +85,31 @@ defmodule BDS.AI.OpenAICompatibleRuntime do
|
||||
end
|
||||
|
||||
defp normalize_models_response(body) do
|
||||
payload = Jason.decode!(body)
|
||||
with {:ok, payload} <- decode_json_body(body) do
|
||||
models =
|
||||
payload
|
||||
|> Map.get("data", [])
|
||||
|> Enum.map(fn entry ->
|
||||
id = entry["id"] || entry[:id]
|
||||
|
||||
models =
|
||||
payload
|
||||
|> Map.get("data", [])
|
||||
|> Enum.map(fn entry ->
|
||||
id = entry["id"] || entry[:id]
|
||||
%{
|
||||
id: id,
|
||||
label: id
|
||||
}
|
||||
end)
|
||||
|> Enum.reject(&is_nil(&1.id))
|
||||
|> Enum.uniq_by(& &1.id)
|
||||
|> Enum.sort_by(&String.downcase(&1.id))
|
||||
|
||||
%{
|
||||
id: id,
|
||||
label: id
|
||||
}
|
||||
end)
|
||||
|> Enum.reject(&is_nil(&1.id))
|
||||
|> Enum.uniq_by(& &1.id)
|
||||
|> Enum.sort_by(&String.downcase(&1.id))
|
||||
{:ok, models}
|
||||
end
|
||||
end
|
||||
|
||||
{:ok, models}
|
||||
defp decode_json_body(body) do
|
||||
case Jason.decode(body) do
|
||||
{:ok, payload} -> {:ok, payload}
|
||||
{:error, reason} -> {:error, %{kind: :invalid_json_response, reason: reason}}
|
||||
end
|
||||
end
|
||||
|
||||
defp maybe_put_auth(headers, nil), do: headers
|
||||
|
||||
@@ -88,6 +88,25 @@ defmodule BDS.AITest do
|
||||
def get(_url, _headers), do: {:error, :not_found}
|
||||
end
|
||||
|
||||
defmodule BadJsonEndpointHttpClient do
|
||||
def get("https://api.example.test/v1/models", _headers) do
|
||||
{:ok, %{status: 200, headers: %{}, body: "not json"}}
|
||||
end
|
||||
end
|
||||
|
||||
defmodule BadJsonCompletionServer do
|
||||
use Plug.Router
|
||||
|
||||
plug(:match)
|
||||
plug(:dispatch)
|
||||
|
||||
post "/v1/chat/completions" do
|
||||
conn
|
||||
|> Plug.Conn.put_resp_content_type("application/json")
|
||||
|> Plug.Conn.send_resp(200, "not json")
|
||||
end
|
||||
end
|
||||
|
||||
defmodule FakeRuntime do
|
||||
def generate(endpoint, request, opts) do
|
||||
test_pid = Keyword.fetch!(opts, :test_pid)
|
||||
@@ -256,6 +275,32 @@ defmodule BDS.AITest do
|
||||
assert [%{id: "gpt-4.1", label: "gpt-4.1"}, %{id: "gpt-4.1-mini", label: "gpt-4.1-mini"}] = models
|
||||
end
|
||||
|
||||
test "list_endpoint_models returns an error for malformed endpoint JSON" do
|
||||
assert {:error, %{kind: :invalid_json_response, reason: %Jason.DecodeError{}}} =
|
||||
BDS.AI.list_endpoint_models(%{url: "https://api.example.test/v1", api_key: "online-secret"},
|
||||
http_client: BadJsonEndpointHttpClient
|
||||
)
|
||||
end
|
||||
|
||||
test "openai-compatible generation returns an error for malformed completion JSON" do
|
||||
server =
|
||||
start_supervised!({Bandit, plug: BadJsonCompletionServer, port: 0, startup_log: false})
|
||||
|
||||
{:ok, {_address, port}} = ThousandIsland.listener_info(server)
|
||||
|
||||
assert {:error, %{kind: :invalid_json_response, reason: %Jason.DecodeError{}}} =
|
||||
BDS.AI.OpenAICompatibleRuntime.generate(
|
||||
%{url: "http://127.0.0.1:#{port}/v1", api_key: nil},
|
||||
%{
|
||||
model: "gpt-test",
|
||||
messages: [%{"role" => "user", "content" => "Hello"}],
|
||||
max_output_tokens: 128,
|
||||
tools: []
|
||||
},
|
||||
[]
|
||||
)
|
||||
end
|
||||
|
||||
test "airplane mode routes title tasks to airplane endpoint and offline title model" do
|
||||
assert {:ok, _endpoint} =
|
||||
BDS.AI.put_endpoint(:online, %{
|
||||
|
||||
Reference in New Issue
Block a user