Skip to content
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.

Sign up for GitHub

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

fix(http client): more user-friendly error messages when decoding fails #853

Merged
merged 6 commits into from
Sep 15, 2022
Merged

fix(http client): more user-friendly error messages when decoding fails #853

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5faaa4a
fix(http client): serde_json::Value -> T
niklasad1 Aug 11, 2022
1f11fc3
Merge remote-tracking branch 'origin/master' into na-more-user-friend…
niklasad1 Aug 16, 2022
43d415c
address grumbles
niklasad1 Aug 17, 2022
42c55a6
Update client/http-client/src/client.rs
niklasad1 Aug 17, 2022
db083b8
Update client/http-client/src/client.rs
niklasad1 Aug 17, 2022
54a4c71
Merge remote-tracking branch 'origin/master' into na-more-user-friend…
niklasad1 Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions client/http-client/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use async_trait::async_trait;
use hyper::http::HeaderMap;
use jsonrpsee_core::client::{CertificateStore, ClientT, IdKind, RequestIdManager, Subscription, SubscriptionClientT};
use jsonrpsee_core::tracing::RpcTracing;
use jsonrpsee_core::{Error, TEN_MB_SIZE_BYTES};
use jsonrpsee_core::{Error, JsonRawValue, TEN_MB_SIZE_BYTES};
use jsonrpsee_types::error::CallError;
use rustc_hash::FxHashMap;
use serde::de::DeserializeOwned;
Expand Down Expand Up @@ -207,16 +207,20 @@ impl ClientT for HttpClient {
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dumb (unrelated to this PR) question: what's the purpose of this async block?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's part of the tracing stuff bug fix, it should be fixed by #846


let response: Response<_> = match serde_json::from_slice(&body) {
// NOTE: it's decoded first `JsonRawValue` and then to `R` below to get
// a better error message if `R` couldn't be decoded.
let response: Response<&JsonRawValue> = match serde_json::from_slice(&body) {
Ok(response) => response,
Err(_) => {
let err: ErrorResponse = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Call(CallError::Custom(err.error_object().clone().into_owned())));
}
};

let result = serde_json::from_str(response.result.get()).map_err(Error::ParseError)?;

if response.id == id {
Ok(response.result)
Ok(result)
} else {
Err(Error::InvalidRequestId)
}
Expand Down Expand Up @@ -254,7 +258,9 @@ impl ClientT for HttpClient {
Ok(Err(e)) => return Err(Error::Transport(e.into())),
};

let rps: Vec<Response<_>> =
// NOTE: it's decoded first `JsonRawValue` and then to `R` below to get
// a better error message if `R` couldn't be decoded.
let rps: Vec<Response<&JsonRawValue>> =
serde_json::from_slice(&body).map_err(|_| match serde_json::from_slice::<ErrorResponse>(&body) {
Ok(e) => Error::Call(CallError::Custom(e.error_object().clone().into_owned())),
Err(e) => Error::ParseError(e),
Expand All @@ -267,7 +273,8 @@ impl ClientT for HttpClient {
Some(pos) => *pos,
None => return Err(Error::InvalidRequestId),
};
responses[pos] = rp.result
let result = serde_json::from_str(rp.result.get()).map_err(Error::ParseError)?;
responses[pos] = result;
}
Ok(responses)
}
Expand Down