Skip to content

Improve Prebid auction diagnostics#671

Open
ChristianPavilonis wants to merge 3 commits intomainfrom
feature/prebid-auction-diagnostics
Open

Improve Prebid auction diagnostics#671
ChristianPavilonis wants to merge 3 commits intomainfrom
feature/prebid-auction-diagnostics

Conversation

@ChristianPavilonis
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis commented Apr 29, 2026

Summary

  • Improve Prebid auction diagnostics so upstream HTTP failures surface status in /auction provider metadata, with capped body previews included only when Prebid debug mode is enabled.
  • Treat successful empty Prebid responses (204 No Content or empty 200) as nobid instead of provider error.
  • Remove Rubicon from the default client_side_bidders sample config so it is not enabled by default.

Changes

File Change
crates/trusted-server-core/src/integrations/prebid.rs Adds upstream HTTP status metadata for non-2xx responses, gates capped Prebid response body previews behind debug, treats empty successful responses as no-bid, uses PREBID_INTEGRATION_ID for provider response IDs, and adds tests for truncation/non-UTF-8 preview behavior.
crates/trusted-server-core/src/auction/orchestrator.rs Preserves provider launch/parse/conversion errors as provider response metadata using client-safe current-context display text, and adds tests for safe output and truncation.
trusted-server.toml Changes default client_side_bidders from ["rubicon"] to [].

Compatibility notes

  • provider_details may include launch-failure diagnostics before awaited provider responses. Bid selection is order-insensitive, but consumers should not rely on provider_details being ordered by response completion.

Closes

N/A — no issue filed.

Test plan

  • cargo test --workspace
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • JS tests: cd crates/js/lib && npx vitest run
  • JS format: cd crates/js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: cargo test -p trusted-server-core prebid -- --nocapture; cargo test -p trusted-server-core auction -- --nocapture; targeted review-feedback tests for provider_error, prebid_body_preview, and parse_response_non_success

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses log macros per CLAUDE.md (template says tracing, but this repo standard is log)
  • New code has tests
  • No secrets or credentials committed

Comment thread crates/trusted-server-core/src/auction/orchestrator.rs
Comment thread crates/trusted-server-core/src/integrations/prebid.rs
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs
Comment thread crates/trusted-server-core/src/integrations/prebid.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/prebid.rs
@aram356
Copy link
Copy Markdown
Collaborator

aram356 commented May 1, 2026

Summary

Richer /auction diagnostics and correct no-bid classification for empty Prebid responses are well-motivated and well-tested. One blocker: error-stack Report Debug formatting bleeds internal source paths into the public /auction JSON response, which contradicts the explicit policy in error.rs separating client-facing messages from server-only details.

Blocking

🔧 wrench

  • Information disclosure via format!("{error:?}"): provider_error_message ships error-stack Debug output (including at <file>:<line> entries) into the public /auction provider_details[].metadata.message. See inline. Switch to error.current_context().to_string() to keep the user-safe Display text but drop file/line/stack.

Non-blocking

🤔 thinking

  • Upstream body_preview proxied to public clients: The first 1000 chars of upstream Prebid error bodies are returned in the /auction response. Consider gating on self.config.debug (mirrors the existing trace-log gate) or a request-time header. See inline.
  • provider_details ordering changed: Launch failures now precede awaited responses in responses, whereas previously the list was ordered by completion. select_winning_bids is unaffected, but external consumers parsing provider_details see a different order. See inline.

♻️ refactor

  • Use PREBID_INTEGRATION_ID constant: New AuctionResponse::error("prebid", …) / ::no_bid("prebid", …) call sites should use the existing constant (crates/trusted-server-core/src/integrations/prebid.rs:36) instead of the literal. See inline.

🌱 seedling

  • Standardize diagnostic metadata across providers: Orchestrator-level error_type tagging is provider-agnostic, but only Prebid produces http_status / body_preview for non-success upstream responses. APS/GAM/etc. would be silent on the same failure mode. Worth a follow-up.

📌 out of scope

  • client_side_bidders = [] is a silent client-side capability change: The JS bundle still ships the rubicon adapter at build time (crates/js/lib/build-all.mjs:32DEFAULT_PREBID_ADAPTERS = 'rubicon'), but the runtime default in trusted-server.toml no longer enables it. Publishers copying the example config will lose rubicon client-side without an adapter-time signal. Worth a CHANGELOG / release note.

⛏ nitpick

  • Missing truncation tests: Neither prebid_body_preview (1000-char cap, String::from_utf8_lossy) nor provider_error_message (500-char cap) has a test pinning truncation/non-UTF-8 behavior. See inline.
  • format!("{error:?}") allocates before truncation: Long error chains materialize a multi-KB string before chars().take(500) discards the tail. Becomes a non-issue if the wrench above is fixed (Display output is much shorter).

CI Status

  • fmt: PASS
  • clippy: PASS
  • rust tests: PASS (incl. 4 new prebid + 1 orchestrator)
  • js tests: PASS
  • format-typescript / format-docs / browser integration tests / CodeQL: PASS

@ChristianPavilonis ChristianPavilonis requested a review from aram356 May 4, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants