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

[RFC] Error handling -- @catch & partial data #5337

Closed
martinbonnin opened this issue Oct 25, 2023 · 3 comments
Closed

[RFC] Error handling -- @catch & partial data #5337

martinbonnin opened this issue Oct 25, 2023 · 3 comments
Assignees

Comments

@martinbonnin
Copy link
Contributor

Description

For previous episodes on error handling, visit #4711

Goal

Simplify the error handling which is currently based on 3 different properties of ApolloResponse:

  1. response.data for GraphQL data
  2. response.errors for GraphQL errors
  3. response.exception for network exceptions

This is confusing and ideally, we want ApolloResponse to be a result-like class with either data or an exception. There is a lot of brain muscle around result-like classes. Kotlin stdlib has Result, Arrow has Either, Rust has Result, etc... So it feels like a natural fit here.

Proposal: @catch client directive

Inspired by Relay error handling, introduce a @catch client directive that allows user to opt-in error handling on a given field.

If this field or any sub-field contains an entry in the errors array then this error is exposed to the user and no data is shown for that field.

Advantages: data contains everything needed to display the UI. There's no need to go back to the errors array because the error is now inline at the position where it happens. We could keep the errors arrays for advanced use cases but in the very large majority of use cases, it shouldn't be needed. We can rely on data or exception again.

A @catch field is modeled in generated models as a sealed interface and matching result types:

Schema:

type Query {
  product: Product!
}

type Product {
  name: String!
  price: Float!
}

Query:

{
  product @catch {
    name
    price
  }
}

Kotlin codegen:

class Data(val product: ProductResult)

sealed interface ProductResult
class ProductSuccess(val product: Product): ProductResult
class ProductError(val error: Error): ProductResult

class Product(val name: String, val price: Double)

Examples

Product error

Query:

{
  product @catch {
    name
    price
  }
}

Error Response:

{
  "errors": [
    {
      "message": "Cannot resolve product",
      "path": ["product"]
    }
  ],
  "data": {
    "product": null
  }
}

Kotlin test:

val response = apolloClient.query(Query()).execute()
val product = response.data.product
check(product is ProductError)
check(product.error.path == listOf("product"))

price error

If a nested fails, the error is exposed on the closest enclosing @catch field:

{
  "errors": [
    {
      "message": "Cannot resolve product price",
      "path": ["product", "price"]
    }
  ],
  "data": {
    "product": {
      "name": "Champagne",
      "price": null
    }
  }
}

Kotlin test:

val response = apolloClient.query(Query()).execute()
val product = response.data.product
check(product is ProductError)
check(product.erro.path == listOf("product"))
@BoD
Copy link
Contributor

BoD commented Oct 26, 2023

I like @catch! It allows you to decide if/where you want to deal with partial results.

To be discussed: for apps that never want partial results, we could think of a global setting where data is a Result too, to avoid having to set @catch on all queries.

Tangential remark: if we feel errors vs exception contributes to the confusion, we could remove (well, deprecate first!) errors, let GraphQL errors only be surfaced in exception as ApolloGraphQLException?

@martinbonnin martinbonnin changed the title [RFC] Error handling -- partial data [RFC] Error handling -- @catch & partial data Nov 8, 2023
@martinbonnin
Copy link
Contributor Author

for apps that never want partial results we could think of a global setting where data is a Result too, to avoid having to set @catch on all queries.

This is the default. If you never want partial results, fields without @catch that fail also fail the whole query so you will have response.data == null and response.exception is ApolloException

if we feel errors vs exception contributes to the confusion, we could remove (well, deprecate first!) errors, let GraphQL errors only be surfaced in exception as ApolloGraphQLException

Is it time to introduce ApolloDelicateAPI? 😅

@martinbonnin
Copy link
Contributor Author

@catch is implemented in #5405 so I'm going to close this issue and centralize future discussions in #4711

A few additional comments:

for apps that never want partial results we could think of a global setting where data is a Result too, to avoid having to @catch on all queries.

With the (still subject to change but as of today) implementation, apps that opt-in "error aware parsing" by importing @catch will never receive partial results.

I've been thinking about changing the type of data but given data is val data: D in ApolloResponse, changing it is going to be quite the API compatibility challenge...

if we feel errors vs exception contributes to the confusion, we could remove (well, deprecate first!) errors, let GraphQL errors only be surfaced in exception as ApolloGraphQLException

As of today (still subject to change), I feel we won't be able to remove the v2/v3 mode where data contains the partial data and you do the lookup manually in errors. Might happen one day but I wouldn't be confident doing this today.

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

No branches or pull requests

2 participants