feat: error translation helper for multi-transport servers#178
Conversation
Add translate_error() and normalize_request() to adcp.server so multi-transport servers (MCP + A2A) can translate AdCP errors to protocol-specific formats without duplicating logic. - translate_error(exc, protocol) converts ADCPError/Error to MCP or A2A error response dicts with typed returns (MCPErrorResult/A2AErrorResult) - normalize_request(params) renames deprecated fields (account_id→account, campaign_ref→buyer_campaign_ref) with safe collision handling - Exception types map to structured codes: AUTH_ERROR, TIMEOUT, SERVICE_UNAVAILABLE, INTERNAL_ERROR - Preserves suggestion field from exceptions through translation - 24 tests covering both protocols, all exception types, field renames Closes #176 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bokelley
left a comment
There was a problem hiding this comment.
Review from the Prebid Sales Agent team
Thanks for the quick turnaround on this — these are two of the helpers we asked for in #174. The direction is right but both implementations are too thin for us to adopt as-is. Here's the detail.
normalize_request() — needs to match real-world complexity
The PR handles 2 renames (account_id → account, campaign_ref → buyer_campaign_ref). Our production request_compat.py handles 6+ transforms with structural differences that matter:
| Transform | PR #178 | Our production code |
|---|---|---|
account_id → account |
Simple rename | Nested object: account_id: "123" → account: {account_id: "123"} |
campaign_ref → buyer_campaign_ref |
All tools | Tool-scoped: only on create_media_buy |
brand_manifest → brand |
Missing | URL string → {domain: hostname} (parses URL) |
promoted_offerings → catalogs |
Missing | Direct rename |
Package-level optimization_goal → optimization_goals |
Missing | Scalar → array wrap |
Package-level catalog → catalogs |
Missing | Scalar → array wrap |
| Version inference | Missing | Detects v2.5 callers from field presence |
The account_id transform is especially important to get right — it's not a rename, it's a structural reshape. An agent sending account_id: "123" expects it to become account: {account_id: "123"}, not account: "123".
Suggestion: The task_name parameter is already accepted but reserved. Wire it up now — campaign_ref → buyer_campaign_ref should only apply to create_media_buy, not globally. And add the structural transforms (brand_manifest URL parsing, package-level array wrapping) that every server implementing AdCP backward compat will need.
We're happy to contribute our normalize_request_params() implementation as a reference — it's been stable in production across MCP + A2A for months.
translate_error() — doesn't match how servers actually use errors
Two issues:
1. Returns dicts, but servers need SDK error types.
For A2A, we raise ServerError(InvalidParamsError(...)) — actual a2a SDK error classes. The PR returns a TypedDict with state: "failed" which is the right shape but not the right type. We can't pass a TypedDict to raise ServerError(...).
For MCP, we raise ToolError from fastmcp. The PR returns a TypedDict with content/structuredContent/isError. Same problem — right shape, wrong type.
If the goal is to help multi-transport servers, the helper should return the actual protocol SDK types, or at minimum document that consumers need to construct those types from the returned dicts.
2. Error code mapping diverges from PR #174's standard codes.
The PR maps ADCPAuthenticationError → AUTH_ERROR and ADCPTimeoutError → TIMEOUT. But PR #174 defines AUTH_TOKEN_INVALID and UPSTREAM_TIMEOUT as the standard codes. These should be consistent.
3. Loses error context that buyer agents need.
Our A2A translator preserves the full AdCPError in the data field:
data = {"recovery": exc.recovery, "error_code": exc.error_code}
if exc.details:
data["details"] = exc.detailsThis lets buyer agents reconstruct the original error and make retry/fix/abandon decisions. The PR's A2A format only includes code and message — no recovery classification, no details, no original error_code.
Suggestion: The translate_error() return type should either be the actual SDK error types (importing from a2a and fastmcp as optional deps), or it should be a dataclass with enough structure that servers can trivially construct those types. And it must preserve recovery + details + error_code in the protocol-specific format.
What we'd need to adopt these
-
normalize_request(): Add brand_manifest URL parsing, package-level transforms, tool-scoping for campaign_ref, and theaccount_id→ nested object structural transform. We'd switch immediately. -
translate_error(): Return actual protocol SDK types (with a2a/fastmcp as optional deps), preserve recovery/details/error_code in the output, and align error codes with #174's standard codes. We'd replace both our MCP and A2A translators.
Both are valuable helpers — they just need to handle what real servers actually encounter.
|
A few thoughts on this: Where does The SDK already handles error translation internally for both transports:
So a developer building a standard seller/signals/creative agent with The use case I can see is a proxy server that catches Two error APIs We now have
Only renames 2 fields. If deprecated fields are showing up in real traffic, fine, but this feels like it could be inline in the handler until there's a third rename. Exporting from Suggestion The code itself is clean and well-tested. I'd keep it in |
Respond to Prebid Sales Agent team feedback on #178: translate_error(): - Returns actual SDK types: ToolError (MCP) and ServerError (A2A) - A2A errors use InvalidParamsError for correctable, InternalError for transient/terminal — enables buyer agent retry/fix/abandon decisions - Preserves recovery classification, error_code, suggestion, details, and original error list in A2A error data field - Error codes align with #174 standard codes (AUTH_REQUIRED, SERVICE_UNAVAILABLE) instead of ad-hoc codes - ADCPTaskError preserves original error codes from the response normalize_request(): - account_id → account: {account_id: "..."} (structural reshape) - brand_manifest URL → brand: {domain: hostname} (URL parsing) - promoted_offerings → catalogs (global rename) - campaign_ref → buyer_campaign_ref (create_media_buy only, tool-scoped) - Package-level optimization_goal → optimization_goals (scalar→array) - Package-level catalog → catalogs (scalar→array) - All transforms respect existing fields (no overwrites) - Package dicts are shallow-copied to avoid mutating originals 39 tests covering all transforms, both protocols, error context preservation, tool-scoping, and edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per review feedback: standard servers using serve() or ADCPAgentExecutor already handle error translation internally. These helpers are for proxy servers and custom multi-transport setups. - Remove translate_error/normalize_request from adcp.server exports - Importable directly: from adcp.server.translate import ... - Update module docstring to clarify proxy/custom-transport use case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
translate_error(exc, protocol)to convert AdCP errors (exceptions or Error models) into MCP or A2A protocol-specific error response dictsnormalize_request(params)to rename deprecated field names (account_id→account,campaign_ref→buyer_campaign_ref) with safe collision handlingadcp.serverfor easy use in multi-transport serversTest plan
Closes #176
🤖 Generated with Claude Code