Skip to content

Conversation

@thedevbirb
Copy link
Contributor

@thedevbirb thedevbirb commented Jan 10, 2025

Some internal errors were reported with a wrong status code, and others were not matching the JSON-RPC specification closely.
The changes required a small refactoring here and there.

impl From<CommitmentError> for JsonError {
fn from(err: CommitmentError) -> Self {
// Reference: https://www.jsonrpc.org/specification#error_object
// TODO: the custom defined ones should be clearly documented.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should write custom defined errors in the bolt documentation website

Copy link
Contributor Author

@thedevbirb thedevbirb Jan 13, 2025

Choose a reason for hiding this comment

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

cc @mempirate, if you want to take a look at current error codes

@thedevbirb thedevbirb force-pushed the lore/fix/error-codes branch from 763126a to 6c22d7a Compare January 13, 2025 10:37
@thedevbirb thedevbirb requested review from estensen and merklefruit and removed request for merklefruit January 13, 2025 16:05
@thedevbirb thedevbirb marked this pull request as ready for review January 13, 2025 16:05
@thedevbirb thedevbirb force-pushed the lore/fix/error-codes branch from d603515 to d94bb81 Compare January 13, 2025 16:06
Comment on lines +68 to +70
/// FIXME: (thedevbirb, 2025-13-01) this should be removed because it is dead code,
/// but it allows Rust to pull the correct axum version and not older ones from
/// dependencies (commit-boost).
Copy link
Collaborator

@merklefruit merklefruit Jan 13, 2025

Choose a reason for hiding this comment

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

huh... what? this is a new one...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I'm getting if I remove that variant:

error[E0277]: the trait bound `WithRejection<axum::Json<JsonRpcRequest>, CommitmentError>: FromRequest<Arc<CommitmentsApiInner>, _>` is not satisfied because the trait comes from a different crate version
  --> src/api/commitments/server/handlers.rs:51:38
   |
51 |     WithRejection(Json(payload), _): WithRejection<Json<JsonRpcRequest>, CommitmentError>,
   |                                      ^^^^^^^^^^^^^ the trait `FromRequest<Arc<CommitmentsApiInner>, _>` is not implemented for `WithRejection<axum::Json<JsonRpcRequest>, CommitmentError>`
   |
   = note: Function argument is not a valid axum extractor.
           See `https://docs.rs/axum/0.8/axum/extract/index.html` for details
note: there are multiple different versions of crate `axum_core` in the dependency graph
  --> /Users/birb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-core-0.5.0/src/extract/mod.rs:85:1
   |
85 | pub trait FromRequest<S, M = private::ViaRequest>: Sized {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: src/api/builder.rs:3:5
   |
3  | use axum::{
   |     ---- one version of crate `axum_core` is used here, as a dependency of crate `axum`
   |
  ::: src/signer/commit_boost.rs:8:5
   |
8  | use cb_common::{
   |     --------- one version of crate `axum_core` is used here, as a dependency of crate `axum`
   |
  ::: /Users/birb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-extra-0.10.0/src/extract/with_rejection.rs:60:1
   |
60 | pub struct WithRejection<E, R>(pub E, pub PhantomData<R>);
   | ------------------------------ this type doesn't implement the required trait
   |
  ::: /Users/birb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-core-0.4.5/src/ext_traits/request.rs:6:5
   |
6  |     pub trait Sealed {}
   |     ---------------- this is the found trait
   = help: you can use `cargo tree` to explore your dependency tree

error[E0277]: the trait bound `fn(HeaderMap, axum::extract::State<Arc<CommitmentsApiInner>>, WithRejection<axum::Json<JsonRpcRequest>, CommitmentError>) -> impl futures::Future<Output = Result<axum::Json<JsonRpcResponse>, CommitmentError>> {handlers::rpc_entrypoint}: Handler<_, _>` is not satisfied because the trait comes from a different crate version
   --> src/api/commitments/server/mod.rs:170:26
    |
170 |         .route("/", post(handlers::rpc_entrypoint))
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Handler<_, _>` is not implemented for fn item `fn(HeaderMap, State<Arc<CommitmentsApiInner>>, WithRejection<Json<JsonRpcRequest>, ...>) -> ... {rpc_entrypoint}`
    |
    = note: the full name for the type has been written to '/Users/birb/work/cb/bolt/bolt-sidecar/target/debug/deps/bolt_sidecar-4f96b73b107ca223.long-type-2057463047293510438.txt'
    = note: consider using `--verbose` to print the full type name to the console
    = note: Consider using `#[axum::debug_handler]` to improve the error message
note: there are multiple different versions of crate `axum` in the dependency graph
   --> /Users/birb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-0.8.1/src/handler/mod.rs:134:1
    |
134 | pub trait Handler<T, S>: Clone + Send + Sync + Sized + 'static {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this is the required trait
    |
   ::: src/api/builder.rs:3:5
    |
3   | use axum::{
    |     ---- one version of crate `axum` is used here, as a direct dependency of the current crate
    |
   ::: src/signer/commit_boost.rs:8:5
    |
8   | use cb_common::{
    |     --------- one version of crate `axum` is used here, as a dependency of crate `cb_common`
    |
   ::: /Users/birb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-0.7.9/src/boxed.rs:62:1
    |
62  | pub(crate) trait ErasedIntoRoute<S, E>: Send {
    | -------------------------------------------- this is the found trait
    = help: you can use `cargo tree` to explore your dependency tree

For more information about this error, try `rustc --explain E0277`.
warning: `bolt-sidecar` (lib) generated 1 warning
error: could not compile `bolt-sidecar` (lib) due to 2 previous errors; 1 warning emitted

I cannot override the axum version of the dependency because it is a breaking change. I think it's fine. Let's wait until they update

Copy link
Collaborator

@merklefruit merklefruit Jan 14, 2025

Choose a reason for hiding this comment

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

Ok fine with me! Will try to bump axum on commit boost

edit: Commit-Boost/commit-boost-client#232

Copy link
Contributor

@mempirate mempirate left a comment

Choose a reason for hiding this comment

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

Nice job

@mempirate mempirate merged commit 75ed391 into unstable Jan 14, 2025
3 checks passed
@mempirate mempirate deleted the lore/fix/error-codes branch January 14, 2025 11:08
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