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

Failed HTTP response does not contain exception #6271

Closed
jiri-muller opened this issue Nov 22, 2024 · 7 comments
Closed

Failed HTTP response does not contain exception #6271

jiri-muller opened this issue Nov 22, 2024 · 7 comments
Labels
🐛 Type: Bug ⌛ Waiting for info More information is required

Comments

@jiri-muller
Copy link

Version

4.1.0

Summary

There is no exception in response when HTTP request fails (i.e 401).

Response in 4.1.0
image

Response in 4.0.1
image

Steps to reproduce the behavior

  1. Execute query to endpoint that returns HTTP 401
  2. Check ApolloResponse content - There is no exception, data or error

Logs

(Your logs here)
@martinbonnin
Copy link
Contributor

Thanks for sending this. This is due to the support for application/graphql-response+json (see https://github.com/apollographql/apollo-kotlin/blob/main/CHANGELOG.md#new-media-type-applicationgraphql-responsejson).

This is an unfortunate behaviour change, alas I haven't found a way to provide a better update path and could probably be considered a bugfix.

If you want to restore the throwing behaviour, you can use an ApolloInterceptor for that (I'll send some code in a bit) but in general, I would recommend parsing the errors to get the information you are looking for. It usually contains more information than the ApolloHttpException.

PS: you can read more details about the new content-type in the GraphQL over HTTP spec

@jiri-muller
Copy link
Author

@martinbonnin Thank you Martin! I was checking release notes but I totally missed this part. I will try to implement the interceptor.

@jiri-muller
Copy link
Author

Hmm looking at the it again I don't think ApolloInterceptor will help me. AFAIK it has access only to the same ApolloResponse that is missing the exception. Also I don't care about throwing behavior much I just need to identify failed requests and I don't think that checking that (data == null) will be enough.

I implemented okhttp3.Interceptor for underlying okHttp client and I can check the HTTP status there and if it's not 2xx I can throw exception that can caught in method calling execute().

I did not read whole GraphQL over HTTP spec specification but I would expect that if server is not returning application/graphql-response+json client should be able to handle potential error directly without interceptor in HTTP client...

@martinbonnin
Copy link
Contributor

martinbonnin commented Nov 22, 2024

I'd say the key is using executionContext[HttpInfo] to retrieve the status code:

          .addInterceptor(object : ApolloInterceptor {
            override fun <D : Operation.Data> intercept(
                request: ApolloRequest<D>,
                chain: ApolloInterceptorChain,
            ): Flow<ApolloResponse<D>> {
              return chain.proceed(request).onEach {
                val httpInfo = it.executionContext[HttpInfo]
                if (httpInfo != null && httpInfo.statusCode !in 200..299) {
                  throw ApolloHttpException(httpInfo.statusCode, httpInfo.headers, null, "HTTP request failed")
                }
              }
            }
          })

I just need to identify failed requests and I don't think that checking that (data == null) will be enough.

I'd recommend your server folks to include something in the GraphQL errors:

{
  "errors": [
    {
      "message": "AuthorizationFailed",
      "extensions": {
        "code": 401 // or something else, doesn't have to match HTTP codes.
      }
    }
  ]
}

That can be used to do the error handling at the GraphQL layer instead of peeking into HTTP (but peeking into HTTP should work too)

Hope this helps!

@martinbonnin martinbonnin added the ⌛ Waiting for info More information is required label Nov 22, 2024
@martinbonnin
Copy link
Contributor

@jiri-muller anything else we can help with here?

@martinbonnin
Copy link
Contributor

Closing for now, please leave a message if you want us to revisit

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug ⌛ Waiting for info More information is required
Projects
None yet
Development

No branches or pull requests

2 participants