Forward error code #3383
Forward error code #3383
Conversation
BillyWooo
requested review from
kziemianek,
Kailai-Wang,
silva-fj and
BillyWooo
| let internal_error: ErrorObject = ErrorCode::InternalError.into(); | ||
| let params = params.parse::<AddWalletParams>()?; | ||
| let params = params.parse::<AddWalletParams>().map_err(|_| { | ||
| log::error!("Failed to parse params"); |
There was a problem hiding this comment.
Doesn't make sense to log reason here (e) ?
|
|
||
| pub fn from_error_code(error_code: ErrorCode) -> Self { | ||
| let (code, message) = match error_code { | ||
| ErrorCode::ParseError => (-32700, "Parse error"), |
There was a problem hiding this comment.
These messages are all in english while the ones returned from pumpx backend will be i18 , is that correct ?
There was a problem hiding this comment.
Here is jsonrpsee internal error code. It's fine.
Yes. They all should be |
| { | ||
| if response.code != 10000 { | ||
| log::error!("{} failed: code={}, message={}", name, response.code, response.message); | ||
| return Err(PumpxRpcError::from_code_and_message(response.code as i32, response.message)); |
There was a problem hiding this comment.
Does it mean if we detect errors from pump-api response, we'll use their code and msg as our response and return it to F/E?
Could that be problematic for F/E to tell if the error actually comes from worker or backend? Especially when the error code happens to coincide.
Would this makes more sense? Like
{
"code": -32055, // just as example
"message": "Pumpx API error",
"error": {
"backend_response": { // the raw backend response
"code": -12345,
"message": ...,
"data": {}
}
}
}
So here -32055 means we return because backend returns error - so we propagate it
| Self { code, message } | ||
| } | ||
|
|
||
| pub fn from_error_code(error_code: ErrorCode) -> Self { |
There was a problem hiding this comment.
How about to have:
pub fn from_error_code(error_code: ErrorCode) -> Self {
Self { code: error_code.code(), message: error_code.message().to_string() }
}
As all ResponseData should be Option, I think there is no need to make any changes, as the response of jsonrpc already has a different response structure for success and error. Success response example like below Error response example like below |
wli-pro
force-pushed
the
forward-error-code
branch
2 times, most recently
from
3b8b651 to
19a50df
Compare
…ract duplicated code into new function, add more logs
…return backend_response, add some error info to log
wli-pro
force-pushed
the
forward-error-code
branch
from
19a50df to
57bd447
Compare
* first prototype * feat: add struct PumpxRpcError for error processing in pumpx rpc, extract duplicated code into new function, add more logs * fix: update data of ApiResponse to Optional, update PumpxRpcError to return backend_response, add some error info to log --------- Co-authored-by: BillyWooo <yang@trustcomputing.de> Co-authored-by: wli-pro <wli-pro>
Context
Labels
Please apply following PR-related labels when appropriate:
C0-breaking: if your change could break the existing client, e.g. API change, critical logic changeC1-noteworthy: if your change is non-breaking, but is still worth noticing for the client, e.g. reference code improvementHow (Optional)
Testing Evidences
Please attach any relevant evidences if applicable