Cannot retrieve GraphQLResponses with ErrorExtensions properly

[bengsparks]

The traits for extending the request builders of reqwest, surf, etc. all appear like this:

pub trait ReqwestExt {
    fn run_graphql<ResponseData, Vars>(
            self,
            operation: Operation<ResponseData, Vars>,
        ) -> BoxFuture<'static, Result<GraphQlResponse<ResponseData>, CynicReqwestError>>
        where
            Vars: serde::Serialize,
            ResponseData: serde::de::DeserializeOwned + 'static,
}

Additionally, the definition of a GraphQLResponse looks like this:

pub struct GraphQlResponse<T, ErrorExtensions = serde::de::IgnoredAny> {
    ... 
}

In the first code example, there is no way to specialize the otherwise defaulted ErrorExtensions generic argument of GraphQlResponse.

This implies that attempting to deserialize responses from GraphQL servers, triggered by requests made via these traits, will lead to these extensions being dropped, as per the docs of serde::de::IgnoredAny.
Likely extending the signatures of run_graphql would be sufficient to allow for the additional extension type

I have hacked together an MRE here that demonstrates the issue and provides a new trait to facilitate the desired behaviour with reqwest. However, it would mean a breaking change, and the API isn't as nice as I would have liked. Nonetheless, I hope it helps :)

[obmarg]

Thanks for the report @bengsparks.

This is a tricky one to solve nicely - I could add another generic param to run_graphql but forcing everyone to deal with that for this relatively niche use case isn't ideal. A separate function for users who want this would also be possible but doesn't seem great either...

I'll have a think - in the mean time I would recommend just using your own reqwest etc. integration.

[bengsparks]

One suggestion I have is to implement a form of delayed evaluation with a builder-esque structure?
Something that would allow for

let response = client
    // Prepare to send HTTP request to already running GraphQL server
    .get(format!("http://{}/graphql", graphql.host_with_port()))
    // Send request with `run_graphql`, WHICH DOES NOT DESERIALIZE YET!
    // Instead, store the response as e.g. `reqwest::Error<reqwest::Response>` inside of a 
    // DelayedResponse<ResponseData>, which is inferrable from the provided arguments.
    .run_graphql(FieldWithString::build(FieldWithStringVariables {
        input: "InputGoesHere",
    }))
    // Perform JSON deserialisation here upon `DelayedResponse<ResponseData>::retain_extensions<Extensions>`,
    // which contains sufficient type information to deserialise via 
    // `serde_json::from_str::<GraphQlResponse<ResponseData, Extensions>>`
    .retain_extensions::<Extension>();

But ultimately all it does it plug the fact that the *Ext::run_graphql trait methods have a hole in their APIs :/.
Nonetheless, thought I should pass this small idea on, maybe it can be used for something better.

[obmarg]

Ooh, actually that is exactly the right answer, good thinking.

run_graphql should return a builder as you suggest, and then it can impl IntoFuture so that users can still just do run_graphql(...).await if they don't need to call retain_extensions.

I don't going to have time to implement this immediately but that sounds like the way to go.

[bengsparks]

Glad to hear my suggestion could be of use!

The only reason I was hesitant to propose this is because of blocking reqwest, which is likely used in non-async environments, and therefore cannot be awaited upon.

But perhaps there is a cool solution here too. Or maybe it will have to be the odd one out and require explicit usage. I imagine the majority of cynic's usages are called from async contexts anyhow :)