From 067fecae15a11d112fd58913b517e72f4584bcbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 1 Sep 2026 17:20:15 +0200 Subject: [PATCH] feat(web_search): always continue on provider failure with a failed result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the web-search failure-policy configuration and always continue the agentic loop with a truthful failed tool result instead of rejecting the request. - Drop `provider_failure_mode` and `status_on_error` from the web-search config; collapse `SearchOutcome` to `Results` | `Failed`. - OpenAI Responses: emit a single public `web_search_call` with `status:"failed"` and feed the model "Web search unavailable."; upsert the accumulated output item by id so the final response never contains a duplicate call for the same search. - Anthropic Messages: append a `tool_result` with `is_error:true` carrying the same bounded message and re-enter the model loop. - Share the `SEARCH_UNAVAILABLE` constant from the neutral web_search module. - Update examples, generated filter docs, and test helpers; add regression coverage including a functional agentic-loop provider-failure test. BREAKING CHANGE: `provider_failure_mode` and `status_on_error` are removed from the web-search filter configuration; provider failures now always continue the loop with a failed tool result. Signed-off-by: Sébastien Han --- apis/src/anthropic/web_search/mod.rs | 68 ++--- apis/src/anthropic/web_search/tests.rs | 68 +++-- apis/src/openai/responses/web_search/mod.rs | 141 +++++++---- apis/src/openai/responses/web_search/tests.rs | 238 ++++++++++++++++++ apis/src/web_search/config.rs | 71 ------ apis/src/web_search/mod.rs | 4 + apis/src/web_search/provider.rs | 147 +++-------- docs/filters/anthropic_web_search.md | 4 - docs/filters/openai_web_search.md | 4 - .../anthropic/messages-web-search.yaml | 1 - .../configs/openai/responses/web-search.yaml | 3 - .../examples/anthropic_messages_web_search.rs | 67 ++++- .../suite/examples/openai_agentic_loop.rs | 125 +++++++++ xtask/src/filter_docs.rs | 2 +- 14 files changed, 640 insertions(+), 303 deletions(-) diff --git a/apis/src/anthropic/web_search/mod.rs b/apis/src/anthropic/web_search/mod.rs index 2e211b3ea6..17a9412e71 100644 --- a/apis/src/anthropic/web_search/mod.rs +++ b/apis/src/anthropic/web_search/mod.rs @@ -16,7 +16,7 @@ use serde::{Deserialize, de::IgnoredAny}; use serde_json::{Value, json}; use crate::web_search::{ - SearchClient, SearchContextSize, SearchOutcome, SearchResult, WebSearchFilterConfig, build_config, + SEARCH_UNAVAILABLE, SearchClient, SearchContextSize, SearchOutcome, WebSearchFilterConfig, build_config, format_search_results, }; @@ -159,8 +159,6 @@ struct ResponseEnvelope<'a> { /// api_key: ${WEB_SEARCH_API_KEY} /// default_context_size: medium /// timeout_ms: 10000 -/// provider_failure_mode: closed -/// status_on_error: 502 /// max_body_bytes: 67108864 /// ``` /// @@ -222,21 +220,14 @@ impl AnthropicWebSearchFilter { })) } - /// Execute one pending call and map provider failure policy to Messages semantics. - async fn execute_pending_search(&self, pending: &PendingSearch) -> Result, Rejection> { - let outcome = self - .search_client + /// Execute one pending call, returning the provider outcome. + /// + /// A provider failure never rejects the Messages response: the caller + /// appends a truthful `is_error` tool result so the loop can continue. + async fn execute_pending_search(&self, pending: &PendingSearch) -> SearchOutcome { + self.search_client .search(&pending.query, Some(self.default_context_size)) - .await; - match outcome { - SearchOutcome::Results(results) => Ok(results), - SearchOutcome::Skipped => Ok(Vec::new()), - SearchOutcome::Rejected { status } => Err(anthropic_rejection( - status, - "api_error", - "web search provider unavailable", - )), - } + .await } /// Execute a retained search and replace the IRR request body. @@ -281,13 +272,8 @@ impl AnthropicWebSearchFilter { "messages must be an array for web search re-entry", ))); } - let results = match self.execute_pending_search(&pending).await { - Ok(results) => results, - Err(rejection) => { - return Ok(FilterAction::Reject(rejection)); - }, - }; - if let Err(rejection) = append_search_turns(&mut request, assistant_content, pending, &results) { + let outcome = self.execute_pending_search(&pending).await; + if let Err(rejection) = append_search_turns(&mut request, assistant_content, pending, &outcome) { return Ok(FilterAction::Reject(rejection)); } let rebuilt = serde_json::to_vec(&request) @@ -524,7 +510,7 @@ fn append_search_turns( request: &mut Value, assistant_content: Vec, pending: PendingSearch, - results: &[SearchResult], + outcome: &SearchOutcome, ) -> Result<(), Rejection> { let Some(messages) = request.get_mut("messages").and_then(Value::as_array_mut) else { return Err(anthropic_rejection( @@ -533,19 +519,12 @@ fn append_search_turns( "messages must be an array for web search re-entry", )); }; - let content = if results.is_empty() { - "No search results found.".to_owned() - } else { - format_search_results(results) - }; let PendingSearch { id, query: _ } = pending; let mut assistant_turn = serde_json::Map::new(); assistant_turn.insert("role".to_owned(), Value::String("assistant".to_owned())); assistant_turn.insert("content".to_owned(), Value::Array(assistant_content)); messages.push(Value::Object(assistant_turn)); - messages.push(json!({"role":"user","content":[{ - "type":"tool_result","tool_use_id":id,"content":content - }]})); + messages.push(build_tool_result_turn(&id, outcome)); if request.get("tool_choice").is_some() && let Some(object) = request.as_object_mut() { @@ -554,6 +533,29 @@ fn append_search_turns( Ok(()) } +/// Build the user turn carrying the search tool result. +/// +/// A provider failure yields a truthful `is_error` result carrying the bounded +/// [`SEARCH_UNAVAILABLE`] message so the loop continues; a successful empty +/// search reports `No search results found.` without `is_error`. +fn build_tool_result_turn(tool_use_id: &str, outcome: &SearchOutcome) -> Value { + match outcome { + SearchOutcome::Results(results) => { + let content = if results.is_empty() { + "No search results found.".to_owned() + } else { + format_search_results(results) + }; + json!({"role":"user","content":[{ + "type":"tool_result","tool_use_id":tool_use_id,"content":content + }]}) + }, + SearchOutcome::Failed => json!({"role":"user","content":[{ + "type":"tool_result","tool_use_id":tool_use_id,"content":SEARCH_UNAVAILABLE,"is_error":true + }]}), + } +} + /// Publish the loop decision for the IRR transition table. fn set_action(ctx: &mut HttpFilterContext<'_>, action: &'static str) -> Result<(), FilterError> { ctx.filter_results diff --git a/apis/src/anthropic/web_search/tests.rs b/apis/src/anthropic/web_search/tests.rs index c35868ecdb..e4bc144fa8 100644 --- a/apis/src/anthropic/web_search/tests.rs +++ b/apis/src/anthropic/web_search/tests.rs @@ -27,13 +27,12 @@ default_context_size: medium AnthropicWebSearchFilter::from_config(&config).unwrap() } -fn test_filter_impl_with_base_url(base_url: &str, provider_failure_mode: &str) -> AnthropicWebSearchFilter { +fn test_filter_impl_with_base_url(base_url: &str) -> AnthropicWebSearchFilter { let config = serde_yaml::from_str(&format!( r#" provider: you api_key: test-key default_context_size: medium -provider_failure_mode: {provider_failure_mode} base_url: "{base_url}" "#, )) @@ -129,6 +128,10 @@ fn valid_you_body() -> String { .to_string() } +fn empty_you_body() -> String { + json!({"results": {"web": [], "news": []}}).to_string() +} + #[test] fn search_stub_reads_full_content_length_body() { let search = start_you_search_stub(200, valid_you_body()); @@ -507,12 +510,12 @@ async fn initial_request_body_is_not_mutated() { #[tokio::test] async fn pending_search_executes_and_appends_tool_result() { let search = start_you_search_stub(200, valid_you_body()); - let filter = test_filter_impl_with_base_url(search.base_url(), "closed"); + let filter = test_filter_impl_with_base_url(search.base_url()); let pending = pending_search("potato"); - let results = filter.execute_pending_search(&pending).await.unwrap(); + let outcome = filter.execute_pending_search(&pending).await; let mut rebuilt = base_request(); - append_search_turns(&mut rebuilt, assistant_content("potato"), pending, &results).unwrap(); + append_search_turns(&mut rebuilt, assistant_content("potato"), pending, &outcome).unwrap(); assert_eq!(rebuilt["model"], "openai/gpt-oss-20b"); assert_eq!(rebuilt["system"], "Answer with sources."); @@ -532,6 +535,10 @@ async fn pending_search_executes_and_appends_tool_result() { .unwrap() .contains("Potato - Wikipedia") ); + assert!( + messages[messages.len() - 1]["content"][0].get("is_error").is_none(), + "a successful search must not mark the tool result as an error" + ); assert_eq!(search.last_json()["query"], "potato"); assert!( search @@ -542,34 +549,51 @@ async fn pending_search_executes_and_appends_tool_result() { } #[tokio::test] -async fn closed_provider_failure_returns_anthropic_error() { +async fn provider_failure_appends_is_error_tool_result() { let search = start_you_search_stub(503, "unavailable".to_owned()); - let filter = test_filter_impl_with_base_url(search.base_url(), "closed"); + let filter = test_filter_impl_with_base_url(search.base_url()); let pending = pending_search("potato"); - let result = filter.execute_pending_search(&pending).await; + let outcome = filter.execute_pending_search(&pending).await; + assert!( + matches!(&outcome, SearchOutcome::Failed), + "a provider 5xx must map to a failed outcome, got {outcome:?}" + ); - let Err(rejection) = result else { - panic!("expected rejection"); - }; - assert_eq!(rejection.status, 502); - assert!(String::from_utf8_lossy(rejection.body.as_ref().unwrap()).contains("api_error")); + let mut rebuilt = base_request(); + append_search_turns(&mut rebuilt, assistant_content("potato"), pending, &outcome).unwrap(); + + let result_block = &rebuilt["messages"].as_array().unwrap().last().unwrap()["content"][0]; + assert_eq!(result_block["type"], "tool_result"); + assert_eq!(result_block["tool_use_id"], "toolu_search_1"); + assert_eq!(result_block["content"], "Web search unavailable."); + assert_eq!( + result_block["is_error"], true, + "a failed search must mark the tool result with is_error" + ); } #[tokio::test] -async fn open_provider_failure_appends_no_results_tool_result() { - let search = start_you_search_stub(503, "unavailable".to_owned()); - let filter = test_filter_impl_with_base_url(search.base_url(), "open"); +async fn empty_results_appends_no_results_tool_result() { + let search = start_you_search_stub(200, empty_you_body()); + let filter = test_filter_impl_with_base_url(search.base_url()); let pending = pending_search("potato"); - let results = filter.execute_pending_search(&pending).await.unwrap(); + let outcome = filter.execute_pending_search(&pending).await; + assert!( + matches!(&outcome, SearchOutcome::Results(results) if results.is_empty()), + "a successful empty search must be a zero-result outcome, got {outcome:?}" + ); + let mut rebuilt = base_request(); - append_search_turns(&mut rebuilt, assistant_content("potato"), pending, &results).unwrap(); + append_search_turns(&mut rebuilt, assistant_content("potato"), pending, &outcome).unwrap(); - let content = rebuilt["messages"].as_array().unwrap().last().unwrap()["content"][0]["content"] - .as_str() - .unwrap(); - assert_eq!(content, "No search results found."); + let result_block = &rebuilt["messages"].as_array().unwrap().last().unwrap()["content"][0]; + assert_eq!(result_block["content"], "No search results found."); + assert!( + result_block.get("is_error").is_none(), + "a successful empty search must not mark the tool result as an error" + ); } #[test] diff --git a/apis/src/openai/responses/web_search/mod.rs b/apis/src/openai/responses/web_search/mod.rs index 42b6e3e3ca..b6093cd76e 100644 --- a/apis/src/openai/responses/web_search/mod.rs +++ b/apis/src/openai/responses/web_search/mod.rs @@ -43,12 +43,9 @@ use serde_json::Value; use tracing::{debug, warn}; use super::state::ResponsesState; -use crate::{ - openai::responses::error::responses_error_rejection, - web_search::{ - SearchClient, SearchContextSize, SearchOutcome, SearchResult, WebSearchFilterConfig, build_config, - format_search_results, - }, +use crate::web_search::{ + SEARCH_UNAVAILABLE, SearchClient, SearchContextSize, SearchOutcome, SearchResult, WebSearchFilterConfig, + build_config, format_search_results, }; // ----------------------------------------------------------------------------- @@ -93,8 +90,6 @@ const INCLUDE_ACTION_SOURCES: &str = "web_search_call.action.sources"; /// api_key: ${WEB_SEARCH_API_KEY} /// default_context_size: medium /// timeout_ms: 10000 -/// provider_failure_mode: closed -/// status_on_error: 502 /// max_body_bytes: 67108864 /// ``` pub struct WebSearchFilter { @@ -162,26 +157,36 @@ impl WebSearchFilter { })) } - /// Execute a single web search call and append results to state. + /// Execute a single web search call and append its outcome to state. + /// + /// A provider failure never rejects the Response. The model instead + /// receives a truthful `failed` `web_search_call` plus a bounded failure + /// message so the agentic loop can continue. async fn execute_single_search( &self, ctx: &mut HttpFilterContext<'_>, call: &Value, context_size: SearchContextSize, - ) -> Result<(), FilterAction> { + ) { let call_id = call.get("id").and_then(Value::as_str).unwrap_or("ws_unknown"); let query = call.get("action").and_then(|a| a.get("query")).and_then(Value::as_str); let Some(query) = query else { warn!(call_id, "web_search_call missing action.query, skipping"); append_result(ctx, call_id, "incomplete", "", &[]); - return Ok(()); + return; }; - let results = resolve_search_outcome(&self.search_client, query, context_size, call_id, false).await?; - - append_result(ctx, call_id, "completed", query, &results); - Ok(()) + match self.search_client.search(query, Some(context_size)).await { + SearchOutcome::Results(results) => append_result(ctx, call_id, "completed", query, &results), + SearchOutcome::Failed => { + warn!( + call_id, + "web search provider failed; continuing with a failed tool result" + ); + append_failed(ctx, call_id, query); + }, + } } } @@ -241,9 +246,7 @@ impl HttpFilter for WebSearchFilter { debug!(count = calls.len(), "executing pending web search calls"); for call in &calls { - if let Err(rejection) = self.execute_single_search(ctx, call, context_size).await { - return Ok(rejection); - } + self.execute_single_search(ctx, call, context_size).await; } if let Some(state) = ctx.extensions.get_mut::() { @@ -281,56 +284,73 @@ impl HttpFilter for WebSearchFilter { } } -/// Append search results to [`ResponsesState`]. +/// Append a completed search turn to [`ResponsesState`]. +/// +/// An empty `results` slice is a successful zero-result search: the model +/// receives `No search results found.` and the public item stays `completed`. fn append_result(ctx: &mut HttpFilterContext<'_>, call_id: &str, status: &str, query: &str, results: &[SearchResult]) { - let include_sources = ctx - .extensions - .get::() - .is_some_and(|s| s.include.iter().any(|v| v == INCLUDE_ACTION_SOURCES)); - + let include_sources = include_action_sources(ctx); let output_item = build_output_item(call_id, status, query, results, include_sources); let tool_result = build_tool_result_message(call_id, results); + push_search_turn(ctx, output_item, tool_result); +} + +/// Append a failed `web_search_call` to [`ResponsesState`]. +/// +/// The public output item is marked `status: "failed"` and the model +/// receives the bounded [`SEARCH_UNAVAILABLE`] message so the agentic loop +/// continues without exposing provider details to the client. +fn append_failed(ctx: &mut HttpFilterContext<'_>, call_id: &str, query: &str) { + let include_sources = include_action_sources(ctx); + let output_item = build_output_item(call_id, "failed", query, &[], include_sources); + let tool_result = build_failed_tool_result_message(call_id); + push_search_turn(ctx, output_item, tool_result); +} + +/// Whether `action.sources` should be included in output items, per the +/// `web_search_call.action.sources` include gate. +fn include_action_sources(ctx: &HttpFilterContext<'_>) -> bool { + ctx.extensions + .get::() + .is_some_and(|s| s.include.iter().any(|v| v == INCLUDE_ACTION_SOURCES)) +} +/// Push a search turn — public output item plus model tool result — into state. +/// +/// The single clone is required: `messages` and `persisted_messages` are +/// distinct owners of the tool result. +fn push_search_turn(ctx: &mut HttpFilterContext<'_>, output_item: Value, tool_result: Value) { if let Some(state) = ctx.extensions.get_mut::() { state.messages.push(tool_result.clone()); state.persisted_messages.push(tool_result); - state.accumulated_output.push(output_item); + upsert_output_item(&mut state.accumulated_output, output_item); } } -// ----------------------------------------------------------------------------- -// Helpers -// ----------------------------------------------------------------------------- - -/// Execute a search and resolve its outcome to a result list. +/// Replace the accumulated `web_search_call` sharing this id, or append it. /// -/// Returns `Err(FilterAction)` when the search is rejected under -/// closed failure mode. -pub(crate) async fn resolve_search_outcome( - search_client: &SearchClient, - query: &str, - context_size: SearchContextSize, - call_id: &str, - streaming: bool, -) -> Result, FilterAction> { - match search_client.search(query, Some(context_size)).await { - SearchOutcome::Results(r) => Ok(r), - SearchOutcome::Skipped => { - warn!(call_id, "search skipped (open failure mode)"); - Ok(Vec::new()) - }, - SearchOutcome::Rejected { status } => { - warn!(call_id, status, "search rejected (closed failure mode)"); - Err(FilterAction::Reject(responses_error_rejection( - status, - "server_error", - "web search provider unavailable", - streaming, - ))) - }, +/// The response phase (`agentic_loop::collect_output_items`) already +/// accumulated the model's placeholder `web_search_call` for this id. Updating +/// it in place keeps exactly one public item per call, rather than emitting a +/// contradictory `completed` + `failed` pair for the same id. When no +/// placeholder exists (isolated unit contexts), the item is appended. +fn upsert_output_item(accumulated: &mut Vec, output_item: Value) { + if let Some(id) = output_item.get("id").and_then(Value::as_str) + && let Some(slot) = accumulated.iter_mut().find(|item| { + item.get("type").and_then(Value::as_str) == Some("web_search_call") + && item.get("id").and_then(Value::as_str) == Some(id) + }) + { + *slot = output_item; + return; } + accumulated.push(output_item); } +// ----------------------------------------------------------------------------- +// Helpers +// ----------------------------------------------------------------------------- + /// Emit a `web_search_call` status update via filter results. #[cfg_attr(not(test), expect(dead_code, reason = "reserved for per-call status tracking"))] pub(crate) fn emit_status(ctx: &mut HttpFilterContext<'_>, call_id: &str, status: &str) { @@ -396,6 +416,19 @@ pub(crate) fn build_tool_result_message(call_id: &str, results: &[SearchResult]) }) } +/// Build a failed tool result message to feed the model. +/// +/// Carries the bounded [`SEARCH_UNAVAILABLE`] message so the agentic loop +/// continues without exposing provider details to the client. +pub(crate) fn build_failed_tool_result_message(call_id: &str) -> Value { + serde_json::json!({ + "type": "web_search_call", + "id": call_id, + "status": "failed", + "output": SEARCH_UNAVAILABLE, + }) +} + /// Write the loop control action to filter results. fn set_action(ctx: &mut HttpFilterContext<'_>, action: &'static str) -> Result<(), FilterError> { ctx.filter_results diff --git a/apis/src/openai/responses/web_search/tests.rs b/apis/src/openai/responses/web_search/tests.rs index e602fbfb13..8c9508280c 100644 --- a/apis/src/openai/responses/web_search/tests.rs +++ b/apis/src/openai/responses/web_search/tests.rs @@ -333,6 +333,38 @@ fn spawn_brave_mock(listener: std::net::TcpListener) { }); } +/// Serve one `(status, body)` per sequential search callout. +/// +/// Each entry answers exactly one connection, letting a single test drive a +/// mixed batch where earlier calls succeed and later calls fail. +fn spawn_search_responses(listener: std::net::TcpListener, responses: Vec<(u16, String)>) { + use std::io::{Read as _, Write as _}; + std::thread::spawn(move || { + for (status, body) in responses { + let (mut stream, _) = listener.accept().unwrap(); + let mut buf = [0_u8; 4096]; + let _n = stream.read(&mut buf).unwrap(); + let response = format!( + "HTTP/1.1 {status} Test\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + stream.write_all(response.as_bytes()).unwrap(); + } + }); +} + +/// A single Brave web result body for search-execution tests. +fn brave_ok_body() -> String { + serde_json::json!({ + "web": {"results": [{ + "title": "Rust Lang", + "url": "https://rust-lang.org", + "description": "Systems programming language" + }]} + }) + .to_string() +} + #[tokio::test] async fn on_request_body_executes_search_and_populates_state() { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); @@ -451,6 +483,175 @@ async fn on_request_body_missing_query_produces_incomplete_status() { ); } +#[tokio::test] +async fn on_request_body_provider_failure_produces_failed_item_and_truthful_input() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + spawn_search_responses(listener, vec![(503, String::new())]); + + let yaml = make_filter_yaml_with_base_url("brave", "test-key", &format!("http://{addr}")); + let filter = WebSearchFilter::from_config(&yaml).unwrap(); + + let req = crate::test_utils::make_request(http::Method::POST, "/v1/responses"); + let mut ctx = crate::test_utils::make_filter_context(&req); + + let body = serde_json::json!({"model": "gpt-4o", "input": "test"}); + let mut state = ResponsesState::from_request_body(body); + // The response phase already accumulated the model's placeholder call. + state.accumulated_output = vec![serde_json::json!({ + "type": "web_search_call", + "id": "ws_fail_1", + "status": "completed", + "action": {"type": "search", "query": "rust language"} + })]; + state.web_search_calls = vec![serde_json::json!({ + "type": "web_search_call", + "id": "ws_fail_1", + "action": {"type": "search", "query": "rust language"} + })]; + ctx.extensions.insert(state); + + let action = filter.on_request_body(&mut ctx, &mut None, true).await.unwrap(); + assert!( + matches!(action, FilterAction::Continue), + "a provider failure must never reject the Response" + ); + + let state = ctx.extensions.get::().unwrap(); + assert!( + state.web_search_calls.is_empty(), + "calls should be cleared after execution" + ); + + // The placeholder is replaced in place: exactly one public item, marked failed. + assert_eq!( + state.accumulated_output.len(), + 1, + "the failed outcome must replace the placeholder, not duplicate it" + ); + let output = &state.accumulated_output[0]; + assert_eq!(output["type"], "web_search_call"); + assert_eq!(output["id"], "ws_fail_1"); + assert_eq!(output["status"], "failed"); + assert_eq!(output["action"]["query"], "rust language"); + + // The model receives a bounded failure message, not provider details. + let tool_result = state.messages.last().unwrap(); + assert_eq!(tool_result["status"], "failed"); + assert_eq!(tool_result["output"], "Web search unavailable."); + assert_eq!( + state.persisted_messages.last().unwrap()["output"], + "Web search unavailable.", + "persisted history mirrors the model input" + ); +} + +#[tokio::test] +async fn on_request_body_empty_results_remain_completed() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let empty_body = serde_json::json!({"web": {"results": []}}).to_string(); + spawn_search_responses(listener, vec![(200, empty_body)]); + + let yaml = make_filter_yaml_with_base_url("brave", "test-key", &format!("http://{addr}")); + let filter = WebSearchFilter::from_config(&yaml).unwrap(); + + let req = crate::test_utils::make_request(http::Method::POST, "/v1/responses"); + let mut ctx = crate::test_utils::make_filter_context(&req); + + let body = serde_json::json!({"model": "gpt-4o", "input": "test"}); + let mut state = ResponsesState::from_request_body(body); + state.web_search_calls = vec![serde_json::json!({ + "type": "web_search_call", + "id": "ws_empty_1", + "action": {"type": "search", "query": "rust language"} + })]; + ctx.extensions.insert(state); + + let action = filter.on_request_body(&mut ctx, &mut None, true).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + + let state = ctx.extensions.get::().unwrap(); + let output = &state.accumulated_output[0]; + assert_eq!( + output["status"], "completed", + "a successful zero-result search stays completed" + ); + let tool_result = state.messages.last().unwrap(); + assert_eq!(tool_result["status"], "completed"); + assert_eq!(tool_result["output"], "No search results found."); +} + +#[tokio::test] +async fn on_request_body_mixed_batch_preserves_completed_and_failed() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + spawn_search_responses(listener, vec![(200, brave_ok_body()), (503, String::new())]); + + let yaml = make_filter_yaml_with_base_url("brave", "test-key", &format!("http://{addr}")); + let filter = WebSearchFilter::from_config(&yaml).unwrap(); + + let req = crate::test_utils::make_request(http::Method::POST, "/v1/responses"); + let mut ctx = crate::test_utils::make_filter_context(&req); + + let body = serde_json::json!({"model": "gpt-4o", "input": "test"}); + let mut state = ResponsesState::from_request_body(body); + // Both placeholders were accumulated during the response phase. + state.accumulated_output = vec![ + serde_json::json!({ + "type": "web_search_call", + "id": "ws_ok", + "status": "completed", + "action": {"type": "search", "query": "rust language"} + }), + serde_json::json!({ + "type": "web_search_call", + "id": "ws_fail", + "status": "completed", + "action": {"type": "search", "query": "rust crates"} + }), + ]; + state.web_search_calls = vec![ + serde_json::json!({ + "type": "web_search_call", + "id": "ws_ok", + "action": {"type": "search", "query": "rust language"} + }), + serde_json::json!({ + "type": "web_search_call", + "id": "ws_fail", + "action": {"type": "search", "query": "rust crates"} + }), + ]; + ctx.extensions.insert(state); + + let action = filter.on_request_body(&mut ctx, &mut None, true).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + + let state = ctx.extensions.get::().unwrap(); + assert_eq!( + state.accumulated_output.len(), + 2, + "each placeholder is replaced in place, so no duplicates are produced" + ); + + let completed = &state.accumulated_output[0]; + assert_eq!(completed["id"], "ws_ok"); + assert_eq!(completed["status"], "completed"); + + let failed = &state.accumulated_output[1]; + assert_eq!(failed["id"], "ws_fail"); + assert_eq!(failed["status"], "failed"); + + // Each model input reflects its own call outcome; both tool results are + // appended after the original input message, preserving order. + let appended = &state.messages[state.messages.len() - 2..]; + assert_eq!(appended[0]["status"], "completed"); + assert!(appended[0]["output"].as_str().unwrap().contains("Rust Lang")); + assert_eq!(appended[1]["status"], "failed"); + assert_eq!(appended[1]["output"], "Web search unavailable."); +} + // ----------------------------------------------------------------------------- // Output formatting tests // ----------------------------------------------------------------------------- @@ -531,6 +732,43 @@ fn build_tool_result_message_with_results() { assert!(output.contains("A description")); } +#[test] +fn build_failed_tool_result_message_carries_bounded_notice() { + let msg = build_failed_tool_result_message("ws_123"); + assert_eq!(msg["type"], "web_search_call"); + assert_eq!(msg["id"], "ws_123"); + assert_eq!(msg["status"], "failed"); + assert_eq!( + msg["output"], "Web search unavailable.", + "failed tool result must feed the model the bounded notice" + ); +} + +#[test] +fn upsert_output_item_replaces_matching_web_search_call() { + let mut accumulated = vec![ + serde_json::json!({"type": "message", "id": "msg_1"}), + serde_json::json!({"type": "web_search_call", "id": "ws_1", "status": "completed"}), + ]; + upsert_output_item( + &mut accumulated, + serde_json::json!({"type": "web_search_call", "id": "ws_1", "status": "failed"}), + ); + assert_eq!(accumulated.len(), 2, "matching id replaces rather than appends"); + assert_eq!(accumulated[1]["status"], "failed"); +} + +#[test] +fn upsert_output_item_appends_when_no_match() { + let mut accumulated = vec![serde_json::json!({"type": "web_search_call", "id": "ws_1"})]; + upsert_output_item( + &mut accumulated, + serde_json::json!({"type": "web_search_call", "id": "ws_2", "status": "failed"}), + ); + assert_eq!(accumulated.len(), 2, "a new id appends a fresh item"); + assert_eq!(accumulated[1]["id"], "ws_2"); +} + #[test] fn format_search_results_multiple() { let results = vec![ diff --git a/apis/src/web_search/config.rs b/apis/src/web_search/config.rs index b5ae24c3b2..bb13a10e68 100644 --- a/apis/src/web_search/config.rs +++ b/apis/src/web_search/config.rs @@ -13,9 +13,6 @@ use serde::Deserialize; /// Default callout timeout (10 seconds — search APIs can be slow). const DEFAULT_TIMEOUT_MS: u64 = 10_000; -/// Default HTTP status when the search callout fails in closed mode. -const DEFAULT_STATUS_ON_ERROR: u16 = 502; - // ----------------------------------------------------------------------------- // SearchProvider // ----------------------------------------------------------------------------- @@ -87,20 +84,6 @@ impl SearchContextSize { } } -// ----------------------------------------------------------------------------- -// FailureMode -// ----------------------------------------------------------------------------- - -/// What happens when a search callout fails. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] -#[serde(rename_all = "snake_case")] -pub(crate) enum FailureMode { - /// Reject the request on search failure (default). - Closed, - /// Continue without search results on failure. - Open, -} - // ----------------------------------------------------------------------------- // WebSearchFilterConfig (YAML deserialization) // ----------------------------------------------------------------------------- @@ -128,14 +111,6 @@ pub(crate) struct WebSearchFilterConfig { #[serde(default)] pub(crate) max_body_bytes: Option, - /// Failure mode for search provider callouts. - #[serde(default)] - pub(crate) provider_failure_mode: Option, - - /// HTTP status code to return when rejecting on error. - #[serde(default)] - pub(crate) status_on_error: Option, - /// Override the provider's default API base URL. #[serde(default)] pub(crate) base_url: Option, @@ -163,12 +138,6 @@ pub(crate) struct ValidatedConfig { /// Maximum request body bytes to buffer. pub max_body_bytes: usize, - /// Failure mode for search callouts. - pub failure_mode: FailureMode, - - /// HTTP status on error. - pub status_on_error: u16, - /// Override the provider's default API base URL. pub base_url: Option, } @@ -181,8 +150,6 @@ impl std::fmt::Debug for ValidatedConfig { .field("default_context_size", &self.default_context_size) .field("timeout_ms", &self.timeout_ms) .field("max_body_bytes", &self.max_body_bytes) - .field("failure_mode", &self.failure_mode) - .field("status_on_error", &self.status_on_error) .field("base_url", &self.base_url) .finish() } @@ -217,8 +184,6 @@ fn build_validated_config( default_context_size: validate_context_size(filter_name, raw.default_context_size.as_deref())?, timeout_ms: validate_timeout_ms(filter_name, raw.timeout_ms)?, max_body_bytes: validate_max_body_bytes_field(filter_name, raw.max_body_bytes)?, - failure_mode: raw.provider_failure_mode.unwrap_or(FailureMode::Closed), - status_on_error: validate_status_on_error(filter_name, raw.status_on_error)?, base_url: raw.base_url.clone(), }) } @@ -234,17 +199,6 @@ fn validate_timeout_ms(filter_name: &'static str, raw: Option) -> Result) -> Result { - let value = raw.unwrap_or(DEFAULT_STATUS_ON_ERROR); - if !(100..=599).contains(&value) { - return Err(FilterError::from(format!( - "{filter_name}: status_on_error must be between 100 and 599, got {value}" - ))); - } - Ok(value) -} - /// Validate `default_context_size`, defaulting to `Medium` when /// absent and rejecting unknown values. fn validate_context_size(filter_name: &'static str, raw: Option<&str>) -> Result { @@ -306,8 +260,6 @@ mod tests { default_context_size: None, timeout_ms: None, max_body_bytes: None, - provider_failure_mode: None, - status_on_error: None, base_url: None, } } @@ -320,8 +272,6 @@ mod tests { assert_eq!(cfg.default_context_size, SearchContextSize::Medium); assert_eq!(cfg.timeout_ms, DEFAULT_TIMEOUT_MS); assert_eq!(cfg.max_body_bytes, MAX_JSON_BODY_BYTES); - assert_eq!(cfg.failure_mode, FailureMode::Closed); - assert_eq!(cfg.status_on_error, DEFAULT_STATUS_ON_ERROR); } #[test] @@ -345,16 +295,6 @@ mod tests { ); } - #[test] - fn parse_config_preserves_provider_failure_mode() { - let yaml = serde_yaml::from_str("\nprovider: brave\napi_key: test-key\nprovider_failure_mode: open\n").unwrap(); - - let raw: WebSearchFilterConfig = parse_filter_config("openai_web_search", &yaml).unwrap(); - let validated = build_config("openai_web_search", &raw).unwrap(); - - assert_eq!(validated.failure_mode, FailureMode::Open); - } - #[test] fn build_config_rejects_zero_timeout() { let mut cfg = base_config(); @@ -372,25 +312,14 @@ mod tests { ); } - #[test] - fn build_config_rejects_invalid_status() { - let mut cfg = base_config(); - cfg.status_on_error = Some(999); - assert!(build_config("openai_web_search", &cfg).is_err()); - } - #[test] fn build_config_custom_values() { let mut cfg = base_config(); cfg.default_context_size = Some("high".into()); cfg.timeout_ms = Some(15_000); - cfg.provider_failure_mode = Some(FailureMode::Open); - cfg.status_on_error = Some(503); let validated = build_config("openai_web_search", &cfg).unwrap(); assert_eq!(validated.default_context_size, SearchContextSize::High); assert_eq!(validated.timeout_ms, 15_000); - assert_eq!(validated.failure_mode, FailureMode::Open); - assert_eq!(validated.status_on_error, 503); } #[test] diff --git a/apis/src/web_search/mod.rs b/apis/src/web_search/mod.rs index 48e1b2cc8b..e76087c053 100644 --- a/apis/src/web_search/mod.rs +++ b/apis/src/web_search/mod.rs @@ -11,6 +11,10 @@ use std::fmt::Write as _; pub(crate) use config::{SearchContextSize, ValidatedConfig, WebSearchFilterConfig, build_config}; pub(crate) use provider::{SearchClient, SearchOutcome, SearchResult}; +/// Bounded tool-result message fed to the model when a search provider fails, +/// so both provider loops continue with a truthful failure instead of rejecting. +pub(crate) const SEARCH_UNAVAILABLE: &str = "Web search unavailable."; + /// Format search results as readable text for a model prompt. pub(crate) fn format_search_results(results: &[SearchResult]) -> String { let mut output = String::with_capacity(results.len() * 200); diff --git a/apis/src/web_search/provider.rs b/apis/src/web_search/provider.rs index 935bece23a..fa8ad3650b 100644 --- a/apis/src/web_search/provider.rs +++ b/apis/src/web_search/provider.rs @@ -19,7 +19,7 @@ use tracing::{debug, warn}; use super::{ ValidatedConfig, - config::{FailureMode, SearchContextSize, SearchProvider}, + config::{SearchContextSize, SearchProvider}, }; use crate::subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}; @@ -49,15 +49,12 @@ pub(crate) struct SearchResult { /// Outcome of a search execution. #[derive(Debug)] pub(crate) enum SearchOutcome { - /// Search succeeded with results. + /// Search succeeded. An empty vector is a successful zero-result search. Results(Vec), - /// Search failed but failure mode is open — continue without results. - Skipped, - /// Search failed and failure mode is closed — reject the request. - Rejected { - /// HTTP status code to return. - status: u16, - }, + /// Search failed — timeout, transport error, non-2xx status, oversized + /// response, or unparseable body. Callers continue with a truthful failed + /// tool result rather than exposing provider details to the client. + Failed, } // ----------------------------------------------------------------------------- @@ -78,10 +75,6 @@ pub(crate) struct SearchClient { api_key: SecretString, /// Default search context size. default_context_size: SearchContextSize, - /// Failure mode governing what happens on errors. - failure_mode: FailureMode, - /// HTTP status to return on rejection. - status_on_error: u16, /// Override the provider's default API base URL. base_url: Option, } @@ -94,8 +87,6 @@ impl std::fmt::Debug for SearchClient { .field("provider", &self.provider) .field("api_key", &"[REDACTED]") .field("default_context_size", &self.default_context_size) - .field("failure_mode", &self.failure_mode) - .field("status_on_error", &self.status_on_error) .field("base_url", &self.base_url) .finish() } @@ -121,8 +112,6 @@ impl SearchClient { provider: config.provider, api_key: config.api_key.clone(), default_context_size: config.default_context_size, - failure_mode: config.failure_mode, - status_on_error: config.status_on_error, base_url: config.base_url.clone(), }) } @@ -153,6 +142,10 @@ impl SearchClient { } /// Map a sub-request result to a [`SearchOutcome`]. + /// + /// Non-2xx statuses and transport errors (including timeouts and + /// oversized responses) map to [`SearchOutcome::Failed`]. Detailed + /// diagnostics are logged; provider specifics never reach the client. fn map_search_result(&self, result: Result) -> SearchOutcome { match result { Ok(response) if (200..300).contains(&(response.status as usize)) => self.parse_response(&response.body), @@ -162,27 +155,15 @@ impl SearchClient { status = response.status, "search callout returned non-2xx" ); - self.transport_failure_outcome() + SearchOutcome::Failed }, Err(e) => { warn!(provider = self.provider.as_str(), error = %e, "search callout failed"); - self.transport_failure_outcome() + SearchOutcome::Failed }, } } - /// Outcome for a transport or non-2xx failure. Under closed - /// mode this is a rejection; under open mode search is silently - /// skipped. - fn transport_failure_outcome(&self) -> SearchOutcome { - match self.failure_mode { - FailureMode::Closed => SearchOutcome::Rejected { - status: self.status_on_error, - }, - FailureMode::Open => SearchOutcome::Skipped, - } - } - /// Build a Brave Search API request. fn build_brave_request(&self, query: &str, count: u32) -> (String, SubRequest) { let encoded_query = percent_encoding::utf8_percent_encode(query, percent_encoding::NON_ALPHANUMERIC); @@ -281,7 +262,7 @@ impl SearchClient { Ok(v) => v, Err(e) => { warn!(provider = self.provider.as_str(), error = %e, "failed to parse search response"); - return self.parse_failure_outcome(); + return SearchOutcome::Failed; }, }; @@ -299,18 +280,6 @@ impl SearchClient { SearchOutcome::Results(results) } - - /// Outcome for a response that arrived as 2xx but could not be - /// parsed. Under closed mode this is an error; under open mode - /// search is silently skipped. - fn parse_failure_outcome(&self) -> SearchOutcome { - match self.failure_mode { - FailureMode::Closed => SearchOutcome::Rejected { - status: self.status_on_error, - }, - FailureMode::Open => SearchOutcome::Skipped, - } - } } // ----------------------------------------------------------------------------- @@ -505,8 +474,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: None, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); @@ -560,8 +527,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: None, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()); @@ -576,8 +541,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: None, }; @@ -599,8 +562,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: Some("http://localhost:9999".into()), }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); @@ -619,8 +580,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: Some("http://localhost:9999".into()), }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); @@ -639,8 +598,6 @@ mod tests { default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: Some("http://localhost:9999".into()), }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); @@ -652,54 +609,48 @@ mod tests { } #[test] - fn parse_failure_closed_mode_rejects() { + fn parse_failure_returns_failed() { let config = ValidatedConfig { provider: SearchProvider::Brave, api_key: SecretString::from("test-key".to_owned()), default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Closed, - status_on_error: 502, base_url: None, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let outcome = client.parse_response(b"not json"); assert!( - matches!(outcome, SearchOutcome::Rejected { status: 502 }), - "closed mode should reject on parse failure" + matches!(outcome, SearchOutcome::Failed), + "an unparseable 2xx body should map to Failed: {outcome:?}" ); } #[test] - fn parse_failure_open_mode_skips() { + fn parse_empty_results_is_successful_zero_result_search() { let config = ValidatedConfig { provider: SearchProvider::Brave, api_key: SecretString::from("test-key".to_owned()), default_context_size: SearchContextSize::Medium, timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, - failure_mode: FailureMode::Open, - status_on_error: 502, base_url: None, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); - let outcome = client.parse_response(b"not json"); + let outcome = client.parse_response(br#"{"web":{"results":[]}}"#); assert!( - matches!(outcome, SearchOutcome::Skipped), - "open mode should skip on parse failure" + matches!(&outcome, SearchOutcome::Results(results) if results.is_empty()), + "a parseable 2xx body with zero results is a successful empty search: {outcome:?}" ); } - fn test_search_client(failure_mode: FailureMode) -> SearchClient { + fn test_search_client() -> SearchClient { let config = ValidatedConfig { provider: SearchProvider::Brave, api_key: SecretString::from("test-key".to_owned()), default_context_size: SearchContextSize::Medium, timeout_ms: 1000, max_body_bytes: 64 * 1024 * 1024, - failure_mode, - status_on_error: 502, base_url: None, }; SearchClient::from_config("test", &config, test_subrequest_client()).unwrap() @@ -732,7 +683,7 @@ mod tests { .to_string(), ); - let client = test_search_client(FailureMode::Closed); + let client = test_search_client(); let url = format!("http://{addr}/res/v1/web/search?q=test&count=5"); let request = SubRequest { method: http::Method::GET, @@ -749,34 +700,12 @@ mod tests { } #[tokio::test] - async fn search_non_2xx_closed_rejects() { + async fn search_non_2xx_returns_failed() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); spawn_http_server(listener, 500, "internal error"); - let client = test_search_client(FailureMode::Closed); - let url = format!("http://{addr}/search"); - let request = SubRequest { - method: http::Method::GET, - uri: "/".parse().unwrap(), - headers: HeaderMap::new(), - body: Bytes::new(), - }; - - let outcome = client.execute_search(&url, request).await; - assert!( - matches!(outcome, SearchOutcome::Rejected { status: 502 }), - "non-2xx under closed mode should reject: {outcome:?}" - ); - } - - #[tokio::test] - async fn search_non_2xx_open_skips() { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - spawn_http_server(listener, 429, "rate limited"); - - let client = test_search_client(FailureMode::Open); + let client = test_search_client(); let url = format!("http://{addr}/search"); let request = SubRequest { method: http::Method::GET, @@ -787,13 +716,13 @@ mod tests { let outcome = client.execute_search(&url, request).await; assert!( - matches!(outcome, SearchOutcome::Skipped), - "non-2xx under open mode should skip: {outcome:?}" + matches!(outcome, SearchOutcome::Failed), + "a non-2xx status should map to Failed: {outcome:?}" ); } #[tokio::test] - async fn search_connection_failure_closed_rejects() { + async fn search_connection_failure_returns_failed() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); std::thread::spawn(move || { @@ -801,7 +730,7 @@ mod tests { drop(stream); }); - let client = test_search_client(FailureMode::Closed); + let client = test_search_client(); let url = format!("http://{addr}/search"); let request = SubRequest { method: http::Method::GET, @@ -812,13 +741,13 @@ mod tests { let outcome = client.execute_search(&url, request).await; assert!( - matches!(outcome, SearchOutcome::Rejected { status: 502 }), - "connection failure under closed mode should reject: {outcome:?}" + matches!(outcome, SearchOutcome::Failed), + "a transport failure should map to Failed: {outcome:?}" ); } #[tokio::test] - async fn search_timeout_open_skips() { + async fn search_timeout_returns_failed() { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); tokio::spawn(async move { @@ -826,7 +755,7 @@ mod tests { tokio::time::sleep(Duration::from_secs(5)).await; }); - let mut client = test_search_client(FailureMode::Open); + let mut client = test_search_client(); client.timeout = Duration::from_millis(50); let url = format!("http://{addr}/search"); let request = SubRequest { @@ -838,13 +767,13 @@ mod tests { let outcome = client.execute_search(&url, request).await; assert!( - matches!(outcome, SearchOutcome::Skipped), - "timeout under open mode should skip: {outcome:?}" + matches!(outcome, SearchOutcome::Failed), + "a timeout should map to Failed: {outcome:?}" ); } #[tokio::test] - async fn search_oversized_response_closed_rejects() { + async fn search_oversized_response_returns_failed() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); std::thread::spawn(move || { @@ -860,7 +789,7 @@ mod tests { stream.write_all(&body).unwrap(); }); - let client = test_search_client(FailureMode::Closed); + let client = test_search_client(); let url = format!("http://{addr}/search"); let request = SubRequest { method: http::Method::GET, @@ -871,8 +800,8 @@ mod tests { let outcome = client.execute_search(&url, request).await; assert!( - matches!(outcome, SearchOutcome::Rejected { status: 502 }), - "oversized response under closed mode should reject: {outcome:?}" + matches!(outcome, SearchOutcome::Failed), + "an oversized response should map to Failed: {outcome:?}" ); } } diff --git a/docs/filters/anthropic_web_search.md b/docs/filters/anthropic_web_search.md index e6944118c8..877b547701 100644 --- a/docs/filters/anthropic_web_search.md +++ b/docs/filters/anthropic_web_search.md @@ -14,8 +14,6 @@ Executes server-owned `WebSearch` tool calls in an Anthropic Messages loop. | `default_context_size` | string | no | Default search context size when the client omits it. | | `timeout_ms` | integer | no | Callout timeout in milliseconds. | | `max_body_bytes` | integer | no | Maximum request body bytes to buffer. | -| `provider_failure_mode` | `closed` \| `open` | no | Failure mode for search provider callouts. | -| `status_on_error` | integer | no | HTTP status code to return when rejecting on error. | | `base_url` | string | no | Override the provider's default API base URL. | ## Examples @@ -36,8 +34,6 @@ provider: you api_key: ${WEB_SEARCH_API_KEY} default_context_size: medium timeout_ms: 10000 -provider_failure_mode: closed -status_on_error: 502 max_body_bytes: 67108864 ``` diff --git a/docs/filters/openai_web_search.md b/docs/filters/openai_web_search.md index ebc1424d3f..b31eff0d4b 100644 --- a/docs/filters/openai_web_search.md +++ b/docs/filters/openai_web_search.md @@ -18,8 +18,6 @@ Detects pending web search calls in the response phase and executes them on re-e | `default_context_size` | string | no | Default search context size when the client omits it. | | `timeout_ms` | integer | no | Callout timeout in milliseconds. | | `max_body_bytes` | integer | no | Maximum request body bytes to buffer. | -| `provider_failure_mode` | `closed` \| `open` | no | Failure mode for search provider callouts. | -| `status_on_error` | integer | no | HTTP status code to return when rejecting on error. | | `base_url` | string | no | Override the provider's default API base URL. | ## Examples @@ -40,7 +38,5 @@ provider: brave api_key: ${WEB_SEARCH_API_KEY} default_context_size: medium timeout_ms: 10000 -provider_failure_mode: closed -status_on_error: 502 max_body_bytes: 67108864 ``` diff --git a/examples/configs/anthropic/messages-web-search.yaml b/examples/configs/anthropic/messages-web-search.yaml index 9f740810e5..7d5f083449 100644 --- a/examples/configs/anthropic/messages-web-search.yaml +++ b/examples/configs/anthropic/messages-web-search.yaml @@ -39,7 +39,6 @@ filter_chains: api_key: ${WEB_SEARCH_API_KEY} default_context_size: medium timeout_ms: 10000 - provider_failure_mode: closed - filter: anthropic_messages_protocol default_version: "2023-06-01" - filter: router diff --git a/examples/configs/openai/responses/web-search.yaml b/examples/configs/openai/responses/web-search.yaml index e6ab0bfc10..73690aa0b6 100644 --- a/examples/configs/openai/responses/web-search.yaml +++ b/examples/configs/openai/responses/web-search.yaml @@ -12,7 +12,6 @@ # api_key: Provider API key (supports ${ENV_VAR} syntax) # default_context_size: How many results to return (low/medium/high) # timeout_ms: Callout timeout in milliseconds -# provider_failure_mode: closed (reject on error) or open (skip on error) listeners: - name: ai-gateway @@ -28,8 +27,6 @@ filter_chains: api_key: ${WEB_SEARCH_API_KEY} default_context_size: medium timeout_ms: 10000 - provider_failure_mode: closed - status_on_error: 502 - filter: router routes: - path_prefix: "/" diff --git a/tests/integration/tests/suite/examples/anthropic_messages_web_search.rs b/tests/integration/tests/suite/examples/anthropic_messages_web_search.rs index dfa80ecf25..09d73b8420 100644 --- a/tests/integration/tests/suite/examples/anthropic_messages_web_search.rs +++ b/tests/integration/tests/suite/examples/anthropic_messages_web_search.rs @@ -27,11 +27,22 @@ impl SearchStub { } fn start_many(responses: &[Value]) -> Self { + Self::start_many_with_status(responses, "200 OK") + } + + /// Serve a single failing HTTP response so the loop maps the provider + /// callout to [`SearchOutcome::Failed`] and continues with an error result. + fn start_failing() -> Self { + Self::start_many_with_status(&[json!({"error": "service unavailable"})], "503 Service Unavailable") + } + + fn start_many_with_status(responses: &[Value], status_line: &str) -> Self { let listener = TcpListener::bind("127.0.0.1:0").expect("bind You.com stub"); let port = listener.local_addr().expect("stub address").port(); let requests = Arc::new(Mutex::new(Vec::new())); let captured = Arc::clone(&requests); let bodies = responses.iter().map(Value::to_string).collect::>(); + let status_line = status_line.to_owned(); std::thread::spawn(move || { for body in bodies { let (mut stream, _) = listener.accept().expect("accept search request"); @@ -40,7 +51,7 @@ impl SearchStub { .expect("capture search request") .push(read_http_request(&mut stream)); let response = format!( - "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + "HTTP/1.1 {status_line}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len() ); stream.write_all(response.as_bytes()).expect("write search response"); @@ -209,6 +220,60 @@ fn messages_web_search_round_trip_re_enters_the_model() { ); } +#[test] +fn provider_failure_appends_is_error_tool_result_and_re_enters_model() { + let fixture = fixture(); + let model = StatefulCapturingBackend::new(vec![ + (200, fixture["first_model_response"].to_string()), + (200, fixture["final_model_response"].to_string()), + ]) + .start_with_shutdown(); + let search = SearchStub::start_failing(); + let proxy_port = free_port(); + let proxy = start_proxy(&load_config(proxy_port, model.port(), search.port())); + + let raw = http_send( + proxy.addr(), + &json_post("/v1/messages", &fixture["initial_request"].to_string()), + ); + + assert_eq!( + parse_status(&raw), + 200, + "a provider failure must not reject the request" + ); + let client_response: Value = serde_json::from_str(&parse_body(&raw)).expect("client response JSON"); + assert_eq!( + client_response, fixture["final_model_response"], + "the loop must return the model's post-failure answer" + ); + + let requests = model.requests(); + assert_eq!( + requests.len(), + 2, + "the loop must re-enter the model after the search fails" + ); + let second: Value = serde_json::from_str(&requests[1].body).expect("second model request JSON"); + let messages = second["messages"].as_array().expect("Messages history"); + let tool_result = &messages[messages.len() - 1]["content"][0]; + assert_eq!(tool_result["type"], "tool_result"); + assert_eq!(tool_result["tool_use_id"], "toolu_web_search_01"); + assert_eq!( + tool_result["is_error"], true, + "a provider failure must produce a truthful is_error result" + ); + assert_eq!( + tool_result["content"], "Web search unavailable.", + "the model must receive the bounded failure notice" + ); + assert_eq!( + search.request_count(), + 1, + "the failed search still counts as one callout" + ); +} + #[test] fn caller_anthropic_headers_are_preserved_across_model_reentry() { let fixture = fixture(); diff --git a/tests/integration/tests/suite/examples/openai_agentic_loop.rs b/tests/integration/tests/suite/examples/openai_agentic_loop.rs index 9c4dc0c05c..041e578466 100644 --- a/tests/integration/tests/suite/examples/openai_agentic_loop.rs +++ b/tests/integration/tests/suite/examples/openai_agentic_loop.rs @@ -455,6 +455,19 @@ fn web_search_round_trip_executes_and_re_enters_inference() { "final response should be the second model response after web search" ); + // A successful search updates the model's placeholder in place, so the public + // response carries exactly one completed web_search_call for ws_1. + let output = response["output"].as_array().expect("final response output array"); + let search_calls: Vec<&serde_json::Value> = + output.iter().filter(|item| item["type"] == "web_search_call").collect(); + assert_eq!( + search_calls.len(), + 1, + "final response must contain exactly one web_search_call, got: {output:#?}" + ); + assert_eq!(search_calls[0]["id"], "ws_1"); + assert_eq!(search_calls[0]["status"], "completed"); + let model_reqs = model.requests(); assert_eq!( model_reqs.len(), @@ -474,6 +487,102 @@ fn web_search_round_trip_executes_and_re_enters_inference() { ); } +#[test] +fn web_search_provider_failure_continues_loop_with_failed_result() { + let first_response = serde_json::json!({ + "id": "resp_ws_1", + "object": "response", + "status": "completed", + "output": [{ + "type": "web_search_call", + "id": "ws_1", + "status": "completed", + "action": {"type": "search", "query": "Rust 2025 edition"} + }] + }); + let second_response = serde_json::json!({ + "id": "resp_ws_2", + "object": "response", + "status": "completed", + "output": [{ + "type": "message", + "role": "assistant", + "content": [{"type": "output_text", "text": "I could not search, but here is what I know."}] + }] + }); + + let model = StatefulCapturingBackend::new(vec![ + (200, serde_json::to_string(&first_response).unwrap()), + (200, serde_json::to_string(&second_response).unwrap()), + ]) + .start_with_shutdown(); + + let search_listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let search_port = search_listener.local_addr().unwrap().port(); + spawn_failing_search_mock(search_listener); + + let proxy_port = free_port(); + let config = load_web_search_config(proxy_port, model.port(), search_port); + let proxy = start_proxy(&config); + + let request_body = serde_json::json!({ + "model": "gpt-4.1", + "input": "Search for Rust 2025 edition features", + "tools": [{"type": "web_search_preview"}] + }); + let raw = http_send( + proxy.addr(), + &json_post("/v1/responses", &serde_json::to_string(&request_body).unwrap()), + ); + + assert_eq!( + parse_status(&raw), + 200, + "a provider failure must not reject the Response" + ); + let response: serde_json::Value = serde_json::from_str(&parse_body(&raw)).expect("response should be JSON"); + assert_eq!( + response["id"], "resp_ws_2", + "the loop must continue to a second inference after the search fails" + ); + + // The public response must carry exactly one web_search_call for ws_1, marked + // failed — not a contradictory completed placeholder plus a failed duplicate. + let output = response["output"].as_array().expect("final response output array"); + let search_calls: Vec<&serde_json::Value> = + output.iter().filter(|item| item["type"] == "web_search_call").collect(); + assert_eq!( + search_calls.len(), + 1, + "final response must contain exactly one web_search_call, got: {output:#?}" + ); + assert_eq!(search_calls[0]["id"], "ws_1"); + assert_eq!( + search_calls[0]["status"], "failed", + "the single web_search_call must reflect the failed outcome" + ); + + let model_reqs = model.requests(); + assert_eq!( + model_reqs.len(), + 2, + "model backend should receive two requests (initial + post-failure)" + ); + + let second_body: serde_json::Value = + serde_json::from_str(&model_reqs[1].body).expect("second model request should be valid JSON"); + let input = second_body["input"] + .as_array() + .expect("second model request input should be an array"); + let has_failure_notice = input.iter().any(|item| { + item["type"] == "web_search_call" && item["status"] == "failed" && item["output"] == "Web search unavailable." + }); + assert!( + has_failure_notice, + "the model must receive a truthful failed web_search_call: {input:#?}" + ); +} + fn spawn_search_mock(listener: std::net::TcpListener) { use std::io::{Read as _, Write as _}; let body = serde_json::json!({ @@ -498,6 +607,22 @@ fn spawn_search_mock(listener: std::net::TcpListener) { }); } +/// Serve a single 5xx so the search client maps the callout to a failed outcome. +fn spawn_failing_search_mock(listener: std::net::TcpListener) { + use std::io::{Read as _, Write as _}; + let body = r#"{"error":"service unavailable"}"#; + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut buf = [0_u8; 4096]; + let _n = stream.read(&mut buf).unwrap(); + let response = format!( + "HTTP/1.1 503 Service Unavailable\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + stream.write_all(response.as_bytes()).unwrap(); + }); +} + fn load_web_search_config(proxy_port: u16, model_port: u16, search_port: u16) -> praxis_core::config::Config { let path = example_config_path("openai/responses/agentic-loop.yaml"); let yaml = std::fs::read_to_string(path).expect("read agentic-loop example"); diff --git a/xtask/src/filter_docs.rs b/xtask/src/filter_docs.rs index c25a79afb5..bd72d20563 100644 --- a/xtask/src/filter_docs.rs +++ b/xtask/src/filter_docs.rs @@ -2674,7 +2674,7 @@ mod tests { RequiredKind::Yes, "{filter_name} should document api_key as required" ); - for expected in ["provider", "provider_failure_mode", "status_on_error", "base_url"] { + for expected in ["provider", "base_url"] { assert!( filter.filter.fields.iter().any(|field| field.name == expected), "{filter_name} should document {expected}"