Client Controlled Nullability: Error Handling

[twof]

Spec changes
To get a better idea of where we stand, I'll add a poll so people can vote on their favorite option. This vote is non-binding, and is only one factor in choosing which syntax we land on I'll open new threads for people to discuss each option below.

I'm going to try and separate null propagation and error handling discussion. In the original thread and during working group discussions, the conflation of the two has been causing some confusion about what options exist I think. For example, we could feasibly decide on any combination of (status quo null propagation/propagation to the nearest question mark) and (caught errors marked handled/caught errors put in a separate array/caught errors include destroyed data).

For some context on this discussion, some clients react to errors by throwing out the entire response. Since CCN is likely to create quite a few more errors than usual, and they are errors that the client is indicating it can handle, some commenters have felt that a means to handle errors needs to be introduced alongside CCN for it to be tenable for more sophisticated use cases.

Status Quo (No Error Handling)

Errors Caused By CCN Are Treated As Distinct From All Existing Errors

Query Syntax Is Introduced To Catch Errors

Update (March 6th, 2022): Folks at the working group meeting didn't feel like we'd explored the full breadth of the options for error handling, and wanted more time to further discuss.

[twof]

Error Handling Option 1

Status Quo

There is no way to catch errors. Errors produced by Required Designators (!) act the same as those produced by SDL Non-Nullable fields.

Example

Schema

type Business {
  name: String
  isStarred: Boolean
}

Query

{
    business {
        name
        isStarred!
    }
}

Response

{
    "errors": [
        {
            "message": "Cannot return null for non-nullable field Business.isStarred.",
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

[twof]

Error Handling Option 2

Errors caused by CCN are treated as distinct from all existing errors

When a client uses CCN, they are indicating that they are capable of handling the error case, so a CCN error may be treated as less serious than other existing errors, or treated as handled-by-default. We have a few options here.

What happens to handled errors

A) Exclude CCN errors from the errors array.
--- i) Include errors in the errors array if a flag called includeCaughtErrors is true. includeCaughtErrors is false by default.
B) Introduce a namespace field to all errors to allow clients to distinguish between them.
C) Introduce a boolean flag to all errors called isHandled which is true if an error is handled. All CCN errors will be isHandled: true.
D) Introduce a separate array of handledErrors where all handled errors are placed instead of the existing errors array.

Each of these has various pros and cons, which we can discuss further if this option is selected for development

Example

Schema

type Business {
  name: String
  isStarred: Boolean
}

Query

{
    business {
        name
        isStarred!
    }
}

Option A Response

{
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Option B Response

{
    "errors": [
        {
            "message": "Cannot return null for non-nullable field Business.isStarred.",
            "namespace": "client_controlled_nullabilty"
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Option C Response

{
    "errors": [
        {
            "message": "Cannot return null for non-nullable field Business.isStarred.",
            "isHandled": true
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Option D Response

{
    "handledErrors": [
         {
            "message": "Cannot return null for non-nullable field Business.isStarred."
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

[benjie]

I'm for option D. Critically I think you've made a mistake in the option D response format; it should not have the "errors" property at all. According to the GraphQL spec currently the errors list can never be empty - it either contains at least one error or it is not present at all. We should continue this property, a simple client (e.g. a bash script) can check if the errors entry exists, and if not it can continue - i.e. if all errors are handled.

I'm not sure the term handledErrors is correct, but that's a separate bit of bike-shedding we can leave for now 👍

[benjie]

@rivantsov I don't think using the extensions is wise; the point of the extensions field is that the spec itself doesn't dictate what's in there (and never specifies anything to be in there) so end users can add what they want into it without fear that it'll conflict with a future GraphQL spec update. It preserves the root level namespace for the GraphQL spec to control and still gives users somewhere to put their own error metadata.

[twof]

@benjie Thanks for pointing that out :). I've fixed the examples for this option and option 3.

[rivantsov]

However, using extensions deserves to be listed as one of the options

[twof]

@rivantsov if you'd like to do another write-up about how an extension option would work, go for it. I'll add it to the poll when you do.

[twof]

Error Handling Option 3

Query syntax is introduced to catch errors

Errors are caught by the nearest catching syntax. In the event that errors are not caught, they are treated as they would be in status quo ie they end up in the errors array.

What happens to handled errors

A) Exclude caught errors from the errors array.
--- i) Include errors in the errors array if a flag called includeCaughtErrors is true. includeCaughtErrors is false by default.
B) Introduce a boolean flag to all errors called isHandled which is true if an error is caught.
C) Introduce a separate array of handledErrors where all caught errors are placed instead of the existing errors array.
D) An additional path is included with errors indicating where errors were caught

There are two additional options associated with this approach.

E) The syntax catches CCN errors
F) The syntax catches all errors

Example

For the purposes of this example, error catching syntax will be @catch and for the sake of brevity we will only be depicting options C and E from above.

Schema

type Business {
  name: String
  isStarred: Boolean
}

Query 1

{
    business {
        name
        isStarred!
    }
}

Response 1

{
    "errors": [
        {
            "message": "Cannot return null for non-nullable field Business.isStarred.",
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Query 2

{
    business {
        name
        isStarred! @catch
    }
}

Response 2

{
    "handledErrors": [
         {
            "message": "Cannot return null for non-nullable field Business.isStarred."
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Query 3

{
    business @catch {
        name
        isStarred!
    }
}

Response 3

{
    "handledErrors": [
         {
            "message": "Cannot return null for non-nullable field Business.isStarred."
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

Query 4

{
    business {
        name @catch
        isStarred!
    }
}

Response 4

{
    "errors": [
        {
            "message": "Cannot return null for non-nullable field Business.isStarred.",
            // ...
        }
    ],
    "data": {
        "business": {
            "name": "Nobu",
            "isStarred": null
        }
    }
}

[rivantsov]

I suggest to change some terminology and naming. CCN -> "Null Assertion"

The error message you use in samples is "isStarred for business could not be fetched.". It sounds like server failed at something and reports it as a server-side error. I think this is a misrepresentation - these are not server errors or failures in any way.

Lets look at the following example. There are Employees, of two kinds - FullTime and Hourly; there are 2 nullable fields: Salary for FullTime and HourRate for Hourly employees. Again, salary field is Nullable.

We are at the page for FullTime employee details, so client sends a query:

query {
   employee(id: 123) {
     name
	 salary!
   }
}  

The client is requesting: "give me employee 123. I know she is FullTime, so salary should not be null. But just to be sure, please check it and let me know if it is null."

And the server responds: "here it is, your 123. Data retrieved OK, no errors. As for your your assumption about salary - it is wrong, salary field is null. You probably got wrong Id, or whatever."

The point is - there was NO Error on server side, it retrieved the data and returned it to the client.
What failed is an extra assertion that client asked server to do. '!' is a non-null ASSERT that client asks the server to run before it returns the data. Thus I think the wording of the message in your sample - "isStarred for business could not be fetched" - is misleading and confusing; it sounds like server failed at something - it didn't. The situation is more like "BadRequest" in Http terms, rather than 'ServerError' - the query is internally inconsistent between Id=123 and salary != null. More appropriate message from server would be something like 'Client assertion failed, 'salary' field value is null'.

As for how to return the message, I do not see a problem returning the message in errors list, just add some code/tag/marker identifying it as FailedAssertion.

The name of the feature - client controlled nullability (CCN) - is misleading too. Client controls nothing. Client asks the server to perform an extra assertion over a value before returning the response. How about 'Null Assertions' instead of CCN? - so '!' is a non-null assertion executed by the server.

Note 1: It is possible hypothetically that employee 123 is FullTime type but salary is null, which means data integrity error on the server - but this would/should manifest inside server code, differently and much earlier during data retrieval, and server would throw real error (server error) and return it with response - regardless if there's ! or not. But this has nothing to do with CCN.

Note 2: when talking about errors, use of terms/keywords: catch/caught errors, handledErrors, isHandled. These terms are quite confusing. Technically, all errors are caught and handled on the server. Otherwise, if server process does not handle an error, it bubbles up (as exception?), and if it is not caught it crashes the server process; and then your server is down, completely, dead. That's a conventional understanding of unhandled error in an app - the OS kills it. So all errors we're talking about here are Caught/Handled - since we have some intelligent response from the server. Thus 'isHandled' etc - sounds strange; all errors are handled some way or another

EDIT on Feb 12: after this comment was posted, the error messages in the original code samples in the preceding entries were edited and changed to 'Cannot return null for...'.

[twof]

The error message you use in samples is "isStarred for business could not be fetched.".

This was intended to be an arbitrary error message because we aren't discussing the content of the error yet, but I've updated the error message to the one being used in the GraphQL-js implementation. It's the same error message the user would receive if null was resolved for a Non-Nullable schema-defined field.

[twof]

Client asks the server to perform an extra assertion over a value before returning the response.

That sounds like the client is dictating how the server should handle nullability to me 🙃

In all seriousness though, quite a few people are already aware of the feature and know it by the name Client Controlled Nullability. I think changing the name now would cause enough confusion that it wouldn't be worth it unless the name was substantially impeding discussion.

[twof]

I do not see a problem returning the message in errors list, just add some code/tag/marker identifying it as FailedAssertion.

Would this would be a variation on option 2C?

[twof]

Technically, all errors are caught and handled on the server. 

I'm not sure for what tech stack or paradigm this is true, but it's not true for HTTP. Status codes in the 400 block are client errors, and status codes in the 500 block are server errors. These may be partially handled by the server, but the client is notified that an error occured so it can further handle it.

The same thing is happening here. The server may partially handle a CCN error (by propagating null for example), but the client is additionally notified that an error occured so it can adequately deal with the response. For each of these options, however, the client can decide (through syntax or something else depending on which we choose) that it does not need to further handle CCN errors, and so they can be separated from other errors (through a flag, exclusion, different placement, etc). This error is already handled as much as the client cares to handle it.

That said, I'm not super attached to the terminology, so feel free to suggest an alternative framing if you've got one in mind.

[rivantsov]

In all seriousness though, quite a few people are already aware of the feature and know it by the name Client Controlled Nullability. I think changing the name now would cause enough confusion that it wouldn't be worth it unless the name was substantially impeding discussion.

For now it is just a few people, but I am talking about future spec readers, thousands and thousands of them.
As the ancient guy said - "if you want to understand the thing - name it!". Proper naming/labeling things is extremely important for easier understanding and adoption. Currently, the feature is in WG-internal discussion loop. I am an outsider coming in, and giving you a feedback of a newbie, of how easy or not to grasp the concepts and understand what's this is all about. So this argument 'we have a few people here that already know it as CCN, so changing it would be confusing.' - this is a horrible argument. Who cares about a few people here?! Let's discuss the name and the concepts it identifies on its merits, keeping in mind thousands who will come to read the spec. I do not suggest you change the title of this thread immediately, but discussing a proper final name for a feature should be part of discussion here.

Again, conceptually CCN does NOT make sense at all, for a newbie or an old timer; client does NOT change anything on the server (like nullability of the field), so client controls nothing. It only humbly asks the server to run an assertion, a simple check on result. Even if this check fails, it does not signal any error on the server; it is just failed client's assumption.

[yaacovCR]

client does NOT change anything on the server (like nullability of the field), so client controls nothing. It only humbly asks the server to run an assertion, a simple check on result. Even if this check fails, it does not signal any error on the server; it is just failed client's assumption.

We must be coming at this from two very different angles. I think client controlled nullability best describes the proposed feature, I agree with you that the client is just instructing the server to run a check/assertion — but that’s all that non-nullability is, ie a field is not nullable if and only if the server performs that check.

For a non nullable field to be useful, it should usually pass that check, but that’s not what defines it as non-nullable. The existence of the check is. The existence of the check is what ensures that the value will never be null. Usually this is defined by the schema. CCN says it can be controlled by the client on a per operation basis.

[yaacovCR]

Mea culpa, I guess I partially see what you are saying though in terms of the name “error” for when the check fails, it’s more like an “exception.” It’s only for schema controlled nullability that we can say it’s an error when the check doesn’t pass because it’s presumably an underlying data layer error that has been detected, while when the client adds additional non-null checks, bubbling of the nulls is what the client asked for. That would for me be some variation of 2c as discussed above. So I guess I see where you are coming from in the end even if I think ccn is fine.

[yaacovCR]

Thought about this more and in a very general sense it can be thought of as an error => but not a server error, it’s more the client’s erroneous assumption.

More importantly, I think we shouldn’t initially take the word error too literally. It makes more sense to ask first whether we want to have the array of ccn exceptions to be mixed with non-ccn exceptions/errors or not. If we anyway don’t want them mixed, for example, we end up not having to worry about the nomenclature. If we do want them mixed (which I kind of doubt) then we probably also are in a universe where we don’t care about the exact terminology…

[lrowe]

I'm not sure I have enough understanding of GraphQL intricacies yet to know whether this is possible or not for scalar fields, but I think it would make most sense to treat object fields decorated with @catch as returning a union of the original field and the Error. This would work better for listings where one could render an error in place of the list item.

Modifying the example from option 3 this would look like:

Query 3

{
    business @catch {
        ... on Business {
            name
            isStarred!
        }
         ... on Error {
            message
        }
    }
}

Response 3

{
    "data": {
        "business": {
            "__typename": "Error",
            "message": "Cannot return null for non-nullable field Business.isStarred."
        }
    }
}

(Edit: updated to be explicit about using match syntax.)

[lrowe]

The use case I have in mind is for listings where you want the error boundary at the list item level so you can render the returned error message in place of a list item when an error occurs fetching data for a particular list item.

With Falcor this could be achieved by having UI code query each list item separately. Internally the Falcor client would batch/merge queries to the server (Falcor didn't support persisted queries.)

[rivantsov]

golang style handling?! nooooo!!!! will fight against it

[AnthonyMDev]

I also really love the idea of exposing CCN errors inline where they occur. While I recognize this creates a lot of additional complexities and would make a lot of work for existing GraphQL implementations, it could make it a lot easier for clients to understand if there is an error or the field is really just null. Which I think would help a lot with the issues being discussed here.

I don't expect this to actually happen, but i'd like to see it get a bit more consideration first.

Exposing it as an actual type like proposed here might be way too much of an overhaul, but there might be other ways to do this. The key to me is expressing a CCN error as something different than just a null. Yes, you don't have a value for the field, but that not because it's actually null. Its because you are catching the null assertion on it's child field. I'd like to be able to recognize the difference between those two situations inline at the site of the CCN error in the response.

[AnthonyMDev]

For example, we could introduce a new introspection field __fieldErrors that can only be selected on selection sets where the @catch directive (or whatever syntax we come up with for this) is used. This would allow us to expose field level errors (like CCN errors) inline.

{
    business @catch {
        __fieldErrors {
          message
        }
        name
        isStarred!        
    }
}

If __fieldErrors is present no other selected fields would be present.

{
    "data": {
        "business": {
            "__fieldErrors": [
                {
                   "message": "Cannot return null for non-nullable field Business.isStarred."
                }
            ]            
        }
    }
}

If we took an approach like this, I think it would be easier for the __fieldErrors selection to just be synthesized purely by the inclusion of the @catch directive. You probably don't need to explicitly include it.

---

I don't think this is the best syntax and could use improvement, but I do think it would be worthwhile to consider introducing a concept of field level errors being exposed inline somehow. And some form of a __fieldErrors introspection field seems like an viable possibility.

[benjie]

(If we were to do this, I'd strongly encourage that we forbid aliasing this field - if we didn't then clients who don't understand queries (only responses) wouldn't necessarily know this is what had happened. Arguably this also means that we should forbid __fieldErrors being used as an alias.)

[rivantsov]

(reposted from other thread)

Two things, about '?' as error boundary

1. If we introduce it, it makes sense to make it work for all errors. Something like: if this field or any child in selection subset fails, I do not need any data there, return null for this root field, and an error in Errors list. So we disconnect the '?' from the bang. The benefits are: a) - to understand the feature (?) you do not need any knowledge of other feature (!), makes it much easier. b)- clients can make use of it for any other errors or failures, not only for bang!. Here, I am not advocating pro- or contra- the qmark, but just that if we add it, let's add it for all errors.
2. What bothers me quite a bit: with '?', whether linked to bang or not, the response error object will have a path that is NOT present in the data returned (it refers to field inside the nullified subtree. That makes the response data internally inconsistent, kind of. As a solution: the path contains path to the null that was question marked as error boundary, and full path to the element is inside Extensions, keyed like fullErrorPath or something like that.

[twof]

One more option I've been thinking about that I want to throw out there to see if it has legs:

Provide server developers with more robust per-field error handling capabilities

We allow server developers to provide an error handling function for each field alongside a resolver. This way if an error is propagated from one field through multiple parents, each parent will be able to actually choose how it wishes to handle that error rather than leaving it for the nullability of each field to decide.

• The default if no error handling function is defined for a field would be the current behavior.
• Error handling functions can be inherited by a field's children ie a child didn't define its own error handling behavior, so it is free to use its parent.
• Error handling functions are composable ie in addition to the default behavior and my parent field's error handling behavior, I would like to do something else.

This will allow servers to choose how they'd like to react not just to CCN errors, but all errors. It will enable developers to do things like return a default value for a field in the event of an error, or decide that certain errors should not be returned to clients.

This would be a pretty substantial change, and is likely overkill for what we're trying to do here. It could be a proposal all on its own. Does it solve the problems folks are trying to solve here?

[twof]

Hello! It's been a while. The Client Controlled Nullability RFC is now winding down in favor of True Schema Nullability, so I'll be closing CCN related issues, PRs, and discussions. All further discussion should be rerouted to one of the following:
#1394
graphql/nullability-wg#39
#1410
graphql/graphql-spec#1065

Additionally, we're in the process of removing the CCN experimental flag from graphql-js.

TL;DR of the new proposal(s) is that instead of allowing clients to adapt to imprecise schema definitions, our new approach is to find ways to allow schema authors to provide the precision that's been missing. There are a few different approaches to that which you can read about in the discussions above. Apollo and a couple other schema authors are in the process of putting together experiments, so if you'd like to have input or beta test the new proposal(s), reach out to us either in the Nullability Working Group, or in the #nullability-wg channel on Discord.