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

Initial support for @catch #5368

Closed
wants to merge 7 commits into from
Closed

Initial support for @catch #5368

wants to merge 7 commits into from

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

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

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.

2 participants