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

Is there a way of reading the Request and Response body more than once? #5007

Closed
marandaneto opened this issue Jun 5, 2023 · 8 comments
Closed

Comments

@marandaneto
Copy link

Question

When reading the Request and Response bodies more than once, it throws IllegalStateException (closed).
The reason is most likely because of MultipartReader.

I've tried using a HttpInterceptor, ApolloInterceptor from apollo3, and Interceptor from okhttp3.

Usually reading request/request.data, or request/request.body, returning a BufferedSource.

If the Sentry SDK reads the Request and Response bodies once, the final App won't be able to and this is a blocker.

The problem is that error monitoring tools such as Sentry.io would be able to leverage GraphQL errors if we are able to read the request and response bodies from the apollo-kotlin lib in order to give more context to those errors.

A bit more context getsentry/sentry#33723

This feature will be opt-in by default due to PII concerns.
Our backend service has data scrubbing capabilities as well.

Is there any workaround to make that work with the Apollo Kotlin library? Thanks a bunch.

@BoD
Copy link
Contributor

BoD commented Jun 5, 2023

Hi!

You can use BufferedSource.peek() to read it without consuming the bytes. For instance, here's an OkHttp interceptor:

class MyInterceptor : Interceptor {
    override fun intercept(chain: Interceptor.Chain): Response {
        val response = chain.proceed(chain.request())
        val responseStr = response.body?.source()?.peek()?.readUtf8()
        println("Intercepted response: $responseStr")
        return response
    }
}

To use it with Apollo Kotlin:

val apolloClient = ApolloClient.Builder()
    .serverUrl("https://...")
    .okHttpClient(
        OkHttpClient.Builder()
            .addInterceptor(MyInterceptor())
            .build()
    )
    .build()

Let us know if that works for you!

@martinbonnin
Copy link
Contributor

As an additional note, peek() works but it's also suboptimal. The Apollo runtime is trying quite hard to stream the HTTP responses to reduce latency when possible. Using peek() is risking buffering the whole response which can have some impact on large responses.

@marandaneto Is there anything else you need besides the operation document and the errors? If you only need that, you can use Interceptor and do logging at the GraphQL layer:

  val interceptor = object: ApolloInterceptor {
    override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
      return chain.proceed(request).onEach {response -> 
        println("GraphQL document: ${request.operation.document()}")
        println("Errors: ${response.errors}")
      }
    }
  }

@marandaneto
Copy link
Author

@BoD Oh indeed, missed that, thanks.

For the request, I believe it's also not a problem if I just:

For the request, I can just do request.body.writeTo(myOwnBuffer), I cannot reuse the myOwnBuffer but that's my problem only, the final app can still consume the request.body before it's closed, right?

@marandaneto
Copy link
Author

marandaneto commented Jun 5, 2023

@martinbonnin The problem with ApolloInterceptor is that it throws ApolloHttpException so in your example, the onEach never gets executed unless I missed something.

Edit:
I'm testing something that returns 400 with this response:

{
  "errors": [
    {
      "message": "Cannot query field \"mySite\" on type \"Launch\". Did you mean \"site\"?",
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED"
      }
    }
  ]
}

From https://github.com/apollographql/apollo-kotlin-tutorial/tree/main/compose/final

@martinbonnin
Copy link
Contributor

martinbonnin commented Jun 5, 2023

The problem with ApolloInterceptor is that it throws ApolloHttpException so in your example, the onEach never gets executed unless I missed something.

You can catch that:

  val interceptor = object : ApolloInterceptor {
    override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
      return chain.proceed(request)
          .catch {
            println("Oh no, network error: $it")
            throw it
          }.onEach { response ->
            println("GraphQL document: ${request.operation.document()}")
            println("GraphQL errors: ${response.errors}")
          }
    }
  }

Edit: in order to read the error body, you'll need exposeHttpErrorBody. So it's definitely more involved but it can save the buffering of the full HTTP response in the non-error case

Edit2: also v4 is changing this to avoid throwing when exceptions happen (#4711)

@marandaneto
Copy link
Author

Thanks @martinbonnin and @BoD , let me try this out.

@marandaneto
Copy link
Author

Alright, looks like this would do:

val apolloClient = ApolloClient.Builder()
    .serverUrl("https://apollo-fullstack-tutorial.herokuapp.com/graphql")
    .webSocketServerUrl("wss://apollo-fullstack-tutorial.herokuapp.com/graphql")
    .okHttpClient(
        OkHttpClient.Builder()
            .addInterceptor(AuthorizationInterceptor())
            .build()
    )
    .webSocketReopenWhen { throwable, attempt ->
        Log.d("Apollo", "WebSocket got disconnected, reopening after a delay", throwable)
        delay(attempt * 1000)
        true
    }
    .httpExposeErrorBody(true)
     // add sentry interceptors    
    .sentryTracing()
    .build()
class TestApolloInterceptor : ApolloInterceptor {
    override fun <D : Operation.Data> intercept(
        request: ApolloRequest<D>,
        chain: ApolloInterceptorChain
    ): Flow<ApolloResponse<D>> {
        return chain.proceed(request)
            .catch {
                val body = (it as? ApolloHttpException)?.body
                // I have to use the peek to read but not to consume the bytes
                println("Body: ${body?.peek()?.readUtf8()}")
                throw it
            }
            .onEach { response ->
                println("GraphQL document: ${request.operation.document()}")
                println("Errors: ${response.errors}")
            }
    }
}

@BoD @martinbonnin a last question, would be possible to access the raw body of the request and response as well? similarly to (it as? ApolloHttpException)?.body but when it does not throw ApolloHttpException.

I will close the issue for now since I can also parse the request.operation.document() and errors on my own anyway, but the raw access would be desirable as well.

Thanks for the great work & support.

@martinbonnin
Copy link
Contributor

the raw access would be desirable as well.

At the end of the day, I'd say it all depends your requirements. If you need access to the raw request/response bytes, then HttpInterceptor might be a better fit. All in all:

  • use ApolloInterceptor if your code needs the GraphQL operation string and GraphQL errors or if you need to log cache hits/misses.
  • use HttpInterceptor if your code needs access to the raw network bytes and doesn't need to log cache misses
    • if you need to buffer all the response/request then use .peek() as @BoD suggestion above with the caveat that this allocates all the response in RAM.
    • if you can "stream" the response, wrap response.body into another BufferedSource where you consume the bytes as they come in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants