Skip to content

Allow solvers to return custom errors#4232

Open
ashleychandy wants to merge 3 commits intocowprotocol:mainfrom
ashleychandy:feat/4222-solver-custom-errors
Open

Allow solvers to return custom errors#4232
ashleychandy wants to merge 3 commits intocowprotocol:mainfrom
ashleychandy:feat/4222-solver-custom-errors

Conversation

@ashleychandy
Copy link

Description

Fixes #4222

Previously, when solvers couldn't compute quotes for RWA tokens, they returned a generic QuotingFailed error. This made it difficult for the frontend to provide helpful feedback.

This change adds support for custom solver errors that can be returned when quoting fails. Solvers can now indicate specific reasons like:

  • TradingOutsideAllowedWindow
  • TokenTemporarilySuspended
  • InsufficientLiquidity
  • UnauthorizedTrader
  • Other

When a solver returns a custom error, the driver propagates it through the API response so the frontend can display an appropriate error message.

Changes

  • Added SolverError enum with 5 variants in solvers-dto
  • Added optional error field to Solutions DTO
  • Updated driver to detect and propagate custom solver errors
  • Added 5 new error kinds to the API error handler
  • Updated observability to track SolverCustomError metric
  • Updated mock solver in e2e tests

@ashleychandy ashleychandy requested a review from a team as a code owner March 5, 2026 09:24
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ashleychandy
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Mar 5, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism for solvers to return custom errors, which are then propagated to the API. This is a valuable improvement for providing more specific feedback to users. The implementation is solid, with changes across the DTOs, driver, and observability components. I've identified one area in the error handling logic where code duplication can be reduced to improve maintainability.

Comment on lines +87 to +132
let (kind, description) = match custom_err {
solvers_dto::solution::SolverError::TradingOutsideAllowedWindow { message } => {
(
Kind::TradingOutsideAllowedWindow,
message.clone().unwrap_or_else(||
"Token can only be traded during specific time windows".to_string()
),
)
}
solvers_dto::solution::SolverError::TokenTemporarilySuspended { message } => {
(
Kind::TokenTemporarilySuspended,
message.clone().unwrap_or_else(||
"Token is temporarily suspended from trading".to_string()
),
)
}
solvers_dto::solution::SolverError::InsufficientLiquidity { message } => {
(
Kind::InsufficientLiquidity,
message.clone().unwrap_or_else(||
"Insufficient liquidity for the requested trade size".to_string()
),
)
}
solvers_dto::solution::SolverError::UnauthorizedTrader { message } => {
(
Kind::UnauthorizedTrader,
message.clone().unwrap_or_else(||
"Token requires special permissions or whitelisting".to_string()
),
)
}
solvers_dto::solution::SolverError::Other { message } => {
(
Kind::CustomSolverError,
message.clone().unwrap_or_else(||
"Solver returned a custom error".to_string()
),
)
}
};
return (
axum::http::StatusCode::BAD_REQUEST,
axum::Json(Error { kind, description }),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This match statement contains significant code duplication for handling the optional message and providing a default description. This can be refactored to reduce complexity and improve maintainability by separating the logic for identifying the error type from the logic for constructing the description string.

                let (kind, message, default_description) = match custom_err {
                    solvers_dto::solution::SolverError::TradingOutsideAllowedWindow { message } => (
                        Kind::TradingOutsideAllowedWindow,
                        message,
                        "Token can only be traded during specific time windows",
                    ),
                    solvers_dto::solution::SolverError::TokenTemporarilySuspended { message } => (
                        Kind::TokenTemporarilySuspended,
                        message,
                        "Token is temporarily suspended from trading",
                    ),
                    solvers_dto::solution::SolverError::InsufficientLiquidity { message } => (
                        Kind::InsufficientLiquidity,
                        message,
                        "Insufficient liquidity for the requested trade size",
                    ),
                    solvers_dto::solution::SolverError::UnauthorizedTrader { message } => (
                        Kind::UnauthorizedTrader,
                        message,
                        "Token requires special permissions or whitelisting",
                    ),
                    solvers_dto::solution::SolverError::Other { message } => (
                        Kind::CustomSolverError,
                        message,
                        "Solver returned a custom error",
                    ),
                };
                let description = message.as_deref().unwrap_or(default_description).to_string();
                return (
                    axum::http::StatusCode::BAD_REQUEST,
                    axum::Json(Error { kind, description }),
                );

// Check if this is a custom solver error
if let quote::Error::Solver(ref solver_err) = value {
if let Some(custom_err) = solver_err.custom_error() {
let (kind, description) = match custom_err {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be it's own function

Copy link
Contributor

Choose a reason for hiding this comment

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

All message fields are optional, that hints that the modelling is not quite there

Copy link
Contributor

Choose a reason for hiding this comment

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

From the code alone I don't see how this change modifies anything in the rest of the code

/// Optional custom error that explains why no solutions could be computed.
/// When multiple solvers return errors, the system will pick one to return.
#[serde(skip_serializing_if = "Option::is_none")]
pub error: Option<SolverError>,
Copy link
Contributor

Choose a reason for hiding this comment

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

If the solution is the payload the solver currently returns, adding an error field is a bit confusing as it's no longer a solution

Feels to me that an enum would be better suited (names up for discussion)

enum SolverResponse { Solution { solutions: Vec<Solution> }, Error { code: SolverError, message: Option<String> }

Separating the message from the code also brings more flexibility

Co-authored-by: José Duarte <duarte.gmj@gmail.com>
@ashleychandy
Copy link
Author

Thanks for the review! Implemented the changes you suggested - separated error code from message, refactored SolverResponse to a proper enum, and simplified the error mapping.

@ashleychandy ashleychandy requested a review from jmg-duarte March 5, 2026 11:46
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.

Allow solvers to return custom errors

2 participants