Repository navigation
feat(reporting)!: rename the reporting-subscription errors to the cross-SDK names - #1736
Conversation
…ss-SDK names The buyer and seller errors swap to the names TypeScript uses (DX plan section 2.12), before beta.3 freezes them. - Seller: ReportingSubscriptionError(ValueError), the sync_principal validation error, is renamed ReportingSubscriptionValidationError. - Buyer: ReportingSubscriptionClientError(RuntimeError) becomes ReportingSubscriptionError(ADCPError). It carries code, recovery, dispatched, would_deactivate and readback. The SDK codes and their classification are published as REPORTING_SUBSCRIPTION_ERROR_CODES: SUBSCRIPTION_WOULD_DEACTIVATE, DESTINATION_WOULD_REVOKE, SUBSCRIPTION_ID_CLASH, SUBSCRIPTION_OPERATOR_SCOPE_REQUIRED and SUBSCRIPTION_SELECTOR_UNMATCHED are correctable pre-dispatch refusals; SUBSCRIPTION_CONFLICT_EXHAUSTED is transient and SUBSCRIPTION_READBACK_MISMATCH terminal. A seller refusal keeps its wire code and takes the wire recovery, else the code table. - ReportingSubscriptionClientError remains a deprecated alias for one beta; reading it warns. BREAKING CHANGE: code that caught the seller's ReportingSubscriptionError must catch ReportingSubscriptionValidationError. The buyer error is no longer a RuntimeError; catch ReportingSubscriptionError or ADCPError. Refs #1695 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| if known is not None: | ||
| self.recovery = known[0] | ||
| if known[1]: | ||
| self.dispatched = False |
There was a problem hiding this comment.
Medium: dispatched = False keys solely on the code string, so it cannot tell an SDK-raised local refusal from a seller refusal that happens to carry the same §2.12 code. _readback (L190-194) feeds the seller's wire code straight into this constructor. A conformant counterpart that refuses a dispatched write with SUBSCRIPTION_ID_CLASH/DESTINATION_WOULD_REVOKE/SUBSCRIPTION_WOULD_DEACTIVATE/SUBSCRIPTION_OPERATOR_SCOPE_REQUIRED/SUBSCRIPTION_SELECTOR_UNMATCHED lands here, gets stamped dispatched=False, and is_pre_dispatch_error returns True — contradicting the PR's own invariant "a seller refusal is not pre-dispatch" (asserted in test_rejection_surfaces_the_seller_code, which only exercises INVALID_REQUEST). The known-branch also discards the wire recovery for these codes. This SDK's seller emits INVALID_REQUEST/UNSUPPORTED_FEATURE today, so it won't bite in-tree, but the §2.12 vocabulary is a wire vocabulary and the readback path has no test with a §2.12 code on the seller side.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — breaking rename of reporting-subscription errors to cross-SDK names, correctly signaled.
This PR renames the reporting-subscription errors to the cross-SDK names (seller ReportingSubscriptionError(ValueError) → ReportingSubscriptionValidationError; buyer ReportingSubscriptionClientError(RuntimeError) → ReportingSubscriptionError(ADCPError) with code/recovery/dispatched/would_deactivate/readback fields and the §2.12 code table). The breaking change is carried as feat(reporting)!: with a BREAKING CHANGE footer and migration note, satisfying the semver-signal floor. The deprecated ReportingSubscriptionClientError alias warns via module __getattr__; no stray references to old names remain, names aren't re-exported above the package, and required imports are present. The registry rename is mechanical.
Medium findings
- src/adcp/reporting/subscriptions/consumer.py:107 —
dispatched=Falsestamped on seller refusals carrying a §2.12 code, so a refusal on the readback path is misclassified as pre-dispatch.
Decision path: No critical/high findings (row 1 n/a). gated_paths false (row 2 n/a). high_risk true but only a (modified) on principal.py with NO medium finding on that file (rows 3/5 n/a — the single medium is on consumer.py). The one medium is not data-loss/schema/infra category (row 4 n/a). No prior decision (row 6 n/a). No team gate (row 7 n/a). Only 1 medium finding, below the 3-finding threshold (row 8 n/a). Falls through to row 9 → approve.
Medium findings
- src/adcp/reporting/subscriptions/consumer.py:107 — dispatched=False stamped on seller refusals carrying a §2.12 code
Summary
Gate slice P3 of the reporting-subscriptions DX plan (§2.12, §6.2), stacked on #1735. The reporting-subscription errors take the cross-SDK names before beta.3 freezes them.
ReportingSubscriptionError(ValueError)ReportingSubscriptionValidationError(ValueError)ReportingSubscriptionClientError(RuntimeError)ReportingSubscriptionError(ADCPError)Fields. The buyer error carries
code,recovery,dispatched,would_deactivateandreadback.Codes.
REPORTING_SUBSCRIPTION_ERROR_CODESpublishes the §2.12 codes with their classification:SUBSCRIPTION_WOULD_DEACTIVATE,DESTINATION_WOULD_REVOKE,SUBSCRIPTION_ID_CLASH,SUBSCRIPTION_OPERATOR_SCOPE_REQUIREDandSUBSCRIPTION_SELECTOR_UNMATCHEDare correctable, withdispatched=False;SUBSCRIPTION_CONFLICT_EXHAUSTEDis transient;SUBSCRIPTION_READBACK_MISMATCHis terminal.P4's write facade raises them.
Seller refusals keep the wire code, and take the wire
recovery, else the code table, else transient.Alias.
ReportingSubscriptionClientErrorstays a deprecated alias for one beta, and reading it warns. The seller's old name can't be aliased, because the buyer error now uses it.Breaking (beta):
ReportingSubscriptionValidationErroron the seller side.RuntimeError.Codes follow the DX plan's table. I'll align them with the spec-repo vectors (X1) when they land.
Tests
Refs #1695
🤖 Generated with Claude Code