Skip to content

Migrate integration and provider HTTP types (PR 13)#626

Open
prk-Jr wants to merge 6 commits intofeature/edgezero-pr12-handler-layer-typesfrom
feature/edgezero-pr13-integration-provider-type-migration
Open

Migrate integration and provider HTTP types (PR 13)#626
prk-Jr wants to merge 6 commits intofeature/edgezero-pr12-handler-layer-typesfrom
feature/edgezero-pr13-integration-provider-type-migration

Conversation

@prk-Jr
Copy link
Copy Markdown
Collaborator

@prk-Jr prk-Jr commented Apr 9, 2026

Summary

  • Replace the remaining Fastly request and response boundaries in the integration and auction provider layers so PR13 runs on platform-agnostic HTTP types.
  • Move provider dispatch onto RuntimeServices HTTP client primitives with async request_bids, PlatformPendingRequest, and PlatformResponse while preserving orchestrator deadline handling.
  • Finish the review hardening for Testlight response normalization and convert the migrated integration tests to HTTP-native fixtures.

Changes

File Change
crates/trusted-server-adapter-fastly/src/main.rs Route integration proxy requests through the HTTP-native registry boundary and pass RuntimeServices into the PR13 entrypoint.
crates/trusted-server-core/src/auction/README.md Update provider guidance and examples to PlatformPendingRequest, PlatformResponse, and the platform HTTP client flow.
crates/trusted-server-core/src/auction/endpoints.rs Thread RuntimeServices into the auction entrypoint context.
crates/trusted-server-core/src/auction/orchestrator.rs Await async provider launches, operate on PlatformPendingRequest/select, and update the remaining PR13 migration comments.
crates/trusted-server-core/src/auction/provider.rs Convert AuctionProvider to async request_bids with PlatformPendingRequest/PlatformResponse and refresh the trait docs.
crates/trusted-server-core/src/auction/types.rs Add RuntimeServices to AuctionContext.
crates/trusted-server-core/src/integrations/adserver_mock.rs Migrate the mock provider to platform HTTP pending/response types.
crates/trusted-server-core/src/integrations/aps.rs Migrate APS provider request dispatch and response parsing to platform HTTP types.
crates/trusted-server-core/src/integrations/datadome.rs Convert DataDome proxying to HTTP-native request/response handling via the platform client.
crates/trusted-server-core/src/integrations/didomi.rs Convert Didomi proxying to HTTP-native request/response handling via the platform client.
crates/trusted-server-core/src/integrations/google_tag_manager.rs Remove Fastly shims, keep bounded POST handling on HTTP bodies, and convert GTM tests to HTTP-native fixtures.
crates/trusted-server-core/src/integrations/gpt.rs Remove Fastly request/response compat from GPT asset proxying and convert GPT tests to HTTP-native fixtures.
crates/trusted-server-core/src/integrations/lockr.rs Convert Lockr SDK/API proxy handling to HTTP-native request/response types.
crates/trusted-server-core/src/integrations/permutive.rs Convert Permutive SDK/API proxy handling to HTTP-native request/response types.
crates/trusted-server-core/src/integrations/prebid.rs Move Prebid provider dispatch to async platform HTTP requests and parse PlatformResponse results.
crates/trusted-server-core/src/integrations/registry.rs Migrate IntegrationProxy/routing types to http types and thread RuntimeServices through proxy dispatch.
crates/trusted-server-core/src/integrations/testlight.rs Convert Testlight to HTTP-native request/response handling, add the stale Content-Length regression test, and drop stale length headers when rebuilding JSON responses.

Closes

Closes #494

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 doc --no-deps --all-features (passes with pre-existing unrelated rustdoc warnings outside this PR's scope)

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses repo-standard logging macros, not println!
  • New code has tests
  • No secrets or credentials committed

@prk-Jr prk-Jr self-assigned this Apr 9, 2026
@prk-Jr prk-Jr changed the title Migrate integration and provider HTTP types Migrate integration and provider HTTP types (PR 13) Apr 9, 2026
@prk-Jr prk-Jr requested review from ChristianPavilonis and aram356 and removed request for aram356 April 9, 2026 12:04
@prk-Jr prk-Jr linked an issue Apr 9, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review — Migrate integration and provider HTTP types (PR 13)

1 blocking finding, 5 non-blocking suggestions.

Blocking

🔧 Missing Content-Type forwarding for Didomi POST/PUT — see inline comment.

Non-blocking (inline)

  • 🤔 Prebid still imports from fastly::http — last remaining use fastly in the integration layer
  • ♻️ Duplicated body collection helpers (collect_response_body / collect_body_bytes)
  • 🌱 Unbounded response body collection (matches pre-migration, but could use a size cap)
  • ⛏ Dead "X-" uppercase branch in custom header copy (http crate lowercases names)
  • ⛏ Duplicated backend_name_for_url helper across 4 integrations

Comment thread crates/trusted-server-core/src/integrations/didomi.rs
Comment thread crates/trusted-server-core/src/integrations/prebid.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/google_tag_manager.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/lockr.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome.rs
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore this

@ChristianPavilonis ChristianPavilonis dismissed their stale review April 14, 2026 19:19

Replacing with full review

Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Solid migration of the integration and auction provider layers from Fastly-specific HTTP types to platform-agnostic http crate types. No blocking issues found. CI is passing.

Non-blocking

♻️ refactor

  • Duplicated body_as_reader helper: proxy.rs:24 and publisher.rs:23 define identical body_as_reader functions. Consider extracting into a shared location (e.g., an into_reader() method on EdgeBody, or a helper in http_util.rs).

📝 note

  • PR description file table incomplete: proxy.rs and publisher.rs have changes in this diff (the body_as_reader extraction and test type alias cleanups) but are not listed in the PR body's change table.

📌 out of scope

  • Orchestrator tests disabled pending mock PlatformHttpClient: Several provider integration tests and timeout enforcement paths in orchestrator.rs:751-765 are disabled pending a mock for PlatformHttpClient::select(). This is acknowledged in the PR, but the scope of untested paths has grown — a follow-up to add thin mock support for select() would significantly improve coverage of the deadline enforcement logic.

Comment thread crates/trusted-server-core/src/integrations/mod.rs
Comment thread crates/trusted-server-core/src/integrations/google_tag_manager.rs Outdated
Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

The refactor is clean and the ensure_integration_backend / collect_body helpers extracted in the fix commits are a genuine improvement. The main risks are latent panics and missing POST size bounds that were introduced in the base migration commit. None are new in the fix commits, but this PR is the natural owner of the HTTP-type boundary, so they should land before merge.

Blocking

🔧 wrench

  • EdgeBody::into_bytes() panics on streaming bodies at auction/endpoints.rs:42 — the inbound auction POST body is user-controlled. Body::into_bytes() panics on Body::Stream(_) (unit test into_bytes_panics_for_stream asserts this). Today Fastly's adapter happens to return Body::Once, so nothing panics — but that invariant is undocumented and the /auction handler is the first publicly-reachable crash if it ever changes. Fix: switch to Body::into_bytes_bounded(max).await with a documented cap (e.g. 256KB).
  • EdgeBody::into_bytes() panics across every migrated provider/proxy — same root cause, see inline comments on prebid.rs:1157, permutive.rs:151, lockr.rs:149, datadome.rs:301, aps.rs:554, adserver_mock.rs:373. Make parse_response async and read bodies with into_bytes_bounded.
  • Unbounded inbound POST body forwarding on the datadome / didomi / permutive / lockr proxies — only GTM enforces a cap. Promote the GTM size-bounded reader into integrations/mod.rs as collect_body_bounded and apply it uniformly. Inline comments on datadome.rs:367, didomi.rs:245, permutive.rs:211, lockr.rs:203.
  • Lockr .expect() on user-controlled Origin header — reachable worker panic at lockr.rs:266.
  • GTM stream-read errors mis-classified as PayloadTooLarge — returns 413 for transport errors at google_tag_manager.rs:313. Add a StreamRead variant and map to 502.

Non-blocking

🤔 thinking

  • collect_body has no size cap (integrations/mod.rs:101) — silently drains whole bodies to RAM. Same concern applies to the GTM response-rewrite path at google_tag_manager.rs:486.
  • Orchestrator select_result.ready error path drops provider identity in run_providers_parallel — when ready is Err, the provider identity is lost so no AuctionResponse::error(...) is pushed and the failing backend vanishes silently from the result set. Track a (backend_name, provider_index) pair alongside fastly_pending so failures are attributable.
  • Deadline enforcement relies on every provider honoring backend_name(effective_timeout) — Phase 1 computes remaining_ms.min(provider.timeout_ms()) correctly, but nothing guarantees each provider actually threads that timeout through to the backend config. Not a new regression, but the async migration makes it more load-bearing. Add a unit test that asserts the select loop returns before deadline + epsilon.

♻️ refactor

  • aps.rs / adserver_mock.rs still build BackendConfig inline — extend the new ensure_integration_backend helper to cover providers too.

🌱 seedling

  • Make parse_response async in the provider trait (auction/provider.rs:48) — needed anyway once the into_bytes_bounded fix lands; zero cost under #[async_trait(?Send)].

📝 note

  • Testlight stale Content-Length test is narrow (testlight.rs:425) — good unit-level coverage, but the two handle() rebuild call sites would benefit from an end-to-end assertion.

👍 praise

  • Clean helper extraction in integrations/mod.rs — real dedup across six integrations.
  • Dropping the dead || name_str.starts_with("X-") branch in lockr.rs / permutive.rsHeaderName::as_str() is always lowercase, so it was unreachable.
  • Adding CONTENT_TYPE to didomi's forwarded headers — POST bodies can now be interpreted by upstream.

CI Status

  • browser integration tests: PASS
  • integration tests: PASS
  • prepare integration artifacts: PASS

Comment thread crates/trusted-server-core/src/integrations/prebid.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/permutive.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/lockr.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/aps.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/mod.rs
Comment thread crates/trusted-server-core/src/auction/provider.rs
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread crates/trusted-server-core/src/integrations/testlight.rs
Comment thread crates/trusted-server-core/src/integrations/mod.rs
prk-Jr added 2 commits April 16, 2026 13:07
Conflicts resolved by keeping PR13's http-native integration layer
throughout — all compat shim insertions from PR12 are superseded by
this branch's completed integration type migration.
- Add collect_body_bounded helper with INTEGRATION_MAX_BODY_BYTES (256 KiB)
  cap to prevent unbounded memory use on streaming bodies
- Replace all into_bytes() calls (panic on Body::Stream) with collect_body
  or collect_body_bounded across integrations and auction endpoint
- Make AuctionProvider::parse_response async so implementations can safely
  drain response bodies without panicking on the Stream variant
- Add .await to both parse_response call sites in the orchestrator
- Cap inbound request bodies in lockr, permutive, datadome, and didomi
  proxy handlers using collect_body_bounded before forwarding upstream
- Fix lockr Origin header: replace expect() with a warn-and-skip fallback
  so an invalid user-supplied origin cannot crash the edge worker
- Add PayloadSizeError::StreamRead variant to google_tag_manager and return
  502 (not 413) when a stream transport error occurs while reading the body
- Remove extra blank line before closing brace in google_tag_manager impl block
@prk-Jr prk-Jr requested a review from aram356 April 16, 2026 12:53
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.

Integration + provider layer type migration

3 participants