Skip to content

refactor!: refine /transaction route response #5395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sleepyqadir
Copy link

Context

This PR updates the HTTP response codes returned by Iroha when handling transactions, making them more consistent and expressive for different failure modes. Currently, the API:

  • Returns 200 OK when a transaction is accepted, with no response body.
  • Returns 400 Bad Request for a wide variety of issues.
  • Returns 500 Internal Server Error when the queue is full.

Problem

This approach lacks granularity and makes it hard for clients to reliably interpret what went wrong, especially in automated systems. It also does not follow common REST conventions.

Fixes #5391


Solution

This PR introduces the following improvements:

  • 202 Accepted when a transaction is successfully accepted into the pipeline (instead of 200).
  • 400 Bad Request for cases like:
    • Transaction already in queue
    • Transaction already in blockchain
    • Transaction expired or has a future timestamp
    • Transaction exceeds user limits
    • Chain ID mismatch
  • 401 Unauthorized when signature verification fails.
  • 403 Forbidden when the transaction is signed by the genesis account.
  • 503 Service Unavailable when the transaction queue is full.

Migration Guide (optional)

Breaking Change:

  • Clients that expect 200 OK upon transaction acceptance will now receive 202 Accepted. This should be updated in client-side logic.
  • Status code-based error handling in clients may need to be updated to handle new codes: 401, 403, 503.

Review notes (optional)

  • The change centralizes HTTP status code logic in IntoResponse, primarily matching error types.
  • Review the logic around AcceptTransactionFail and PushIntoQueue for accurate status mapping.

Checklist

  • I've read CONTRIBUTING.md.
  • [] All CI checks pass.
  • The response codes are aligned with the issue proposal.
  • The status codes follow conventional RESTful standards.
  • (optional) I've written unit tests for the error mapping logic.
  • [] All review comments have been resolved.

@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Apr 14, 2025
Copy link
Contributor

@0x009922 0x009922 left a comment

Choose a reason for hiding this comment

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

This seems to do the job!

However, please add tests to ensure this behaviour in not broken by future refactors. You can do that in two places:

  • iroha_torii Pytests
  • crates/iroha/tests integration tests

Also, have you run iroha_torii Pytests? They might be failing as they check specific status codes.

@0x009922 0x009922 changed the title chore: (iroha_torii) refine /transaction route response refactor!: refine /transaction route response Apr 15, 2025
@sleepyqadir
Copy link
Author

Yup i saw few tests failing due to status code change, Fixing and adding new tests

@s8sato s8sato marked this pull request as draft May 8, 2025 00:11
@s8sato
Copy link
Contributor

s8sato commented May 8, 2025

I’ve taken the liberty of converting this to a draft for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refine response of /transaction
3 participants