graphql-rust / juniper

GraphQL server library for Rust
Other
5.72k stars 425 forks source link

Return `Response<String>` instead of `Response<Body>` in hyper integration #1096

Closed LukasKalbertodt closed 2 years ago

LukasKalbertodt commented 2 years ago

Is your feature request related to a problem? Please describe. Sometimes it's useful to inspect the response returned by juniper_hyper::graphql. For example, in case of an error I would like to debug-log the whole JSON response. This is currently very tricky.

The function returns Response<Body> and the only way to read that Body is via the Stream trait (or the helper functions in hyper::body which also use that trait). But all those methods require Body by value or &mut Body. And in general that makes sense: Body can be many things and is not necessarily a byte buffer.

The best I could come up with to log the body is:

let (parts, body) = response.into_parts();
let body = hyper::body::to_bytes(body).await
    .expect("could not read API response body (this should never happen)");
debug!("Response body of failed API request: {}", String::from_utf8_lossy(&body));
response = hyper::Response::from_parts(parts, body.into());

I need to completely unpack the response to then assemble it again. And the await and expect are both not nice here.

Describe the solution you'd like From a quick glance at the juniper_hyper code, it seems like the Body is always created via From<String>. But String implements the trait hyper::HttpBody directly! So my suggestion is that all functions return Response<String>. With that, my logging would be as simple as:

debug!("Response body of failed API request: {}", response.body());
tyranron commented 2 years ago

@LukasKalbertodt makes sense! Would love to see a PR for this.