Skip to content

Commit

Permalink
Include invalid JSON in an error returned with command execution
Browse files Browse the repository at this point in the history
  • Loading branch information
ryo33 committed Feb 16, 2024
1 parent 3489675 commit 20ef13b
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 16 deletions.
2 changes: 1 addition & 1 deletion chromiumoxide_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ include = ["src/**/*", "LICENSE-*"]

[dependencies]
serde = { version = "1.0.130", features = ["derive"] }
serde_json = "1.0.72"
serde_json = { version = "1.0.72", features = ["raw_value"] }
13 changes: 8 additions & 5 deletions chromiumoxide_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ pub trait Command: serde::ser::Serialize + Method {
/// The type of the response this request triggers on the chromium server
type Response: serde::de::DeserializeOwned + fmt::Debug;

/// deserialize the response from json
fn response_from_value(response: serde_json::Value) -> serde_json::Result<Self::Response> {
serde_json::from_value(response)
/// A helper method to make the response type inferenced without fullly qualifying response type that returns the identical value.
///
/// For example: `let response = serde_json::from_str(res).map(CaptureSnapshotParams::infer_response)?;`
/// instead of `let response = serde_json::from_str::<<CaptureSnapshotParams as Command>::Response>(res)?;`
fn infer_response(res: Self::Response) -> Self::Response {
res
}
}

Expand Down Expand Up @@ -186,12 +189,12 @@ impl Request {
}

/// A response to a [`MethodCall`] from the chromium instance
#[derive(Deserialize, Debug, PartialEq, Eq, Clone)]
#[derive(Deserialize, Debug, Clone)]
pub struct Response {
/// Numeric identifier for the exact request
pub id: CallId,
/// The response payload
pub result: Option<serde_json::Value>,
pub result: Option<Box<serde_json::value::RawValue>>,
/// The Reason why the [`MethodCall`] failed.
pub error: Option<Error>,
}
Expand Down
14 changes: 8 additions & 6 deletions src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ pub(crate) fn to_command_response<T: Command>(
method: MethodId,
) -> Result<CommandResponse<T::Response>> {
if let Some(res) = resp.result {
let result = serde_json::from_value(res)?;
Ok(CommandResponse {
id: resp.id,
result,
method,
})
match serde_json::from_str(res.get()) {
Ok(result) => Ok(CommandResponse {
id: resp.id,
result,
method,
}),
Err(error) => Err(CdpError::InvalidResponse { result: res, error }),
}
} else if let Some(err) = resp.error {
Err(err.into())
} else {
Expand Down
6 changes: 6 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ pub enum CdpError {
JavascriptException(Box<ExceptionDetails>),
#[error("{0}")]
Url(#[from] url::ParseError),
#[error("got invalid response with error: {error}")]
InvalidResponse {
// not Value to avoid deserialization and additional allocations
result: Box<serde_json::value::RawValue>,
error: serde_json::Error,
},
}
impl CdpError {
pub fn msg(msg: impl Into<String>) -> Self {
Expand Down
9 changes: 5 additions & 4 deletions src/handler/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,11 @@ impl Target {
#[allow(clippy::single_match)] // allow for now
match method {
GetFrameTreeParams::IDENTIFIER => {
if let Some(resp) = resp
.result
.and_then(|val| GetFrameTreeParams::response_from_value(val).ok())
{
if let Some(resp) = resp.result.and_then(|val| {
serde_json::from_str(val.get())
.map(GetFrameTreeParams::infer_response)
.ok()
}) {
self.frame_manager.on_frame_tree(resp.frame_tree);
}
}
Expand Down

0 comments on commit 20ef13b

Please sign in to comment.