Repository navigation
Fall back locally after mediator HTTP errors - #1493
Conversation
Match local winning bids by instance ID and price so an earlier bid from the same bidder cannot claim the win. Preserve mediated association when mediation rewrites IDs or supplies decoded prices. Cover missing and reused IDs, provider collisions, ties, and HTTP-error fallback telemetry. Use level-neutral wording for mediator failure logs.
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Non-2xx mediator responses now return an error, so they go through the existing local-ranking fallback (with floors) instead of discarding collected provider bids. Local winner telemetry is now matched on bid ID and price. The fix is correct and well tested; only non-blocking notes below.
1 inline comment carries a one-click
suggestion(compile-unverified — please runcargo fmt --checkafter applying).
Non-blocking
- 🌱 Failed mediation is only visible in logs — inline at
adserver_mock.rs:391 - 📝 Local-win matching assumes winners are verbatim clones — inline at
telemetry.rs:761 - ⛏ Error message wording — inline at
adserver_mock.rs:393 - 👍 Thorough fallback and telemetry tests — inline at
orchestrator.rs:7152
CI Status
All 29 checks: PASS.
State on select_winning_bids that winners are verbatim clones of provider bids, since telemetry attributes local wins by bid_id and price. Restore the "non-success" wording in the mediator HTTP error so existing log searches still match.
aram356
left a comment
There was a problem hiding this comment.
Summary
adserver_mock now returns an error for a non-2xx mediator response, so both production collect paths fall back to local ranking with floors instead of dropping the collected bids. Local-ranking telemetry now marks the bid that was actually selected. The fix is correct and well covered: reverting the provider change fails both new fallback tests, and reverting the telemetry clause fails three. One blocking item: the behavior and telemetry shifts need a CHANGELOG entry. The inline comments are non-blocking.
1 of the inline comments below carries a one-click GitHub
suggestion. Use Commit suggestion to apply it. The others describe the change in prose because it spans lines outside the diff or asks a question.
Blocking
🔧 wrench
- CHANGELOG entry missing: see Cross-cutting below
Non-blocking
- ♻️ Local-win telemetry keys off
mediator_response.is_some(): see inline atcrates/trusted-server-core/src/auction/telemetry.rs:760 - 🤔 A mediator HTTP 204 falls back instead of ending as a no-bid: see inline at
crates/trusted-server-core/src/integrations/adserver_mock.rs:391 - ⛏ Mediator failure wording in the guide: see inline at
docs/guide/auction-orchestration.md:919 - 🌱 The orchestrator still treats a
BidStatus::Errormediator response as final: see inline atcrates/trusted-server-core/src/auction/orchestrator.rs:2656
Cross-cutting / body-level findings
-
🔧 CHANGELOG entry missing. Both changes in this PR are operator-visible:
- During a mediator outage, deployments now serve locally ranked bids instead of none.
is_winchanges for local ranking on every deployment that emits auction telemetry, not only after a mediator fallback. When one bidder code returned several bids for a slot without distinguishing ad IDs, an earlier lower-priced bid (possibly from another provider) was marked as the win at its own price.auction_bid_stats_mvgroupswinsandwinning_cpm_quantilesby provider and seat, so those series shift after upgrading.
Other recent PRs (#1405, #1406, #1433) update the changelog, and
[Unreleased]has a### Fixedlist for bugs like this one. Proposed entries for the top of### Fixed(prettier-clean when inserted there):- A mediator that answers with a non-2xx HTTP status no longer discards the collected provider bids. The orchestrator now logs a warning and ranks those bids locally with floors, as it already did for mediator launch, transport, deadline and parse failures (#1203). - Auction telemetry now marks the bid that local ranking selected as the slot's winner. When one bidder code returned several bids for a slot without distinguishing ad IDs, an earlier lower-priced bid, possibly from another provider, was marked as the win at its own price. Tinybird per-provider win counts and winning-CPM quantiles for those slots shift after upgrading. Mediated wins are unchanged.
Apply manually:
CHANGELOG.mdis not in this PR's diff, so it can't be a suggestion.
CI Status
- Analyze (actions): PASS
- Analyze (javascript-typescript): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
- Analyze (rust): PASS
- browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- CodeQL: PASS
- docs-proposal scripts: PASS
- format-docs: PASS (required)
- format-typescript: PASS (required)
- HTML EOF runtime (Fastly): PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- trace runtime and browser acceptance: PASS
- ts dev lint domains (changed lines): PASS
- vitest: PASS
- worker-deadlines (cloudflare): PASS
- worker-deadlines (spin): PASS
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
The change restores local ranking after mediator HTTP errors, preserves floor enforcement, and corrects local winner telemetry without changing successful mediation behavior. No verified code findings.
Reviewed fe9a220e6 and checked the update to 734bf8962. The update is a clean merge of main with no conflict-resolution edits. All five reviewed files are unchanged, and the PR patch against its updated base is byte-for-byte identical to the previously reviewed patch.
CI status
28 reported checks pass. worker-deadlines (cloudflare) failed during tooling setup because the GitHub release download for wasm-bindgen returned HTTP 504. The fixture build and Clippy passed before that failure; its runtime test did not execute. This is an external download failure, not evidence of a regression in this patch. That job still needs a successful rerun.
- browser integration tests: PASS
- integration tests: PASS
- trace runtime and browser acceptance: PASS
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- prepare integration artifacts: PASS
- cargo fmt: PASS (required)
- format-typescript: PASS (required)
- worker-deadlines (spin): PASS
- Analyze (rust): PASS
- vitest: PASS
- ts dev lint domains (changed lines): PASS
- HTML EOF runtime (Fastly): PASS
- cargo test: PASS (required)
- worker-deadlines (cloudflare): FAIL
- cargo test (ts CLI, native) (macos-latest): PASS
- CLAUDE.md symlink guard: PASS
- Analyze (javascript-typescript): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (cross-adapter parity): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- docs-proposal scripts: PASS
- format-docs: PASS (required)
- Analyze (actions): PASS
- cargo test (axum native): PASS
- CI Node pin guard: PASS
- Analyze (python): PASS
- Analyze (javascript-typescript): PASS
Local git diff --check passed. Passing hosted checks were reused rather than rerun locally; deployed runtime behavior was not independently exercised.
Attribute local telemetry wins to the exact winning provider bid instead of inferring local ranking from an absent mediator response, so keeping a failed mediator response cannot reattach the win to an earlier, lower bid. Treat a mediator HTTP 204 as a final no-bid, matching how bidder providers handle 204, and fall back to local ranking when a mediator reports an error-status response on both the production and harness paths. Clarify the mediator failure wording in the auction guide and add CHANGELOG entries for the fallback and telemetry changes.
|
@aram356's blocking CHANGELOG item is addressed in 7ff0d45. I added two entries at the top of
Prettier with Local gates on this commit: |
Both the production orchestrator and the planned harness now log a warning before ranking locally after a mediator reports BidStatus::Error, matching the other mediator failure paths. Add a regression for each site and pin exact attribution when a mediator keeps a provider bid's identity.
Summary
adserver_mockreturns a non-2xx HTTP status. The shared parser now returns an error, allowing the existing warning and local-ranking fallback to run with floor enforcement.Changes
crates/trusted-server-core/src/integrations/adserver_mock.rscrates/trusted-server-core/src/auction/orchestrator.rscrates/trusted-server-core/src/auction/telemetry.rsdocs/guide/auction-orchestration.mddocs/guide/integrations/adserver_mock.mdCloses
Closes #1203
Spec
Not a feature. This bug fix restores the documented mediation-failure fallback contract.
Test plan
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1(not run)fastly compute serve(not run)POST /auction, with controlled HTTPS bidder and mediator endpoints.The initial HTTP-status fix was verified through an actual Axum server: the original server lost a collected $2.00 provider bid after mediator 503; the fix preserved it while retaining successful mediation and valid no-bid decisions.
The final telemetry correction was verified through the actual Fastly application under Viceroy using normal strict config validation/local push, controlled HTTPS bidder/mediator/ingestion endpoints, and captured Tinybird NDJSON. Before this correction, the server selected a $2.00 bid but marked an earlier $1.50 bid as winning. The corrected artifact emitted
$1.50: is_win=0and$2.00: is_win=1for repeated 503s, malformed-response fallback, and no-mediator execution. Valid no-bid and existing mediated association behavior remained intact. This correction targets local winner matching; successful mediation's documented duplicate-identity ambiguity remains unchanged.Local TLS mappings used task-owned CA certificates through the maintained Viceroy per-backend configuration pattern; TLS verification remained enabled. All owned runtime processes stopped and TLS private keys were removed. Floors were verified in core tests because
/auctionhas no floor input.Local checks used Rust 1.98.1, Node 24.20.0, Viceroy 0.21.0 and Spin 4.1.0. Checks requiring macOS certificate or loopback access passed outside the restricted sandbox. Hosted CI and deployed runtime behavior are separate from these local results.
Checklist
unwrap()in production codelogmacros