diff --git a/crates/llm-client/src/anthropic.rs b/crates/llm-client/src/anthropic.rs index 1c8562a..b0b6cf1 100644 --- a/crates/llm-client/src/anthropic.rs +++ b/crates/llm-client/src/anthropic.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use tokio::sync::mpsc; use tracing::{debug, info, trace, warn}; -use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, headers_to_json, redact_key}; +use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, error_response_body, headers_to_json, redact_key}; const DEFAULT_BASE_URL: &str = "https://api.anthropic.com"; const ANTHROPIC_VERSION: &str = "2023-06-01"; @@ -203,6 +203,10 @@ impl AnthropicClient { }) } + /// Sends the request and returns the raw response **without** `error_for_status`, + /// so the tool-calling paths can read the error body and attach the request + /// payload to the `LlmError` (a `reqwest` status error discards the body). The + /// plain `chat` path keeps its own `error_for_status`. async fn send_request(&self, body: &Value) -> reqwest::Result { self.http .post(self.url()) @@ -211,8 +215,7 @@ impl AnthropicClient { .header("X-Title", core_api::APP_NAME) .json(body) .send() - .await? - .error_for_status() + .await } /// Joined `thinking` blocks of a content array, if any (extended thinking). @@ -252,6 +255,23 @@ impl AnthropicClient { let http_resp = self.send_request(&body).await?; let response_headers = headers_to_json(http_resp.headers()); + let status = http_resp.status(); + if !status.is_success() { + let resp_text = http_resp.text().await?; + return Err(crate::LlmError { + status: Some(status.as_u16()), + message: format!( + "anthropic: HTTP {status} from {url}\nbody: {resp_text}", + url = self.url(), + ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), + }.into()); + } /// One content block being accumulated by index. #[derive(Default)] @@ -562,7 +582,23 @@ impl ChatbotClient for AnthropicClient { let http_resp = self.send_request(&body).await?; let response_headers = headers_to_json(http_resp.headers()); + let status = http_resp.status(); let resp_text = http_resp.text().await?; + if !status.is_success() { + return Err(crate::LlmError { + status: Some(status.as_u16()), + message: format!( + "anthropic: HTTP {status} from {url}\nbody: {resp_text}", + url = self.url(), + ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), + }.into()); + } let resp: Value = serde_json::from_str(&resp_text) .map_err(|e| anyhow::anyhow!("anthropic: failed to parse response JSON: {e}\nbody: {resp_text}"))?; let response_body: Value = serde_json::from_str(&resp_text).unwrap_or(Value::Null); diff --git a/crates/llm-client/src/lib.rs b/crates/llm-client/src/lib.rs index 274fd29..1d44eed 100644 --- a/crates/llm-client/src/lib.rs +++ b/crates/llm-client/src/lib.rs @@ -66,6 +66,14 @@ pub fn headers_to_json(headers: &reqwest::header::HeaderMap) -> Value { Value::Object(map) } +/// Turns a raw error-response body into a JSON `Value` for the payload log: +/// the parsed JSON when the provider returned JSON (the common case — an +/// `{"error": …}` object), else the raw text wrapped as a JSON string so a +/// non-JSON body (HTML gateway page, plain text) is still preserved verbatim. +pub fn error_response_body(text: String) -> Value { + serde_json::from_str::(&text).unwrap_or(Value::String(text)) +} + /// Returns a redacted preview of an API key: first 7 chars + "***". pub fn redact_key(key: &str) -> String { if key.len() > 7 { @@ -82,12 +90,17 @@ pub fn redact_key(key: &str) -> String { /// substring-matching a formatted message — which mis-fires when a model id, token /// count or URL merely contains "401"/"404"/… (bug B6). Non-HTTP failures (network, /// JSON parse, cancellation) stay ordinary `anyhow` errors with no status. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct LlmError { /// HTTP status code, when the failure came from an HTTP response. pub status: Option, /// Human-readable detail (provider tag + body), used for logs and the UI. pub message: String, + /// Request/response payload captured at the failing call, so the debug log + /// can show what was actually sent even when the provider rejected it (e.g. + /// a 400). `None` for failures with no HTTP round-trip (network, cancellation, + /// parse) — those carry no body to surface. + pub raw_meta: Option, } impl std::fmt::Display for LlmError { diff --git a/crates/llm-client/src/openai.rs b/crates/llm-client/src/openai.rs index bfc3cc7..c9a655e 100644 --- a/crates/llm-client/src/openai.rs +++ b/crates/llm-client/src/openai.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use tokio::sync::mpsc; use tracing::{debug, info, trace, warn}; -use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, headers_to_json, redact_key}; +use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, error_response_body, headers_to_json, redact_key}; use core_api::APP_NAME; /// OpenAI ChatGPT client (also compatible with any OpenAI-spec endpoint). @@ -140,6 +140,12 @@ impl OpenAiClient { "openai: HTTP {status} from {url}\nbody: {resp_text}", url = self.url(), ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), }.into()); } @@ -348,6 +354,12 @@ impl ChatbotClient for OpenAiClient { "openai: HTTP {status} from {url}\nbody: {resp_text}", url = self.url(), ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), }.into()); } diff --git a/crates/skald-core/src/session/handler/llm_call.rs b/crates/skald-core/src/session/handler/llm_call.rs index 70e099a..52c9982 100644 --- a/crates/skald-core/src/session/handler/llm_call.rs +++ b/crates/skald-core/src/session/handler/llm_call.rs @@ -13,7 +13,7 @@ use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; use tracing::{error, warn}; -use crate::chatbot::{ChatOptions, LlmTurn, StreamDelta}; +use crate::chatbot::{ChatOptions, LlmError, LlmTurn, StreamDelta}; use crate::db::llm_request_payloads; use crate::events::{ServerEvent, TokenDeltaKind}; use crate::llm::{LlmEntry, LlmStrength}; @@ -121,6 +121,27 @@ impl ChatSessionHandler { Err(e) => e, }; + // Persist the payload even on failure so the debug log shows the request + // that was rejected (e.g. a provider 400). Only the HTTP clients attach a + // body (`LlmError::raw_meta`); a network/parse/cancel error carries none. + // Fire-and-forget, keyed on the same `request_id` as the metadata row the + // logging wrapper wrote to system.db. + if let Some(meta) = e.downcast_ref::().and_then(|le| le.raw_meta.as_ref()) { + let row = llm_request_payloads::PayloadRow { + request_id: request_id.clone(), + request_json: meta.request_body.as_ref().map(|v| v.to_string()).unwrap_or_default(), + request_headers: meta.request_headers.as_ref().map(|v| v.to_string()), + response_json: meta.response_body.as_ref().map(|v| v.to_string()), + response_headers: meta.response_headers.as_ref().map(|v| v.to_string()), + }; + let pool = Arc::clone(&self.db); + tokio::spawn(async move { + if let Err(e) = llm_request_payloads::insert(&pool, row).await { + tracing::warn!(error = %e, "llm_request_payloads: failed to insert error payload"); + } + }); + } + error!(session_id = self.session_id, client = %cur_name, error = %e, "LLM call failed"); self.llm_manager.mark_failure(cur_name, &e.to_string()).await; @@ -226,7 +247,7 @@ mod tests { use crate::chatbot::LlmError; fn http_err(status: u16, message: &str) -> anyhow::Error { - LlmError { status: Some(status), message: message.to_string() }.into() + LlmError { status: Some(status), message: message.to_string(), ..Default::default() }.into() } #[test]