diff --git a/crates/trusted-server-core/src/auction/orchestrator.rs b/crates/trusted-server-core/src/auction/orchestrator.rs index 9cbcd2b9..faaab07b 100644 --- a/crates/trusted-server-core/src/auction/orchestrator.rs +++ b/crates/trusted-server-core/src/auction/orchestrator.rs @@ -13,6 +13,34 @@ use super::config::AuctionConfig; use super::provider::AuctionProvider; use super::types::{AuctionContext, AuctionRequest, AuctionResponse, Bid, BidStatus}; +const PROVIDER_ERROR_MESSAGE_CHARS: usize = 500; + +fn provider_error_message(error: &Report) -> String { + error + .current_context() + .to_string() + .chars() + .take(PROVIDER_ERROR_MESSAGE_CHARS) + .collect() +} + +fn provider_error_response( + provider_name: &str, + response_time_ms: u64, + error_type: &str, + error: &Report, +) -> AuctionResponse { + AuctionResponse::error(provider_name, response_time_ms) + .with_metadata("error_type", serde_json::json!(error_type)) + .with_metadata("message", serde_json::json!(provider_error_message(error))) +} + +fn provider_launch_failed_response(provider_name: &str, response_time_ms: u64) -> AuctionResponse { + AuctionResponse::error(provider_name, response_time_ms) + .with_metadata("error_type", serde_json::json!("launch_failed")) + .with_metadata("message", serde_json::json!("Provider launch failed")) +} + /// Compute the remaining time budget from a deadline. /// /// Returns the number of milliseconds left before `timeout_ms` is exceeded, @@ -274,6 +302,7 @@ impl AuctionOrchestrator { let mut backend_to_provider: HashMap = HashMap::new(); let mut pending_requests: Vec = Vec::new(); + let mut responses = Vec::new(); for provider_name in provider_names { let provider = match self.providers.get(provider_name) { @@ -353,11 +382,16 @@ impl AuctionOrchestrator { ); } Err(e) => { + let response_time_ms = start_time.elapsed().as_millis() as u64; log::warn!( "Provider '{}' failed to launch request: {:?}", provider.provider_name(), e ); + responses.push(provider_launch_failed_response( + provider.provider_name(), + response_time_ms, + )); } } } @@ -377,7 +411,6 @@ impl AuctionOrchestrator { // transport timeout fires). Hard deadline enforcement therefore depends // on every backend's `first_byte_timeout` being set to at most the // remaining auction budget — which Phase 1 above guarantees. - let mut responses = Vec::new(); let mut remaining = pending_requests; while !remaining.is_empty() { @@ -419,9 +452,11 @@ impl AuctionOrchestrator { provider_name, e ); - responses.push(AuctionResponse::error( + responses.push(provider_error_response( provider_name, response_time_ms, + "parse_response", + &e, )); } } @@ -432,8 +467,12 @@ impl AuctionOrchestrator { provider_name, e ); - responses - .push(AuctionResponse::error(provider_name, response_time_ms)); + responses.push(provider_error_response( + provider_name, + response_time_ms, + "unsupported_response_body", + &e, + )); } } } else { @@ -635,8 +674,9 @@ mod tests { use crate::auction::config::AuctionConfig; use crate::auction::test_support::create_test_auction_context; use crate::auction::types::{ - AdFormat, AdSlot, AuctionRequest, Bid, MediaType, PublisherInfo, UserInfo, + AdFormat, AdSlot, AuctionRequest, Bid, BidStatus, MediaType, PublisherInfo, UserInfo, }; + use crate::error::TrustedServerError; // All-None ClientInfo used across tests that don't need real IP/TLS data. // Defined as a const so &EMPTY_CLIENT_INFO has 'static lifetime, avoiding @@ -648,6 +688,7 @@ mod tests { }; use crate::platform::test_support::noop_services; use crate::test_support::tests::crate_test_settings_str; + use error_stack::Report; use fastly::Request; use std::collections::{HashMap, HashSet}; @@ -700,6 +741,80 @@ mod tests { crate::settings::Settings::from_toml(&settings_str).expect("should parse test settings") } + #[test] + fn provider_error_response_includes_diagnostic_metadata() { + let error = Report::new(TrustedServerError::Auction { + message: "parse failed".to_string(), + }) + .attach("internal/source.rs:12:34"); + + let response = super::provider_error_response("prebid", 37, "parse_response", &error); + + assert_eq!( + response.status, + BidStatus::Error, + "should mark diagnostic provider responses as errors" + ); + assert_eq!( + response.metadata["error_type"], + serde_json::json!("parse_response"), + "should include the provider error classification" + ); + + let message = response.metadata["message"] + .as_str() + .expect("should include provider error message"); + assert!( + message.contains("parse failed"), + "should include user-safe diagnostic detail" + ); + assert!( + !message.contains("internal/source.rs"), + "should not include attached internal details" + ); + } + + #[test] + fn launch_failed_response_has_safe_static_message() { + let response = super::provider_launch_failed_response("prebid", 58); + + assert_eq!( + response.status, + BidStatus::Error, + "should mark launch failures as errors" + ); + assert_eq!( + response.metadata["error_type"], + serde_json::json!("launch_failed"), + "should include launch_failed classification" + ); + assert_eq!( + response.metadata["message"], + serde_json::json!("Provider launch failed"), + "should use a safe, stable public launch failure message" + ); + } + + #[test] + fn provider_error_message_truncates_user_safe_context() { + let long_message = "x".repeat(super::PROVIDER_ERROR_MESSAGE_CHARS + 100); + let error = Report::new(TrustedServerError::Auction { + message: long_message, + }); + + let message = super::provider_error_message(&error); + + assert_eq!( + message.chars().count(), + super::PROVIDER_ERROR_MESSAGE_CHARS, + "should cap provider error messages" + ); + assert!( + message.starts_with("Auction error: "), + "should preserve the current context display text" + ); + } + #[test] fn filters_winning_bids_below_floor() { let orchestrator = AuctionOrchestrator::new(AuctionConfig::default()); diff --git a/crates/trusted-server-core/src/integrations/prebid.rs b/crates/trusted-server-core/src/integrations/prebid.rs index 11d153eb..011c43dc 100644 --- a/crates/trusted-server-core/src/integrations/prebid.rs +++ b/crates/trusted-server-core/src/integrations/prebid.rs @@ -41,6 +41,23 @@ const ZONE_KEY: &str = "zone"; /// Default currency for `OpenRTB` bid floors and responses. const DEFAULT_CURRENCY: &str = "USD"; +/// Maximum number of characters from upstream failure payloads included in +/// debug-facing `body_preview` metadata. +const PREBID_ERROR_BODY_PREVIEW_CHARS: usize = 1000; + +/// Maximum number of bytes processed when constructing debug-facing upstream +/// failure previews. +const PREBID_ERROR_BODY_PREVIEW_BYTES: usize = PREBID_ERROR_BODY_PREVIEW_CHARS * 4; + +fn prebid_body_preview(body: &[u8]) -> String { + let bounded_body = &body[..body.len().min(PREBID_ERROR_BODY_PREVIEW_BYTES)]; + + String::from_utf8_lossy(bounded_body) + .chars() + .take(PREBID_ERROR_BODY_PREVIEW_CHARS) + .collect() +} + /// CCPA/US-privacy string sent when the `Sec-GPC` header signals opt-out. /// /// Encodes: version `1`, notice given (`Y`), user opted out (`Y`), LSPA not @@ -1253,9 +1270,9 @@ impl PrebidAuctionProvider { } if bids.is_empty() { - AuctionResponse::no_bid("prebid", response_time_ms) + AuctionResponse::no_bid(PREBID_INTEGRATION_ID, response_time_ms) } else { - AuctionResponse::success("prebid", bids, response_time_ms) + AuctionResponse::success(PREBID_INTEGRATION_ID, bids, response_time_ms) } } @@ -1373,7 +1390,7 @@ impl PrebidAuctionProvider { impl AuctionProvider for PrebidAuctionProvider { fn provider_name(&self) -> &'static str { - "prebid" + PREBID_INTEGRATION_ID } fn request_bids( @@ -1478,27 +1495,55 @@ impl AuctionProvider for PrebidAuctionProvider { response_time_ms: u64, ) -> Result> { // Parse response + let status = response.get_status(); let body_bytes = response.take_body_bytes(); - if !response.get_status().is_success() { + if !status.is_success() { log::warn!( - "Prebid returned non-success status: {}", - response.get_status(), + "Prebid returned non-success status: {status}; {} bytes", + body_bytes.len() ); - if log::log_enabled!(log::Level::Trace) { - let body_preview = String::from_utf8_lossy(&body_bytes); - log::trace!( - "Prebid error response body: {}", - &body_preview[..body_preview.floor_char_boundary(1000)] - ); + + let mut auction_response = + AuctionResponse::error(PREBID_INTEGRATION_ID, response_time_ms) + .with_metadata("error_type", serde_json::json!("http_status")) + .with_metadata("http_status", serde_json::json!(status.as_u16())); + + if self.config.debug { + let body_preview = prebid_body_preview(&body_bytes); + if !body_preview.is_empty() { + log::debug!("Prebid non-success response body: {body_preview}"); + auction_response = auction_response + .with_metadata("body_preview", serde_json::json!(body_preview)); + } } - return Ok(AuctionResponse::error("prebid", response_time_ms)); + + return Ok(auction_response); } - let response_json: Json = - serde_json::from_slice(&body_bytes).change_context(TrustedServerError::Prebid { - message: "Failed to parse Prebid response".to_string(), - })?; + if body_bytes.is_empty() { + log::info!( + "Prebid returned successful empty response with status {status}; treating as no-bid" + ); + return Ok( + AuctionResponse::no_bid(PREBID_INTEGRATION_ID, response_time_ms) + .with_metadata("reason", serde_json::json!("empty_response")) + .with_metadata("http_status", serde_json::json!(status.as_u16())), + ); + } + + let response_json: Json = match serde_json::from_slice(&body_bytes) { + Ok(response_json) => response_json, + Err(error) => { + log::warn!( + "Prebid: failed to parse response JSON (status {status}, {} bytes): {error}", + body_bytes.len() + ); + return Err(Report::new(TrustedServerError::Prebid { + message: "Failed to parse Prebid response JSON".to_string(), + })); + } + }; // Log the full response body when debug is enabled to surface // ext.debug.httpcalls, resolvedrequest, bidstatus, errors, etc. @@ -3115,28 +3160,44 @@ server_url = "https://prebid.example" assert_eq!(routes.len(), 0); } - /// Verifies body-preview truncation keeps a UTF-8 char boundary. #[test] - fn body_preview_truncation_is_utf8_safe() { - // 999 ASCII bytes + U+2603 SNOWMAN (3 bytes: E2 98 83) = 1002 bytes. - // Byte index 1000 lands on 0x98, the second byte of the snowman. - let mut body = "x".repeat(999); - body.push('\u{2603}'); // ☃ - assert_eq!(body.len(), 1002); - - let truncation_index = body.floor_char_boundary(1000); - assert!( - body.is_char_boundary(truncation_index), - "should truncate at a valid UTF-8 boundary" + fn prebid_body_preview_truncates_to_character_limit() { + let body = "x".repeat(PREBID_ERROR_BODY_PREVIEW_CHARS + 100); + + let preview = prebid_body_preview(body.as_bytes()); + + assert_eq!( + preview.chars().count(), + PREBID_ERROR_BODY_PREVIEW_CHARS, + "should cap the upstream body preview" ); + } + + #[test] + fn prebid_body_preview_handles_non_utf8_lossily() { + let preview = prebid_body_preview(&[b'o', b'k', 0xff, b'!']); + assert_eq!( - body[..truncation_index].len(), - 999, - "should drop the partial multibyte character" + preview, "ok\u{fffd}!", + "should replace invalid UTF-8 bytes without panicking" ); + } + + #[test] + fn prebid_body_preview_ignores_bytes_after_bounded_slice() { + let mut body = vec![b'x'; PREBID_ERROR_BODY_PREVIEW_BYTES]; + body.extend_from_slice(&[0xff, b't', b'a', b'i', b'l']); + + let preview = prebid_body_preview(&body); + assert_eq!( - truncation_index, 999, - "should step back to the previous char boundary" + preview.chars().count(), + PREBID_ERROR_BODY_PREVIEW_CHARS, + "should keep the public preview capped" + ); + assert!( + !preview.contains('\u{fffd}') && !preview.contains("tail"), + "should not process bytes beyond the bounded preview slice" ); } @@ -3219,6 +3280,154 @@ server_url = "https://prebid.example" serde_json::from_value(ext["prebid"].clone()).expect("should deserialise ext.prebid") } + #[test] + fn parse_response_non_success_returns_error_with_http_metadata() { + let provider = PrebidAuctionProvider::new(base_config()); + let response = Response::from_status(StatusCode::BAD_REQUEST).with_body("invalid request"); + + let auction_response = provider + .parse_response(response, 58) + .expect("should convert non-success status to provider error"); + + assert_eq!( + auction_response.status, + crate::auction::types::BidStatus::Error, + "should mark non-success upstream responses as errors" + ); + assert_eq!( + auction_response.metadata["error_type"], + json!("http_status"), + "should classify the error source" + ); + assert_eq!( + auction_response.metadata["http_status"], + json!(400), + "should include upstream HTTP status" + ); + assert!( + !auction_response.metadata.contains_key("body_preview"), + "should omit upstream body preview unless Prebid debug is enabled" + ); + } + + #[test] + fn parse_response_non_success_includes_body_preview_when_debug_enabled() { + let mut config = base_config(); + config.debug = true; + let provider = PrebidAuctionProvider::new(config); + let body = "x".repeat(PREBID_ERROR_BODY_PREVIEW_CHARS + 100); + let response = Response::from_status(StatusCode::BAD_REQUEST).with_body(body); + + let auction_response = provider + .parse_response(response, 58) + .expect("should convert non-success status to provider error"); + + let body_preview = auction_response.metadata["body_preview"] + .as_str() + .expect("should include upstream body preview in debug mode"); + assert_eq!( + body_preview.chars().count(), + PREBID_ERROR_BODY_PREVIEW_CHARS, + "should cap debug upstream body preview" + ); + } + + #[test] + fn parse_response_invalid_json_returns_safe_client_error() { + let provider = PrebidAuctionProvider::new(base_config()); + let response = Response::from_status(StatusCode::OK).with_body(r#"{"seatbid":["bid""#); + + let error = provider + .parse_response(response, 42) + .expect_err("should return parse failure for invalid JSON"); + + let message = format!("{error}"); + assert!( + message.contains("Failed to parse Prebid response JSON"), + "should include stable user-safe parse failure message" + ); + assert!( + !message.contains("expected value"), + "should not leak serde parse details" + ); + assert!( + !message.contains("bytes"), + "should not leak response length in the user-safe message" + ); + } + + #[test] + fn parse_response_no_content_returns_no_bid_with_reason() { + let provider = PrebidAuctionProvider::new(base_config()); + let response = Response::from_status(StatusCode::NO_CONTENT); + + let auction_response = provider + .parse_response(response, 42) + .expect("should convert no-content status to no-bid"); + + assert_eq!( + auction_response.status, + crate::auction::types::BidStatus::NoBid, + "should treat 204 as a no-bid response" + ); + assert_eq!( + auction_response.metadata["reason"], + json!("empty_response"), + "should explain why the provider returned no bids" + ); + assert_eq!( + auction_response.metadata["http_status"], + json!(204), + "should include upstream HTTP status" + ); + } + + #[test] + fn parse_response_ok_empty_body_returns_no_bid_with_reason() { + let provider = PrebidAuctionProvider::new(base_config()); + let response = Response::from_status(StatusCode::OK); + + let auction_response = provider + .parse_response(response, 17) + .expect("should convert empty successful response to no-bid"); + + assert_eq!( + auction_response.status, + crate::auction::types::BidStatus::NoBid, + "should treat empty 200 as a no-bid response" + ); + assert_eq!( + auction_response.metadata["reason"], + json!("empty_response"), + "should explain why the provider returned no bids" + ); + assert_eq!( + auction_response.metadata["http_status"], + json!(200), + "should include upstream HTTP status" + ); + } + + #[test] + fn parse_response_valid_json_without_bids_returns_no_bid() { + let provider = PrebidAuctionProvider::new(base_config()); + let response = Response::from_status(StatusCode::OK).with_body(r#"{"seatbid":[]}"#); + + let auction_response = provider + .parse_response(response, 23) + .expect("should parse valid no-bid JSON"); + + assert_eq!( + auction_response.status, + crate::auction::types::BidStatus::NoBid, + "should preserve valid JSON no-bid behavior" + ); + assert!( + auction_response.metadata.is_empty(), + "should not add empty-response metadata for valid no-bid JSON" + ); + } + // ======================================================================== // bid_param_overrides tests // ======================================================================== diff --git a/trusted-server.toml b/trusted-server.toml index f57e9146..780d211c 100644 --- a/trusted-server.toml +++ b/trusted-server.toml @@ -45,7 +45,7 @@ debug = false # Bidders that run client-side via native Prebid.js adapters instead of # being routed through the server-side auction. Their adapter modules must # be statically imported in the JS bundle. -client_side_bidders = ["rubicon"] +client_side_bidders = [] # Compatibility sugar for static per-bidder params merged into every outgoing # PBS request. These normalize into bid_param_override_rules internally.