Fix errors, move some handlers over to handler modules #498

Open
ryan wants to merge 4 commits from ryansquared/fix-errors into main
Owner
No description provided.
Rationale:
* Replace the shared OrgQuorumError (which carried a StatusCode and
  client text) with per-function, status-free dterror types; statuses
  and client strings are now chosen only in IntoResponse.
* Colocate the /quorum-bundles and /secrets-bundles handlers in
  cryptographic_bundles::handlers and the org handlers in
  org_quorum::handlers.
Rationale:
* A reviewer found two status regressions from the typed-error rewrite.
* CredentialSnapshotError::Encode (serialize failure) is 500 again
  instead of collapsing into the holder-selection 400.
* VerifyProofError::Policy (Nitro policy failure) is 503 again
  instead of collapsing into the certificate-service 502.
* The DB e2e test now asserts the real handler IntoResponse status
  rather than a duplicated mapping.
Owner

[P2] Preserve the existing public quorum error responses

Reviewed d4c6368ce678405753cb5de12f11ae994214c731 against 1fad41270748eff75de9027b6469b5c4135b7acf. The typed-error/handler refactor changes client-visible bodies, losing actionable guidance and invalidating existing E2E assertions.

Please keep the typed errors and extracted handlers, but make these changes before merging:

  1. Restore variant-specific, location-free generation messages in org_quorum/handlers.rs:126–145. For example, threshold zero previously returned HTTP 400 threshold must be between 1 and the holder count, but now returns invalid quorum request. An unverified passkey similarly loses the instruction to verify with PIN/biometrics in Dashboard Authentication or use external PGP, returning only invalid quorum holder selection. The CLI displays the API body, so these explanations matter to users.

  2. Preserve upload verification messages in the shared create/update mapper, cryptographic_bundles/handlers.rs:128–137. A legacy upload without opt-in should still explain legacy upload requires explicit allow_legacy: true, rather than just invalid quorum bundle.

  3. Preserve the proof-failure bodies and add focused IntoResponse status/body tests. The mappings at org_quorum/handlers.rs:208–222 no longer satisfy the unchanged E2E assertions at quorum.rs:478–491: does not satisfy the CLI's trusted Keymaker policy and Keymaker response does not match the requested quorum. Keep those expectations and cover the actual emitted responses, not only internal error variants. Use fixed literals or a dedicated client-message accessor; do not expose location-bearing Display output.

Verification:

CARGO_TARGET_DIR=/home/tuna/git/platform/target cargo +1.96.1 test --locked -p api --bin api org_quorum:: -- --nocapture
# 29 passed, 1 opt-in database test ignored
CARGO_TARGET_DIR=/home/tuna/git/platform/target cargo +1.96.1 test --locked -p api --bin api cryptographic_bundles:: -- --nocapture
# 6 passed

Five temporary review-only probes exercising the real IntoResponse mappings reproduced all five body incompatibilities above; statuses were preserved. The temporary import was removed and the exact-head unit tests rechecked. Full E2E/deployment tests were not run for this PR.

### [P2] Preserve the existing public quorum error responses Reviewed `d4c6368ce678405753cb5de12f11ae994214c731` against `1fad41270748eff75de9027b6469b5c4135b7acf`. The typed-error/handler refactor changes client-visible bodies, losing actionable guidance and invalidating existing E2E assertions. Please keep the typed errors and extracted handlers, but make these changes before merging: 1. **Restore variant-specific, location-free generation messages** in [`org_quorum/handlers.rs:126–145`](https://git.caution.co/public/platform/src/commit/d4c6368ce678405753cb5de12f11ae994214c731/src/api/src/org_quorum/handlers.rs#L126-L145). For example, threshold zero previously returned HTTP 400 `threshold must be between 1 and the holder count`, but now returns `invalid quorum request`. An unverified passkey similarly loses the instruction to verify with PIN/biometrics in Dashboard Authentication or use external PGP, returning only `invalid quorum holder selection`. The CLI displays the API body, so these explanations matter to users. 2. **Preserve upload verification messages** in the shared create/update mapper, [`cryptographic_bundles/handlers.rs:128–137`](https://git.caution.co/public/platform/src/commit/d4c6368ce678405753cb5de12f11ae994214c731/src/api/src/cryptographic_bundles/handlers.rs#L128-L137). A legacy upload without opt-in should still explain `legacy upload requires explicit allow_legacy: true`, rather than just `invalid quorum bundle`. 3. **Preserve the proof-failure bodies and add focused `IntoResponse` status/body tests.** The mappings at [`org_quorum/handlers.rs:208–222`](https://git.caution.co/public/platform/src/commit/d4c6368ce678405753cb5de12f11ae994214c731/src/api/src/org_quorum/handlers.rs#L208-L222) no longer satisfy the unchanged [E2E assertions at `quorum.rs:478–491`](https://git.caution.co/public/platform/src/commit/d4c6368ce678405753cb5de12f11ae994214c731/tests/e2e/soft-authenticator/src/quorum.rs#L478-L491): `does not satisfy the CLI's trusted Keymaker policy` and `Keymaker response does not match the requested quorum`. Keep those expectations and cover the actual emitted responses, not only internal error variants. Use fixed literals or a dedicated client-message accessor; do not expose location-bearing `Display` output. **Verification:** ```sh CARGO_TARGET_DIR=/home/tuna/git/platform/target cargo +1.96.1 test --locked -p api --bin api org_quorum:: -- --nocapture # 29 passed, 1 opt-in database test ignored CARGO_TARGET_DIR=/home/tuna/git/platform/target cargo +1.96.1 test --locked -p api --bin api cryptographic_bundles:: -- --nocapture # 6 passed ``` Five temporary review-only probes exercising the real `IntoResponse` mappings reproduced all five body incompatibilities above; statuses were preserved. The temporary import was removed and the exact-head unit tests rechecked. Full E2E/deployment tests were not run for this PR.
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin ryansquared/fix-errors:ryansquared/fix-errors
git switch ryansquared/fix-errors
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
public/platform!498
No description provided.