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.

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

Include invalid JSON in an error returned with command execution #209

Closed
wants to merge 2 commits into from

Conversation

ryo33
Copy link
Contributor

@ryo33 ryo33 commented Feb 16, 2024

Related: #167

In the main branch, users cannot get invalid JSON after getting an error with calling a function that executes a command, and it does not help to write fallback logic on receiving invalid responses from Chromium.

In my case, I had created a custom command just for getting serde_json::Value like the following.

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct CustomGetFullAxTree;

impl Command for CustomGetFullAxTree {
    type Response = Value;
}

impl Method for CustomGetFullAxTree {
    fn identifier(&self) -> chromiumoxide::types::MethodId {
        chromiumoxide::types::MethodId::Borrowed("Accessibility.getFullAXTree")
    }
}

@ryo33 ryo33 changed the title [snail] Include invalid JSON in an error returned with command execution Include invalid JSON in an error returned with command execution Feb 16, 2024
@@ -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)]
Copy link
Contributor Author

@ryo33 ryo33 Feb 16, 2024

Choose a reason for hiding this comment

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

This is a downside of using RawValue because it does not implement PartialEq.

from_value takes ownership of the Value, so we need to use RawValue to avoid cloning it here

Comment on lines +51 to 57
/// 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
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I turned the response_from_value into a generic helper function to use it here.

I don't believe this function must be as this, so I'm fine to revert the change or remove it.

@ryo33 ryo33 closed this Oct 25, 2024
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.

1 participant