https://kotlinlang.org logo
Join Slack
Powered by
Hey folks. The topic of partial responses has come...
# apollo-kotlin
s
Hey folks. The topic of partial responses has come up internally, and I was trying to find good sources on what tradeoffs are typically included when making decisions around this. It's the common problem of not wanting to mark fields as nullable, since backend considers those to always be resolvable, unless there is a 500 error or something else out of the ordinary. And the clients being unhappy when a big query for a big screen completely errors out because one tiny thing somewhere failed to result and the error gets bubbled up to the entire query. Splitting up the queries client-side into granular ones that can individually fail would definitely work, but I feel like we'd be misusing the tools we have on our hand, not to mention the multiplication of the number of network requests we'd do, which would be a negative of course. So, do y'all mark a lot of things nullable only to let errors bubble up to them?
The thing is, there is a client-side directive to potentially turn nullable fields to nonnull, but there isn't the opposite, so in a way I feel like GQL is nudging us to lean more towards making more rhings nullable, but it would be very hard to work with if everything was null too 😅 But yeah, I think I'm definitely not the first one to ask this question, if there are any good links/blogposts/talks or whatever that discuss this trade-off I would be super happy to read them. And of course, your personal experiences are valuable as input too!
m
Yea, it's a pain. Graphql.org still recommends nullable by default. It was probably good in 2014 feels wrong in 2024
Apollo Kotlin and relay both support 
@semanticNonNull
you can read more about it there: https://www.apollographql.com/docs/kotlin/advanced/nullability
If you want all the details, the notes from the GraphQL nullability working group are available there
The thing is, there is a client-side directive to potentially turn nullable fields to nonnull, but there isn't the opposite
There is 
@semanticNonNull
to turn a field that is null only for errors into a non-null Kotlin properties. Note that ideally it's applied server-side so that it can be shared cross teams (the backend team knows whether a field is null only for errors) There is kind of the opposite with 
@catch
which allows you to decide field by field what to do with errors: conflate them to 
null
again, throw the whole query or expose a 
FieldResult
class
s
Thanks for all of this, I am definitely a bit more up-to-date with all the latest happenings on this now! So let me try to write some things and you can tell me if I have understood this correctly or not. For a field like:
Copy code
type User {
  id: ID!
  # name is never null unless there is an error
  name: String @semanticNonNull
  # avatarUrl may be null even if there is no error. In that case the UI should be prepared to display a placeholder. 
  avatarUrl: String
}
What apollo-kotlin will then generate is
Copy code
data class User(
  val id: String,
  val name: String,
  val avatarUrl: String?,
)
Since we know that 
name
should be null in all normal scenarios. So far so good, on the client nothing has changed as far as the generated code goes. But there is semantic meaning now in the schema to what null may mean for that field. --- Here comes the part I am unsure about Then to make use of this new information on the client, one would need to mark it with 
@catch
on the operation level now, not on the schema, on a case-by-case basis. And there we can decide on the three options as described in the docs: • handle errors as 
FieldResult<T>
, getting access to the colocated error. • throw the error and let another parent field handle it or bubble up to 
data == null
. • coerce the error to 
null
(current GraphQL default). If we do nothing (aka do not add the 
@catch
, we will fall-back to just the third option, which is to turn it to null, and since the code-gen has generated that as non-null, the error will bubble up the exact way it was doing before too. And for scenarios where nothing is nullable, it will, just like before, bubble all the way to the root of the response, making all of it null and the errors response will be populated with the same errors there. OR Is it that by default now it will mark those fields as nullable in Kotlin, which would be a change in behavior? As I understand the three options now, I thought that the second option, aka 
@catch(to: THROW)
would be the behavior that preserves the previous behavior.
--- Actually you know what, I think I am mixing up the two a bit, because I am thinking of this in the context where our fields are right now marked as 
Foo!
already, and I would like them to perhaps change to in the future be 
@semanticNotNull Foo
. Would this be a migration which can be done in a way which will not break older clients? As I understand, if the schema does a 
Foo!
-> 
Foo
change, that is a breaking change in a sense, but if it does a 
Foo!
-> 
@semanticNotNull Foo
change this will "change" the schema itself, but practically they would mean the same thing right? And if we combine that with having a default of 
@catch(to: THROW)
that would preserve the same functionality as before doing this change too. Does what I say make sense here?
So, if we were to do a migration (at least for some fields initially, to get the ball rolling) we would do this: Change some type in the schema itself, so that one of its fields will for example go from 
String!
to 
@semanticNotNull String
• Existing clients who were built with the old schema before this change: This will mean that now the schema we built with was expecting that to be not-null, so when it is null, the parsing will fail, and I hope this will the surface in a way where the null will bubble up, and the errors will be populated. So the old clients will interpret that as a complete failure, and the "bad path" will be taken, just it does today when backend gets some internal exception and returns null anyway. • On new clients who know about @semanticNotNull: We will now be able to see the @semanticNotNull, the type will still be generated as non-null, and if we do nothing and we do not annotate with 
@catch
, the error will bubble up the same way upwards, and we will get the same behavior as I describe above • On new clients who know about this and have opted into using 
@catch
in specific operations: We will then be able to be more smart about this, even if that means using the 
FieldResult
or a normal Kotlin nullable type. If we don't opt-in, nothing should change from the old behavior. Does this sound correct, or am I missing something? 😄
m
go from 
String!
to 
@semanticNotNull String
That's typically not what you would do 
String!
is "stronger" than 
@semanticNotNull String
Usually you would go from 
String
to 
@semanticNotNull String
s
Yes, that I do understand, I can try to explain why I am considering this in the first place. There are specific types, with say 10+ separate fields, all of which are resolved independently and should absolutely be able to fail individually, and the client would work perfectly fine to receive that partial data and render only what we have received there. Right now all of them are 
!
and our only option to be able to break this query down in a way where partial data would work is to split it in 10 separate queries, so that they can fail individually and we can stitch back the data class from all those separate queries. So the problem is that we've eagerly set a lot of things as 
!
now already in the schema, and that limits our ability to ever have partial responses. And if 1 thing goes bad, entire screens completely fail because they could not render a nice pretty thingy on the side that is completely optional for that screen. Does this use case make sense?
m
Oh I see 👍
That makes perfect sense. Most teams use JS/python frameworks which are "nullable by default" but looks like your backend team is the opposite and now they want to introduce the possibility of failure
s
Yeah our backend team is full Kotlin folks so they are familiar with the pains of nullability and we've historically been very aggressive with marking things as not-null when the business logic says that "if nothing goes hay-wire, this must exist". This has historically worked super well for us, and working with these types is super nice on the client, all the way until we wanted to now support partial responses 😄
m
Conflating errors and nullability was a mistake...
Existing clients who were built with the old schema before this change
Those will see 
response.exception != null
and should fail the whole query
s
> Those will see 
response.exception != null
and should fail the whole query That would be perfect then! As this is what those clients experience today anyway with our current setup!
m
Yep, that should work
> On new clients who know about this and have opted into using 
@catch
in specific operations: Note that you have to opt-in a default 
@catchByDefault(to: CatchTo)
Typically, you would use 
@catchByDefault(to: NULL)
But you can use something else if you really want to
s
@catchByDefault(to: CATCH)
That'd be 
extend schema @catchByDefault(to: THROW)
you mean right?
m
You have to chose 
NULL
, 
THROW
, 
RESULT
at the schema level when opting-in error aware parsing
The "compatible" way is 
extend schema @catchByDefault(to: NULL)
but I think I like 
extend schema @catchByDefault(to: THROW)
better
This can be overriden field by field
s
Yeah since we don't really have 
null
things like that, throwing feels like a sane default preserving the old behavior for us. And we can then take it on a case by case basis if some do not want this behavior.
m
Right, forgot you were going the "reverse" route
s
Haha sorry for making this even more confusing than it already is 😂
😂 1
m
If all you do is 
String! => @semanticNonNull
, 
extend schema @catchByDefault(to: THROW)
is the "compatible" option I guess
Yea, who knew nullability can be so confusing
😅 1
Last thing to add to the confusion 😅: if your backend team decides to go 
String! => @semanticNonNull String
(which is cool!), your clients will bump into https://github.com/graphql/graphql-spec/issues/300 as 
@semanticNonNull
is lost in introspection 😐 so you'll need to find a way to ditribute your schema that is not relying on introspection
s
Nooo 😂
m
It's all doable. Most schema registries should have that option but still something to keep in mind
What framework is your backend using? I'd love to help improve the tooling there
s
And for one last potential blocker. We'd also have to rely on all of our fontend/backend services are running things that are able to read @semanticNotNull in the first place right? Do all of the apollo provided ones have support for it at this point? https://github.com/apollographql/apollo-client https://github.com/apollographql/apollo-ios I imagine https://github.com/graphql-java/graphql-java is the one that I feel is the least likely to have support for such experimental stuff though
m
No support in 
apollo-client
nor 
apollo-ios
yet
s
Yeah backend uses https://github.com/spring-projects/spring-graphql (which internally uses https://github.com/graphql-java/graphql-java afaik). But okay as I understand this is quite early in the pipeline then if the other two apollo clients don't have support for it. This would more or less be a total blocker then until all those three might support this directive right? Otherwise we would need to somehow be getting a different version of the schema compared to the web and iOS which would still expect all of those to be 
!
instead of nullable with this directive
👍 1
m
It's a bit of a chicken and egg problem right now. The spec needs to change but before commiting a change to all GraphQL users, it needs to be veted against real life use cases, make sure it scales, etc... So it needs feedback
graphql-java
At some point I think GraphQL java added 
AppliedDirective
to introspection which would fix #300. I'm not 100% sure though, would need to look that up
a different version of the schema compared to the web and iOS which would still expect all of those to be 
!
instead of nullable with this directive
Yup, I guess a runtime switch on the server would be nice. This is amongst the discussed options right now
s
Yeah I totally understand. Very interesting, so I think for today, I can't really do anything about this then besides split up the query in separate queries to handle "partial" data manually. Even if I were to patch the schema client-side that would not make any difference, since the backend will never in fact return a proper partial response there, since for it, the field is non-null, and if that fails everything fails regardless. And changing it to nullable without this directive would be something that would require a very strong reason to do, especially since it would massively degrade the experience of iOS and web as-is.
nod 1
m
What would be really cool for your use case is client + server-side support for 
@noBubblesPlease
:
Copy code
query GetUser @noBubblesPlease {
  user {
    name
    email
  }
}
Copy code
type User {
  name: String!
  email: String!
}
If email fails, a 
@noBubblesPlease
-compliant server would reply with
Copy code
{
  "data": "user": { "name": "foo", "email": null },
  "errors": [{"message": "oops", location: ["user", "email"]}]
}
👍 1
s
How would this surface in the code generated by apollo-kotlin? If 
email
throws, it would have to be null, and name would have to be present. But both would need to be nullable on the code generated to support either of the two cases where either might throw internally right?
m
I'm pretty confident we could support that in Apollo Kotlin with 
@catch
👍 1
s
Right, so the choice would still be on a per-operation case, and it'd be opt-in as well
m
Yup, which I think is fine? The dev writing the operation knows whether they want to handle that error or just fail the whole query?
It's like a Kotlin try/catch
s
Yeah, I am saying that as a positive thing 😄 Since that way doing nothing just gives you the existing behavior of everything burning down, and if you know you are ok with nulls there, you can do that with a 
@catch
👍 1
m
We'd "just" need support in graphql-java which is probably doable. I think Hot Chocolate has it, it wasn't too hard to implement
s
There is such potential for a nice future, free of conflation of errors and nulls. Alas that future is not in 2024 it seems 😅
😅 1
As I understand, and forgive me if I am wrong here, graphql-java leans a bit more on the "conservative" side when it comes to supporting such more experimental things. Is that the case? Because if yes perhaps I should not hold my breath at least 😄
m
I'm not sure I think they've done experimental stuff in the past
Let me check that AppliedDirective thing
blob hug 1
I'll ask for a branch or experimental release. graphql-js has done a bunch for things like 
@defer
, 
oneOf
, etc... It's quite important for field feedback
s
Yeah, otherwise the chicken and the egg problem only becomes stronger if nobody can practically use it to even start giving feedback in the first place.
💯 1
m
s
Amazing, thanks so much! Is your hope that such a directive would be faster to get any sort of experimental support compared to something which may be more involved like @semanticNotNull? Since you do not mention anything around 
@catch
in this context, won't the first question be how should clients handle this if they do not know how to change their generated code to account for this directive in the first place? Or is the hope that if such an experimental directive exists in the backend, the clients can then more easily build the necessary tooling required to make it work? And just so it happens that apollo-kotlin is in a perfect spot to support it, since it already supports 
@semanticNotNull
?
m
Is your hope that such a directive would be faster to get any sort of experimental support compared to something which may be more involved like @semanticNotNull?
Yes 🙂 It's much simpler
Since you do not mention anything around 
@catch
in this context, won't the first question be how should clients handle this
Maybe? Let's see. The example shows that 
user.name
is returned with 
@noBubbling
. How the client handle this felt irrelevant for that specific backend issue but I'll add more context if needed.
if such an experimental directive exists in the backend, the clients can then more easily build the necessary tooling required to make it work?
Exactly. This is what I hope with this. It doesn't add a type, it "just" changes the execution logic a tiny bit
Thanks for providing a nice use case!
s
Amazing, this feels really good then. I love it how you managed to take my use case + a lot of blabbering and turn it into a nice issue to be discussed there! Let's hope people find it interesting and chime in with their thoughts.
m
🤞
Gave it a shot at 
@errorHandling(onError: NULL)
(previously known as 
@noBubbling
) there: https://github.com/martinbonnin/graphql-java/?tab=readme-ov-file#graphql-java-fork
🌟 1
🚀 1
s
Is this adding the directive itself, or also making it work as we described here? I tried to go through the code a bit but I saw it mostly being things to define the directive itself right? I am afk though right now and I might be missing something. Btw I like the naming of this, noBubbling is fun but this sounds more like what the real directive would be like
m
It’s also doing the work. It’s all happening here
(the logic is a single if in the end 😄 )
Re: naming, that’s exactly the idea. It might not be the final name but it’ll save an iteration hopefully
s
Wow well, I absolutely missed it then. I figured it must be at least more than one line, so I skipped over those one liner changes 😅 Will you link this in the issue that you made which proposed the bubbling directive? As an idea of how to implement this. Showing how simple it is might be a positive thing in general.
m
Yup, good idea!
8 Views
Open in Slack
PreviousNext
Powered by