diff --git a/CHANGELOG.md b/CHANGELOG.md index 01146e3..bba91f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,85 @@ All notable changes to this project will be documented in this file. +## [0.15.8] - 2026-05-06 + +### Fixed + +- **Vertex AI / Gemini whitespace text parts no longer crash the + request pipeline.** Gemini occasionally returns `text` parts whose + content is only newlines (e.g. `"\n\n\n"`) — typically between tool + calls or as filler when the model is blocked. Ecto's default + `:empty_values` for `cast/3` treats whitespace-only strings as + empty, so `Nous.Message.ContentPart`'s changeset dropped the + `content` field entirely and then raised + `%Ecto.InvalidChangesetError{errors: [content: {"content is required", + []}]}` from `ContentPart.new!/1`, taking down the whole + `Nous.LLM.run_with_tools/6` call. `ContentPart` now overrides + `:empty_values` to `[""]` so legitimate whitespace content is + preserved, and `Nous.Messages.Gemini.parse_content/1` defensively + skips whitespace-only text parts to avoid creating useless + `ContentPart`s. The streaming normalizer (`Nous.StreamNormalizer.Gemini`) + already had this guard; the non-streaming path is now consistent. +- **`Nous.Messages.Gemini.parse_content/1` no longer silently drops + function calls without `args`.** Nullary tool calls + (`%{"functionCall" => %{"name" => "get_time"}}`) were falling into + the catch-all clause and disappearing. Pattern now requires only + `name` and falls back to `%{}` for `args`, matching the behavior of + the sibling `parse_parts/1` helper. + +### Added + +- **`Nous.Errors.RetryInfo`** parses server-suggested retry hints from + provider error responses. Checks `error.details[]` for + `google.rpc.RetryInfo` (Vertex AI / Gemini) first, then the + `Retry-After` HTTP header. Returns delay in milliseconds, or `nil` + when no hint is available — `nil` is itself meaningful for Google + APIs, since long-term/daily quota exhaustion deliberately omits + `RetryInfo` to discourage retry loops. +- **`Nous.Errors.ProviderError` gains `:retry_after_ms`** alongside + the existing `:status_code`. `Nous.Provider.request/3` and + `request_stream/3` now populate both fields automatically when the + underlying HTTP layer returns an error tuple, so callers can branch + on rate-limit hints without parsing provider-specific bodies: + + ```elixir + case Nous.LLM.run_with_tools(...) do + {:error, %Nous.Errors.ProviderError{retry_after_ms: ms}} when is_integer(ms) -> + {:snooze, ms} # use server-suggested delay + {:error, %Nous.Errors.ProviderError{status_code: 429}} -> + {:snooze, exp_backoff(attempt)} # rate-limited, no hint + ... + end + ``` + +- **Gemini/Vertex `finishReason` and `promptFeedback` are surfaced.** + `Nous.Messages.Gemini.from_response/1` now stores both in + `message.metadata` (when present) and emits a `Logger.warning` when + the candidate produced empty content for a non-STOP reason + (`SAFETY`, `RECITATION`, `MAX_TOKENS`, etc.) or when the prompt was + blocked. Previously these signals were discarded, so blocked + generations manifested as silent empty messages with no diagnostic. + +### Changed + +- **HTTP error tuples now carry response headers.** + `Nous.HTTP.Backend.Req`, `Nous.HTTP.Backend.Hackney`, and + `Nous.HTTP.StreamBackend.Req` previously returned + `{:error, %{status, body}}` and dropped headers entirely, which made + it impossible to read `Retry-After`. They now return + `{:error, %{status, body, headers}}` with `headers` as a list of + `{name, value}` tuples (lowercased per HTTP spec, both string). + Existing pattern matches on `%{status: _, body: _}` continue to work + since map matching is non-exhaustive. +- **Gemini tool-call ID generation unified.** + `Nous.Messages.Gemini.parse_content/1` previously used + `"gemini_#{:rand.uniform(10_000)}"` (~50% birthday-paradox collision + at ~118 calls) while `parse_parts/1` used + `"call_#{:rand.uniform(1_000_000)}"` — two formats, two ranges. Both + now share a `generate_tool_call_id/0` helper using 64 bits of + `:crypto.strong_rand_bytes/1`, base64url-encoded with the + `gemini_` prefix preserved. + ## [0.15.7] - 2026-05-05 ### Changed diff --git a/lib/nous/errors.ex b/lib/nous/errors.ex index 23d6bab..5b05515 100644 --- a/lib/nous/errors.ex +++ b/lib/nous/errors.ex @@ -85,14 +85,25 @@ defmodule Nous.Errors do Error from an LLM provider. Raised when a provider API call fails. + + ## Fields + + * `:provider` — provider id atom (e.g., `:vertex_ai`) + * `:status_code` — HTTP status when applicable + * `:retry_after_ms` — server-suggested backoff in milliseconds, parsed + from the response body (`google.rpc.RetryInfo`) or `Retry-After` + header. `nil` when the failure is not retry-hinted (e.g. daily quota + exhaustion deliberately omits `RetryInfo` per Google's spec). + * `:details` — raw error payload from the HTTP layer """ - defexception [:message, :provider, :status_code, :details] + defexception [:message, :provider, :status_code, :retry_after_ms, :details] @type t :: %__MODULE__{ message: String.t(), provider: atom() | nil, status_code: integer() | nil, + retry_after_ms: pos_integer() | nil, details: any() } @@ -100,6 +111,7 @@ defmodule Nous.Errors do def exception(opts) when is_list(opts) do provider = Keyword.get(opts, :provider) status_code = Keyword.get(opts, :status_code) + retry_after_ms = Keyword.get(opts, :retry_after_ms) details = Keyword.get(opts, :details) message = @@ -112,6 +124,7 @@ defmodule Nous.Errors do message: message, provider: provider, status_code: status_code, + retry_after_ms: retry_after_ms, details: details } end diff --git a/lib/nous/errors/retry_info.ex b/lib/nous/errors/retry_info.ex new file mode 100644 index 0000000..57f57eb --- /dev/null +++ b/lib/nous/errors/retry_info.ex @@ -0,0 +1,103 @@ +defmodule Nous.Errors.RetryInfo do + @moduledoc """ + Parse server-suggested retry delays from provider error responses. + + Two sources are checked, body first then headers: + + 1. **Body** — Google APIs (Vertex AI, Gemini) embed + `google.rpc.RetryInfo` inside `error.details[]` with a + `retryDelay` field as a `google.protobuf.Duration` string + (e.g. `"34s"`, `"1.500s"`). + 2. **Headers** — Standard HTTP `Retry-After` (RFC 7231). Integer + seconds is supported; HTTP-date form is intentionally not handled + here as no LLM provider in production uses it for rate limits. + + Returns the suggested delay in **milliseconds**, or `nil` when no + hint is available. A missing hint is itself meaningful for Google + APIs — daily/long-term quota exhaustion deliberately omits + `RetryInfo` to discourage retry loops, so callers should treat + `nil` as "do not auto-retry". + """ + + @retry_info_type "type.googleapis.com/google.rpc.RetryInfo" + + @doc """ + Extract a retry delay (ms) from an HTTP error tuple's payload. + + Accepts the shape produced by `Nous.HTTP.Backend` implementations: + `%{status: integer, body: term, headers: list}`. Missing fields are + tolerated. + + ## Examples + + iex> RetryInfo.parse(%{ + ...> status: 429, + ...> body: %{"error" => %{"details" => [ + ...> %{"@type" => "type.googleapis.com/google.rpc.RetryInfo", + ...> "retryDelay" => "34s"} + ...> ]}} + ...> }) + 34_000 + + iex> RetryInfo.parse(%{status: 429, headers: [{"retry-after", "60"}]}) + 60_000 + + iex> RetryInfo.parse(%{status: 429, body: %{"error" => %{"message" => "rate limited"}}}) + nil + """ + @spec parse(any()) :: pos_integer() | nil + def parse(%{} = error) do + body = Map.get(error, :body) + headers = Map.get(error, :headers, []) + + from_body(body) || from_headers(headers) + end + + def parse(_), do: nil + + # --------------------------------------------------------------------------- + # Body: google.rpc.RetryInfo inside error.details[] + + defp from_body(%{"error" => %{"details" => details}}) when is_list(details) do + Enum.find_value(details, fn + %{"@type" => @retry_info_type, "retryDelay" => delay} -> parse_duration(delay) + _ -> nil + end) + end + + defp from_body(_), do: nil + + # google.protobuf.Duration: "s" — int or fractional. + defp parse_duration(s) when is_binary(s) do + case Float.parse(s) do + {seconds, "s"} when seconds > 0 -> trunc(seconds * 1000) + _ -> nil + end + end + + defp parse_duration(_), do: nil + + # --------------------------------------------------------------------------- + # Headers: Retry-After (case-insensitive) + + defp from_headers(headers) when is_list(headers) do + Enum.find_value(headers, fn + {k, v} -> + if to_string(k) |> String.downcase() == "retry-after" do + parse_retry_after(to_string(v)) + end + + _ -> + nil + end) + end + + defp from_headers(_), do: nil + + defp parse_retry_after(s) do + case Integer.parse(s) do + {seconds, ""} when seconds > 0 -> seconds * 1000 + _ -> nil + end + end +end diff --git a/lib/nous/http/backend.ex b/lib/nous/http/backend.ex index 2344b02..3bff30c 100644 --- a/lib/nous/http/backend.ex +++ b/lib/nous/http/backend.ex @@ -10,8 +10,11 @@ defmodule Nous.HTTP.Backend do `docs/benchmarks/http_backend.md` for performance characteristics. Custom backends just need to implement `c:post/4` and return one of: - `{:ok, decoded_body}` for 2xx, `{:error, %{status: status, body: body}}` - for 4xx/5xx, or `{:error, term()}` for transport / decode failures. + `{:ok, decoded_body}` for 2xx, + `{:error, %{status: status, body: body, headers: headers}}` for 4xx/5xx + (headers as a list of `{name, value}` tuples — used by + `Nous.Errors.RetryInfo` to extract `Retry-After` hints), or + `{:error, term()}` for transport / decode failures. Streaming requests do NOT go through this behaviour — those always use hackney's `:async, :once` mode for backpressure (see diff --git a/lib/nous/http/backend/hackney.ex b/lib/nous/http/backend/hackney.ex index 1380adb..3f6b48e 100644 --- a/lib/nous/http/backend/hackney.ex +++ b/lib/nous/http/backend/hackney.ex @@ -63,12 +63,14 @@ defmodule Nous.HTTP.Backend.Hackney do {:ok, status, _resp_headers, body_bin} when status in 200..299 -> {:ok, decode_body(body_bin)} - {:ok, status, _resp_headers, body_bin} -> + {:ok, status, resp_headers, body_bin} -> decoded = decode_body(body_bin) Logger.warning("HTTP request failed with status #{status}: #{truncate_for_log(decoded)}") - {:error, %{status: status, body: decoded}} + # Headers surfaced for Nous.Errors.RetryInfo. Hackney returns names + # and values as charlists; convert to strings for downstream parsing. + {:error, %{status: status, body: decoded, headers: stringify_headers(resp_headers)}} {:error, reason} = err -> Logger.error("Hackney request error: #{inspect(reason)}") @@ -76,6 +78,10 @@ defmodule Nous.HTTP.Backend.Hackney do end end + defp stringify_headers(headers) when is_list(headers) do + Enum.map(headers, fn {k, v} -> {to_string(k), to_string(v)} end) + end + defp decode_body(""), do: %{} defp decode_body(bin) when is_binary(bin) do diff --git a/lib/nous/http/backend/req.ex b/lib/nous/http/backend/req.ex index 653a9d4..6e66031 100644 --- a/lib/nous/http/backend/req.ex +++ b/lib/nous/http/backend/req.ex @@ -36,12 +36,16 @@ defmodule Nous.HTTP.Backend.Req do {:ok, %Req.Response{status: status, body: response_body}} when status in 200..299 -> {:ok, response_body} - {:ok, %Req.Response{status: status, body: response_body}} -> + {:ok, %Req.Response{status: status, body: response_body, headers: resp_headers}} -> Logger.warning( "HTTP request failed with status #{status}: #{truncate_for_log(response_body)}" ) - {:error, %{status: status, body: response_body}} + # Headers are surfaced (Retry-After etc.) so Nous.Errors.RetryInfo + # can extract server-suggested backoff. Req returns headers as a + # map of lowercased name => list of values; flatten to a list of + # {name, value} pairs to match the shape other layers expect. + {:error, %{status: status, body: response_body, headers: normalize_headers(resp_headers)}} {:error, %Mint.TransportError{reason: reason} = error} -> Logger.error("Transport error: #{inspect(reason)}") @@ -53,6 +57,12 @@ defmodule Nous.HTTP.Backend.Req do end end + # Req returns headers as %{"name" => ["value", ...]}. Flatten into the + # [{name, value}] shape that downstream consumers (RetryInfo) expect. + defp normalize_headers(headers) when is_map(headers) do + Enum.flat_map(headers, fn {k, vs} -> Enum.map(vs, &{k, &1}) end) + end + defp truncate_for_log(data) when is_binary(data) do if byte_size(data) > 500 do String.slice(data, 0, 500) <> "... (truncated)" diff --git a/lib/nous/http/stream_backend/req.ex b/lib/nous/http/stream_backend/req.ex index 554e899..29efcc7 100644 --- a/lib/nous/http/stream_backend/req.ex +++ b/lib/nous/http/stream_backend/req.ex @@ -95,9 +95,19 @@ defmodule Nous.HTTP.StreamBackend.Req do {:ok, %Req.Response{status: status}} when status in 200..299 -> send(parent, {ref, :done}) - {:ok, %Req.Response{status: status, body: response_body}} -> + {:ok, %Req.Response{status: status, body: response_body, headers: resp_headers}} -> Logger.error("Req stream got error status #{status}") - send(parent, {ref, {:error, %{status: status, body: response_body}}}) + + send( + parent, + {ref, + {:error, + %{ + status: status, + body: response_body, + headers: normalize_headers(resp_headers) + }}} + ) {:error, reason} -> Logger.error("Req stream error: #{inspect(reason)}") @@ -172,6 +182,12 @@ defmodule Nous.HTTP.StreamBackend.Req do end end + # Mirror Nous.HTTP.Backend.Req.normalize_headers/1 — flatten the map shape + # Req returns into [{name, value}] tuples that RetryInfo expects. + defp normalize_headers(headers) when is_map(headers) do + Enum.flat_map(headers, fn {k, vs} -> Enum.map(vs, &{k, &1}) end) + end + defp cleanup(%{task: nil}), do: :ok defp cleanup(%{task: task}) do diff --git a/lib/nous/message/content_part.ex b/lib/nous/message/content_part.ex index fa78396..dd7fffa 100644 --- a/lib/nous/message/content_part.ex +++ b/lib/nous/message/content_part.ex @@ -473,8 +473,11 @@ defmodule Nous.Message.ContentPart do # Private functions defp changeset(content_part, attrs) do + # Override Ecto's default :empty_values, which treats whitespace-only + # strings as empty and drops them. Gemini/Vertex sometimes returns text + # parts that are just newlines, and they're legitimate content here. content_part - |> cast(attrs, [:type, :content, :options]) + |> cast(attrs, [:type, :content, :options], empty_values: [""]) |> validate_required([:type]) |> validate_content() end @@ -490,9 +493,6 @@ defmodule Nous.Message.ContentPart do {_, nil} -> add_error(changeset, :content, "content is required") - {_, ""} -> - add_error(changeset, :content, "content cannot be empty") - {:image_url, content} -> validate_image_url(changeset, content) diff --git a/lib/nous/messages/gemini.ex b/lib/nous/messages/gemini.ex index c1c97fa..91470ef 100644 --- a/lib/nous/messages/gemini.ex +++ b/lib/nous/messages/gemini.ex @@ -8,6 +8,8 @@ defmodule Nous.Messages.Gemini do alias Nous.{Message, Usage} alias Nous.Message.ContentPart + require Logger + @doc """ Convert messages to Gemini format. @@ -56,22 +58,39 @@ defmodule Nous.Messages.Gemini do candidates = Map.get(response, "candidates", []) usage_data = Map.get(response, "usageMetadata", %{}) model_version = Map.get(response, "modelVersion", "gemini-model") + prompt_feedback = Map.get(response, "promptFeedback") candidate = List.first(candidates) || %{} content_data = Map.get(candidate, "content", %{}) parts_data = Map.get(content_data, "parts", []) + finish_reason = Map.get(candidate, "finishReason") {content_parts, reasoning_content, tool_calls} = parse_content(parts_data) - attrs = %{ - role: :assistant, - content: consolidate_content_parts(content_parts), - reasoning_content: consolidate_content_parts(reasoning_content), - metadata: %{ + consolidated_content = consolidate_content_parts(content_parts) + + log_if_blocked( + consolidated_content, + tool_calls, + finish_reason, + prompt_feedback, + model_version + ) + + metadata = + %{ model_name: model_version, usage: parse_usage(usage_data), timestamp: DateTime.utc_now() } + |> maybe_put_metadata(:finish_reason, finish_reason) + |> maybe_put_metadata(:prompt_feedback, prompt_feedback) + + attrs = %{ + role: :assistant, + content: consolidated_content, + reasoning_content: consolidate_content_parts(reasoning_content), + metadata: metadata } attrs = if length(tool_calls) > 0, do: Map.put(attrs, :tool_calls, tool_calls), else: attrs @@ -228,18 +247,26 @@ defmodule Nous.Messages.Gemini do {content_parts, reasoning_content, tool_calls} = Enum.reduce(parts_data, {[], [], []}, fn item, {parts, reasoning, tools} -> case item do - %{"text" => text} -> - if Map.get(item, "thought") do - {parts, [ContentPart.thinking(text) | reasoning], tools} - else - {[ContentPart.text(text) | parts], reasoning, tools} + %{"text" => text} when is_binary(text) -> + cond do + # Drop whitespace-only text. Gemini emits these between tool + # calls and after blocked generations. Carrying them forward + # produces empty ContentParts that add no value. + String.trim(text) == "" -> + {parts, reasoning, tools} + + Map.get(item, "thought") -> + {parts, [ContentPart.thinking(text) | reasoning], tools} + + true -> + {[ContentPart.text(text) | parts], reasoning, tools} end - %{"functionCall" => %{"name" => name, "args" => args}} -> + %{"functionCall" => %{"name" => name} = function_call} when is_binary(name) -> tool_call = %{ - "id" => "gemini_#{:rand.uniform(10000)}", + "id" => generate_tool_call_id(), "name" => name, - "arguments" => args + "arguments" => Map.get(function_call, "args", %{}) } {parts, reasoning, [tool_call | tools]} @@ -269,8 +296,7 @@ defmodule Nous.Messages.Gemini do function_call = Map.get(part, "functionCall") tool_call = %{ - # Generate random ID since Gemini doesn't provide one - "id" => "call_#{:rand.uniform(1_000_000)}", + "id" => generate_tool_call_id(), "name" => Map.get(function_call, "name"), "arguments" => Map.get(function_call, "args", %{}) } @@ -311,6 +337,31 @@ defmodule Nous.Messages.Gemini do def parse_usage(_), do: %Usage{} + # Gemini doesn't provide tool call IDs, so we synthesize one. 64 bits of + # entropy keeps collision probability negligible across high-volume jobs. + defp generate_tool_call_id do + "gemini_" <> (:crypto.strong_rand_bytes(8) |> Base.url_encode64(padding: false)) + end + + defp maybe_put_metadata(metadata, _key, nil), do: metadata + defp maybe_put_metadata(metadata, key, value), do: Map.put(metadata, key, value) + + # Surface non-STOP finish reasons (SAFETY, RECITATION, MAX_TOKENS, etc.) + # and prompt blocks so they don't manifest as silent empty responses. + defp log_if_blocked(content, tool_calls, finish_reason, prompt_feedback, model_version) do + empty? = (content == "" or is_nil(content)) and tool_calls == [] + block_reason = prompt_feedback && Map.get(prompt_feedback, "blockReason") + interesting_finish? = finish_reason not in [nil, "STOP", "FINISH_REASON_UNSPECIFIED"] + + if empty? and (interesting_finish? or block_reason) do + Logger.warning( + "Gemini/Vertex returned empty content. " <> + "model=#{model_version} finishReason=#{inspect(finish_reason)} " <> + "promptFeedback=#{inspect(prompt_feedback)}" + ) + end + end + defp consolidate_content_parts([]), do: "" defp consolidate_content_parts([%ContentPart{type: :text, content: content}]), do: content defp consolidate_content_parts([%ContentPart{type: :thinking, content: content}]), do: content diff --git a/lib/nous/provider.ex b/lib/nous/provider.ex index 5d459f8..597e691 100644 --- a/lib/nous/provider.ex +++ b/lib/nous/provider.ex @@ -241,6 +241,8 @@ defmodule Nous.Provider do wrapped_error = Nous.Errors.ProviderError.exception( provider: @provider_id, + status_code: error_status(error), + retry_after_ms: Nous.Errors.RetryInfo.parse(error), message: "Request failed: #{inspect(error)}", details: error ) @@ -350,6 +352,8 @@ defmodule Nous.Provider do wrapped_error = Nous.Errors.ProviderError.exception( provider: @provider_id, + status_code: error_status(error), + retry_after_ms: Nous.Errors.RetryInfo.parse(error), message: "Streaming request failed: #{inspect(error)}", details: error ) @@ -358,6 +362,10 @@ defmodule Nous.Provider do end end + # Pull HTTP status from the standard backend error tuple shape. + defp error_status(%{status: status}) when is_integer(status), do: status + defp error_status(_), do: nil + # Build provider options from model config defp build_provider_opts(model) do opts = [ diff --git a/mix.exs b/mix.exs index 248fa10..587e0c8 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ defmodule Nous.MixProject do use Mix.Project - @version "0.15.7" + @version "0.15.8" @source_url "https://github.com/nyo16/nous" def project do diff --git a/test/nous/content_part_test.exs b/test/nous/content_part_test.exs index 57757e6..fc6a4e9 100644 --- a/test/nous/content_part_test.exs +++ b/test/nous/content_part_test.exs @@ -83,6 +83,24 @@ defmodule Nous.Message.ContentPartTest do end end + describe "new/1 and text/1 with whitespace content" do + test "accepts whitespace-only text content (regression: Vertex \"\\n\\n\\n\")" do + # Ecto's default :empty_values trims whitespace and treats it as empty, + # which used to drop the content field and trigger "content is required" + # for legitimate Gemini text parts containing only newlines. + assert %ContentPart{type: :text, content: "\n\n\n"} = ContentPart.text("\n\n\n") + assert {:ok, %ContentPart{content: " "}} = ContentPart.new(%{type: :text, content: " "}) + end + + test "still rejects nil content" do + assert {:error, %Ecto.Changeset{}} = ContentPart.new(%{type: :text}) + end + + test "still rejects literal empty string" do + assert {:error, %Ecto.Changeset{}} = ContentPart.new(%{type: :text, content: ""}) + end + end + describe "from_binary/2" do test "converts binary data to content part with auto-detected MIME" do binary = File.read!(@parthenon_path) diff --git a/test/nous/errors/retry_info_test.exs b/test/nous/errors/retry_info_test.exs new file mode 100644 index 0000000..c46468e --- /dev/null +++ b/test/nous/errors/retry_info_test.exs @@ -0,0 +1,169 @@ +defmodule Nous.Errors.RetryInfoTest do + use ExUnit.Case, async: true + + alias Nous.Errors.RetryInfo + + describe "parse/1 — Google body shape" do + test "extracts retryDelay from RetryInfo detail" do + error = %{ + status: 429, + body: %{ + "error" => %{ + "code" => 429, + "status" => "RESOURCE_EXHAUSTED", + "details" => [ + %{"@type" => "type.googleapis.com/google.rpc.QuotaFailure", "violations" => []}, + %{ + "@type" => "type.googleapis.com/google.rpc.RetryInfo", + "retryDelay" => "34s" + } + ] + } + } + } + + assert RetryInfo.parse(error) == 34_000 + end + + test "handles fractional second durations" do + error = %{ + body: %{ + "error" => %{ + "details" => [ + %{ + "@type" => "type.googleapis.com/google.rpc.RetryInfo", + "retryDelay" => "1.5s" + } + ] + } + } + } + + assert RetryInfo.parse(error) == 1500 + end + + test "returns nil when details exist but no RetryInfo entry" do + # Daily-quota exhaustion deliberately omits RetryInfo — absence is + # the signal not to retry-loop. + error = %{ + status: 429, + body: %{ + "error" => %{ + "details" => [ + %{"@type" => "type.googleapis.com/google.rpc.QuotaFailure", "violations" => []} + ] + } + } + } + + assert RetryInfo.parse(error) == nil + end + + test "returns nil for malformed retryDelay" do + error = %{ + body: %{ + "error" => %{ + "details" => [ + %{"@type" => "type.googleapis.com/google.rpc.RetryInfo", "retryDelay" => "soon"} + ] + } + } + } + + assert RetryInfo.parse(error) == nil + end + + test "returns nil for zero or negative delay" do + error = %{ + body: %{ + "error" => %{ + "details" => [ + %{"@type" => "type.googleapis.com/google.rpc.RetryInfo", "retryDelay" => "0s"} + ] + } + } + } + + assert RetryInfo.parse(error) == nil + end + + test "returns nil when error body has no details key" do + error = %{status: 500, body: %{"error" => %{"message" => "internal"}}} + assert RetryInfo.parse(error) == nil + end + end + + describe "parse/1 — Retry-After header" do + test "extracts integer seconds from Retry-After" do + error = %{status: 429, headers: [{"retry-after", "60"}]} + assert RetryInfo.parse(error) == 60_000 + end + + test "matches Retry-After case-insensitively" do + error = %{status: 429, headers: [{"Retry-After", "30"}]} + assert RetryInfo.parse(error) == 30_000 + end + + test "returns nil for non-integer Retry-After (e.g. HTTP-date)" do + # HTTP-date form intentionally not handled; no LLM provider uses it. + error = %{status: 429, headers: [{"retry-after", "Wed, 21 Oct 2026 07:28:00 GMT"}]} + assert RetryInfo.parse(error) == nil + end + + test "returns nil for negative seconds" do + error = %{status: 429, headers: [{"retry-after", "-5"}]} + assert RetryInfo.parse(error) == nil + end + end + + describe "parse/1 — precedence and edge cases" do + test "body takes precedence over header when both present" do + error = %{ + status: 429, + body: %{ + "error" => %{ + "details" => [ + %{ + "@type" => "type.googleapis.com/google.rpc.RetryInfo", + "retryDelay" => "10s" + } + ] + } + }, + headers: [{"retry-after", "60"}] + } + + assert RetryInfo.parse(error) == 10_000 + end + + test "falls back to headers when body has no RetryInfo" do + error = %{ + status: 429, + body: %{"error" => %{"message" => "rate limited"}}, + headers: [{"retry-after", "5"}] + } + + assert RetryInfo.parse(error) == 5_000 + end + + test "tolerates missing :headers key" do + error = %{status: 429, body: %{"error" => %{"message" => "no info"}}} + assert RetryInfo.parse(error) == nil + end + + test "tolerates missing :body key" do + error = %{status: 429, headers: [{"retry-after", "12"}]} + assert RetryInfo.parse(error) == 12_000 + end + + test "returns nil for non-map input" do + assert RetryInfo.parse(:transport_error) == nil + assert RetryInfo.parse(nil) == nil + assert RetryInfo.parse("oops") == nil + end + + test "returns nil for entirely empty error map" do + assert RetryInfo.parse(%{}) == nil + end + end +end diff --git a/test/nous/http/backend_test.exs b/test/nous/http/backend_test.exs index 7a933bf..88b9e93 100644 --- a/test/nous/http/backend_test.exs +++ b/test/nous/http/backend_test.exs @@ -88,6 +88,46 @@ defmodule Nous.HTTP.BackendTest do Nous.Providers.HTTP.post("http://x", "not a map", [], backend: @backend) end + test "surfaces response headers in error tuple (e.g. Retry-After)", + %{bypass: bypass, url: url} do + Bypass.expect_once(bypass, "POST", "/v1/test", fn conn -> + conn + |> Plug.Conn.put_resp_header("retry-after", "42") + |> Plug.Conn.put_resp_header("content-type", "application/json") + |> Plug.Conn.resp(429, ~s({"error":{"message":"rate limited"}})) + end) + + assert {:error, %{status: 429, headers: headers}} = + @backend.post(url, %{}, [], []) + + assert is_list(headers) + + retry_after = + Enum.find_value(headers, fn {k, v} -> + if String.downcase(to_string(k)) == "retry-after", do: to_string(v) + end) + + assert retry_after == "42" + + # Must round-trip through Nous.Errors.RetryInfo unchanged. + assert Nous.Errors.RetryInfo.parse(%{status: 429, headers: headers}) == 42_000 + end + + test "extracts Vertex/Gemini RetryInfo from error body", %{bypass: bypass, url: url} do + body = + ~s({"error":{"code":429,"status":"RESOURCE_EXHAUSTED","details":[) <> + ~s({"@type":"type.googleapis.com/google.rpc.RetryInfo","retryDelay":"7s"}]}}) + + Bypass.expect_once(bypass, "POST", "/v1/test", fn conn -> + conn + |> Plug.Conn.put_resp_header("content-type", "application/json") + |> Plug.Conn.resp(429, body) + end) + + assert {:error, error} = @backend.post(url, %{}, [], []) + assert Nous.Errors.RetryInfo.parse(error) == 7_000 + end + test "passes custom headers through", %{bypass: bypass, url: url} do Bypass.expect_once(bypass, "POST", "/v1/test", fn conn -> assert ["Bearer test-token"] = Plug.Conn.get_req_header(conn, "authorization") diff --git a/test/nous/messages_gemini_test.exs b/test/nous/messages_gemini_test.exs index b079303..0286cad 100644 --- a/test/nous/messages_gemini_test.exs +++ b/test/nous/messages_gemini_test.exs @@ -171,6 +171,76 @@ defmodule Nous.MessagesGeminiTest do assert msg.content == "First chunk. Second chunk. Third chunk." end + test "skips whitespace-only text parts (regression: Vertex \"\\n\\n\\n\")" do + # Vertex/Gemini sometimes emits text parts that are pure whitespace, + # particularly between tool calls or after blocked generations. They + # should not crash ContentPart.text/1 and should not pollute content. + response = %{ + "candidates" => [ + %{ + "content" => %{ + "parts" => [ + %{"text" => "\n\n\n"}, + %{"text" => "real content"} + ] + } + } + ] + } + + msg = Gemini.from_response(response) + assert msg.content == "real content" + end + + test "skips text part that is only whitespace with no other content" do + response = %{ + "candidates" => [ + %{"content" => %{"parts" => [%{"text" => "\n\n\n"}]}} + ] + } + + msg = Gemini.from_response(response) + # Message's own changeset trims "" → nil; either result is fine here. + assert msg.content in [nil, ""] + end + + test "captures finishReason in metadata" do + response = %{ + "candidates" => [ + %{"content" => %{"parts" => [%{"text" => "Hi"}]}, "finishReason" => "STOP"} + ] + } + + msg = Gemini.from_response(response) + assert msg.metadata.finish_reason == "STOP" + end + + test "captures promptFeedback when present" do + response = %{ + "candidates" => [%{"content" => %{"parts" => []}, "finishReason" => "SAFETY"}], + "promptFeedback" => %{"blockReason" => "SAFETY"} + } + + msg = Gemini.from_response(response) + assert msg.metadata.finish_reason == "SAFETY" + assert msg.metadata.prompt_feedback == %{"blockReason" => "SAFETY"} + end + + test "parses functionCall without args (nullary tool)" do + response = %{ + "candidates" => [ + %{ + "content" => %{ + "parts" => [%{"functionCall" => %{"name" => "get_time"}}] + } + } + ] + } + + msg = Gemini.from_response(response) + assert [%{"name" => "get_time", "arguments" => %{}}] = msg.tool_calls + end + test "joins multiple thought parts into reasoning_content" do response = %{ "candidates" => [ diff --git a/test/nous/providers/provider_test.exs b/test/nous/providers/provider_test.exs index 820563d..a291480 100644 --- a/test/nous/providers/provider_test.exs +++ b/test/nous/providers/provider_test.exs @@ -51,6 +51,36 @@ defmodule Nous.ProviderTest do do: build_request_params(model, messages, settings) end + # Provider used for the error-wrapping tests: chat/2 returns an error tuple + # whose shape matches what Nous.HTTP.Backend produces (status + body + headers). + defmodule ErrorWrappingProvider do + use Nous.Provider, + id: :openai_compatible, + default_base_url: "https://err.example.com/v1", + default_env_key: "ERR_WRAP_TEST_API_KEY" + + @impl true + def chat(_params, opts) do + Keyword.fetch!(opts, :__simulated_error__) + end + + @impl true + def chat_stream(_params, opts) do + Keyword.fetch!(opts, :__simulated_error__) + end + + # Override to inject the error tuple via opts so request/3 wraps it. + defp build_provider_opts(model) do + [ + base_url: model.base_url, + api_key: model.api_key, + timeout: model.receive_timeout, + finch_name: Nous.Finch, + __simulated_error__: model.default_settings[:__simulated_error__] + ] + end + end + defmodule CustomTokenProvider do use Nous.Provider, id: :custom_token, @@ -609,6 +639,78 @@ defmodule Nous.ProviderTest do # High-Level Request Callback Tests # ============================================================================ + describe "request/3 error wrapping" do + alias Nous.Errors.ProviderError + alias Nous.Message + alias Nous.Model + + defp err_model(simulated_error) do + %Model{ + provider: :openai_compatible, + model: "test-model", + base_url: "https://err.example.com/v1", + default_settings: %{__simulated_error__: simulated_error} + } + end + + test "populates :status_code from HTTP error tuple" do + error = {:error, %{status: 500, body: %{"error" => "boom"}, headers: []}} + + assert {:error, %ProviderError{} = err} = + ErrorWrappingProvider.request(err_model(error), [Message.user("hi")], %{}) + + assert err.status_code == 500 + assert err.retry_after_ms == nil + assert err.provider == :openai_compatible + assert err.details == elem(error, 1) + end + + test "populates :retry_after_ms from Vertex/Gemini RetryInfo body" do + error = + {:error, + %{ + status: 429, + body: %{ + "error" => %{ + "code" => 429, + "details" => [ + %{ + "@type" => "type.googleapis.com/google.rpc.RetryInfo", + "retryDelay" => "34s" + } + ] + } + }, + headers: [] + }} + + assert {:error, %ProviderError{status_code: 429, retry_after_ms: 34_000}} = + ErrorWrappingProvider.request(err_model(error), [Message.user("hi")], %{}) + end + + test "populates :retry_after_ms from Retry-After header" do + error = + {:error, + %{ + status: 429, + body: %{"error" => "rate limited"}, + headers: [{"retry-after", "12"}] + }} + + assert {:error, %ProviderError{status_code: 429, retry_after_ms: 12_000}} = + ErrorWrappingProvider.request(err_model(error), [Message.user("hi")], %{}) + end + + test "leaves :status_code and :retry_after_ms nil for transport errors" do + error = {:error, %Mint.TransportError{reason: :econnrefused}} + + assert {:error, %ProviderError{status_code: nil, retry_after_ms: nil} = err} = + ErrorWrappingProvider.request(err_model(error), [Message.user("hi")], %{}) + + assert %Mint.TransportError{reason: :econnrefused} = err.details + end + end + describe "request/3 and request_stream/3 callbacks" do test "providers have request/3 function injected" do Code.ensure_loaded!(Nous.Providers.OpenAICompatible)