Regression after upgrading from 4.0.0-beta.2 to 4.0.0-beta.3

[leinardi]

Version

4.0.0-beta.4 (the issue started with 4.0.0-beta.3)

Summary

After upgrading from 4.0.0-beta.2 our unit tests are failing because the apolloClient.enqueueTestResponse() doesn't seem to work the same as before: when we enqueue an error the exception property of the response is now null when, up to version 4.0.0-beta.2, it was set to ApolloGraphQLException.

Is this change intentional?

Steps to reproduce the behavior

We are using the apolloClient.enqueueTestResponse() like this:

    const val API_RESPONSE_ERROR_MESSAGE = "apolloApiError"
    val mockApiError: Error = Error.Builder(API_RESPONSE_ERROR_MESSAGE).build()
    val mockListApiError: List<Error> = listOf(mockApiError)

    fun enqueueApolloErrorResponse() {
        val operation = AddFollowedExpertMutation("1")
        apolloClient.enqueueTestResponse(
            operation = operation,
            data = null,
            errors = mockListApiError,
        )
    }

    @Test
    fun someTest(): TestResult = runTest {
        // Given
        enqueueApolloErrorResponse()

        // When
        val response: Result<Any?, AuthCallError> = getTimelineInteractor("expertId")

        // Then
        val error = response.unwrapError() as AuthCallError.ApiError.Generic
        assertEquals(API_RESPONSE_ERROR_MESSAGE, error.errorMessages?.first())
    }
}

Logs

No response

[martinbonnin]

Hi 👋

This is intentional. 4.0.0-beta.2 would set exception != null on GraphQL errors but this would break a bunch of use cases about partial data and more generally breaks the promise of "if a GraphQL response is received, exception is null".

Error handling in GraphQL is not easy but latest versions of 4.0.0 betas add support for nullability directives, which I hope can clarify the situation.

Especially, if you want GraphQL errors to set exception, you can opt-in error aware handling and default @catch(to: THROW):

# Import the `@catch` directive definition. 
# When it is imported, parsers generate extra code to deal with errors.
extend schema @link(
  url: "https://specs.apollo.dev/nullability/v0.1", 
  import: ["@catch", "CatchTo"]
)

# Errors stop the parsing. The data never contains error. Use `@catch` in operations to let partial data through
extend schema @catch(to: THROW)

Hope this helps, let us know how that goes!

PS: another good read about error handling is here

[leinardi]

Hi @martinbonnin 👋

Thank you for the quick response, if this is the intended behavior, I'll close the issue and update our error handling logic to take into account a null exception 👍

[martinbonnin]

👍 By default, I recommend handling errors like this:

if (response.data != null) {
  // Handle (potentially partial) data
} else {
  // Something wrong happened
  if (response.exception != null) {
    // Handle non-GraphQL errors
  } else {
    // Handle GraphQL errors in response.errors
  }
}

This default exposes partial data which may make writing UI logic more cumbersome.

@catch(to: THROW) changes the default to no-partial data (and you can opt-in partial data again with @catch(to: RESULT))

[leinardi]

One more question: do you know if it could still happen that an instance of ApolloGraphQLException will be present inside the response.exception or if, with the default options, it will always be either null or an instance of something different from ApolloGraphQLException.

I just want to know if I need to do a check like this:

response.exception == null && response.hasErrors() || response.exception is ApolloGraphQLException -> Err(AuthCallError.ApiError.Generic(last.errors?.map { it.message }))

or if I can skip the || response.exception is ApolloGraphQLException part.

[martinbonnin]

ApolloGraphQLException is now only thrown from the parsers if you opt-in error aware parsing or if you're using dataAssertNoErrors. If you're not using any of those, you can safely ignore this exception. (which is another reason to the behaviour change IMO, it makes code like this much nicer)

PS: you can also drop the response.exception == null part. If response.hasErrors() is true, a GraphQL response has been received and response.exception is null.