Skip to content

Conversation

@martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Nov 14, 2023

See #5337

This PR adds support for 2 directives:

@nullOnlyOnError on field definitions:

"""
The given field can only be null if there is a matching error in the 'errors' array.
Fields of this type will be generated as non-null Kotlin types.

level: the level in the list dimensions where to apply the modifier or null to apply it on all dimensions.
"""
directive @nullOnlyOnError(level: Int = null) on FIELD_DEFINITION

@catch on fields:

"""
Catches error occuring on a field. If this field or a sub field cannot be parsed, recover and expose the 
error instead of failing the whole query.

This field will be generated as a `com.apollographql.apollo3.Result` type.

level: the level in the list dimensions where to apply the modifier or null to apply it on all dimensions.
"""
directive @catch(level: Int = null) on FIELD

With those, you can extend the schema on the client side:

type User {
  # name is nullable in the schema but it's only null on errors
  name: String @nullOnlyOnError
}

And decide to handle the partial data gracefully:

query GetUserPartial {
  # if name is ever null, user will be an instance of `Result.Error`
  user @catch {
    name
  }
}

Or just throw the whole query if no @catch is present:

query GetUserOrThrow {
  # if name is ever null, it will fail to parse and throw the whole query
  user {
    name
  }
}

This PR also changes the behaviour of ApolloResponse.exception to not throw if there are GraphQL errors. So now data and exception are exclusive.

Todo/questions:

  • naming: is Result/Error good enough or is the risk of name clash too big?
  • robustness-mode: do we need a mode to ignore the @nullOnlyOnError annotation completely and expose the raw server response?
  • validation is still TBD
  • feature flag? we probably want to make this opt-in until we have more confidence

@martinbonnin martinbonnin requested a review from BoD as a code owner November 14, 2023 11:50
@netlify
Copy link

netlify bot commented Nov 14, 2023

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit c22b9f1
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/6553a8cb91a20700089d451f

@martinbonnin martinbonnin marked this pull request as draft November 14, 2023 11:56
@BoD
Copy link
Contributor

BoD commented Nov 14, 2023

👍👍👍 🎉

About the name clash: I'd find it nice to name it ApolloResult or something, since there's already a Result in kotlin, and probably also in a lot of projects. Thanks to package names it's a not a huge issue but could be a mildly annoying or confusing.

@martinbonnin
Copy link
Contributor Author

About the name clash: I'd find it nice to name it ApolloResult or something, since there's already a Result in kotlin, and probably also in a lot of projects. Thanks to package names it's a not a huge issue but could be a mildly annoying or confusing.

Agreed. Only issue is we're not really consistent as we have non-prefixed Error and Optional already. And Result felt closer to Optional than ApolloResponse so I went with the consistency choice but not 100% convinced by it. I'm curious if there are any other thoughts about it.

@martinbonnin
Copy link
Contributor Author

Closing in favour of #5405
I ended up going for FieldResult instead of just Result

@martinbonnin martinbonnin deleted the catch branch December 13, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants