Skip to content

feat(proof-gen-api): return 422 for BlockNotReady#889

Open
DylanVerstraete wants to merge 1 commit intousc-devfrom
fix/block-not-ready-422
Open

feat(proof-gen-api): return 422 for BlockNotReady#889
DylanVerstraete wants to merge 1 commit intousc-devfrom
fix/block-not-ready-422

Conversation

@DylanVerstraete
Copy link
Copy Markdown
Contributor

What

Map BlockNotReady from the proof-gen API to HTTP 422 Unprocessable Entity instead of 404.

Requested by Dave: when a client waits until a block is attested but asks for a proof before the attestation is finalized, the server returned an opaque error. On the client side this surfaced as a generic 500 (because the SDK retry/wrap path didn't preserve the underlying status), which covers too many failure modes. 422 makes it cleanly distinguishable from "the tx/block will never exist" (404) and "transient infra failure" (5xx).

Why 422?

  • The request is well-formed (valid chain, valid block number that exists on the source chain).
  • It cannot be processed in the current state — the block exists but isn't attested yet.
  • That's the canonical 422 use case. 404 was wrong because the resource will exist; the client should keep polling. retriable: true in the body still tells clients to wait.

Changes

  • proof-gen-api-server/src/services/errors.rs
    • ServiceError::BlockNotReadyStatusCode::UNPROCESSABLE_ENTITY
    • TxHashNotFound and BlockNotOnSourceChain stay on 404 (they're genuinely "not found").
  • proof-gen-api-server/src/networking/routes/continuity.rs
    • Added (status = 422, …) to the utoipa responses(...) on all four proof endpoints so the generated OpenAPI/Swagger spec advertises it.
  • proof-gen-api-server/README.md
    • Documented the 422 alongside the example BlockNotReady body.
  • proof-gen-api-server/tests/route_tests.rs
    • Bumped the swagger-json to_bytes cap from 10 KiB to 32 KiB. The four extra response variants pushed the doc past the old limit and broke test_swagger_json_route_should_return_valid_json with a LengthLimitError. 32 KiB gives headroom for future endpoints.

Client impact

The traffic simulator and the SDK already retry on retriable || status >= 500, and BlockNotReady is retriable: true, so existing waiters keep retrying as before. The only behavioral change is callers that key off the HTTP status now get a stable, accurate code instead of a 500/404 grab-bag.

Verification

  • cargo fmt -p proof-gen-api-server
  • cargo clippy -p proof-gen-api-server --all-targets -- -D warnings
  • cargo test -p proof-gen-api-server ✅ (27 + 14 tests, all green)

@DylanVerstraete DylanVerstraete requested review from a team, BradleyOlson64, Trantorian1, atodorov, beqaabu, creditcoinprotoclaw and didac-gluwa and removed request for a team April 28, 2026 13:33
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 28, 2026

PR Summary

Medium Risk
Changes HTTP status semantics for BlockNotReady from 404 to 422, which may affect clients that branch on status codes (even though response bodies and retry hints remain the same). Scope is limited to error mapping, OpenAPI docs, and a test adjustment.

Overview
Updates proof-gen API error semantics by mapping ServiceError::BlockNotReady to HTTP 422 Unprocessable Entity (instead of 404), while keeping true not-found cases (TxHashNotFound, BlockNotOnSourceChain) on 404.

API documentation is updated so all proof endpoints advertise the new 422 response in the generated OpenAPI spec, and the README now explicitly documents BlockNotReady as 422.

Tests increase the Swagger JSON response size limit (10 KiB → 32 KiB) to accommodate the expanded OpenAPI document.

Reviewed by Cursor Bugbot for commit 40c2720. Bugbot is set up for automated code reviews on this repo. Configure here.

@DylanVerstraete DylanVerstraete requested a review from a team April 28, 2026 13:33
Block exists but the requested block is not yet attested. The request
is well-formed; it just can't be processed in the current state. 404
conflated this with "the block/tx will never exist" and forced clients
to inspect the body to tell the two apart.

Map BlockNotReady to 422 Unprocessable Entity so clients (e.g. the
traffic simulator and SDK) can distinguish "keep waiting" from "give
up" by status code alone, instead of pattern-matching on a 500 or 404
body.

- errors.rs: BlockNotReady -> UNPROCESSABLE_ENTITY (other NOT_FOUND
  cases unchanged).
- continuity routes: add 422 to utoipa response specs so OpenAPI/
  Swagger reflects the new status.
- README: document the 422 status alongside the example body.
- tests: bump swagger-json body cap to 32 KiB so future response
  additions don't trip the LengthLimitError again.

Requested by Dave.
@DylanVerstraete DylanVerstraete force-pushed the fix/block-not-ready-422 branch from 1666a9b to 40c2720 Compare April 28, 2026 15:29
Copy link
Copy Markdown
Contributor

@BradleyOlson64 BradleyOlson64 left a comment

Choose a reason for hiding this comment

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

This pr doesn't actually do the thing we need it to. We need to discover why the following edge case produces error code 500. Then we need to change the error handling so that it gives code 422 instead.

Edge case:

  1. An attestation was just committed on-chain which covers our target block
  2. That attestation hasn't been finalized yet at the time when the proof gen api receives its proving request

Original message:

Dave wanted me to flag this issue. Basically, we get a really opaque error message when we wait until a block is attested but request a proof from the proof gen server before the attestation is finalized:

Error: Failed to generate proof: Failed to generate proof via API: Error: Failed to fetch proof: AxiosError: Request failed with status code 500

 Error code 500 covers quite a few error cases, so I haven't hunted down which one we're hitting. Dave requested this as an alternative error code in this case: 422 Unprocessable Entity

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.

4 participants