Skip to content

Allow clients to disable error propagation via request parameter (take 2) #1163

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions spec/Section 3 -- Type System.md
Original file line number Diff line number Diff line change
Expand Up @@ -2167,3 +2167,27 @@ to the relevant IETF specification.
```graphql example
scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122")
```

### @behavior

```graphql
directive @behavior(onError: __ErrorBehavior! = PROPAGATE) on SCHEMA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on directive @onError(behavior: __ErrorBehavior!) on SCHEMA to mirror the naming of the client onError field?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s why I think it should be a request parameter and not a directive. (Further, I don’t think we should allow both because that opens up questions around what happens if the client sets one and the user sets the other: who wins?)

Copy link
Contributor

@fotoetienne fotoetienne May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about defining the server default onError behavior only in introspection and not in the SDL? We can instead define abstract "execution configuration" parameters that informs execution behavior. Implementations can then choose to have this be configured via a schema directive or as a separate configuration file.

```

The `@behavior` _built-in directive_ is used within the type system definition
language to indicate the _default error behavior_ of a GraphQL schema. It may be
omitted only if the _default error behavior_ is {"PROPAGATE"}.

Note: See [Handling Execution Errors](#sec-Handling-Execution-Errors) for more
information on _error behavior_.

<!-- https://github.com/prettier/prettier/issues/17286 -->
<!-- prettier-ignore -->
In this example, the schema indicates it is using the {"NO\_PROPAGATE"} _default
error behavior_:

```graphql example
schema @behavior(onError: NO_PROPAGATE) {
query: Query
}
```
12 changes: 12 additions & 0 deletions spec/Section 4 -- Introspection.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type __Schema {
mutationType: __Type
subscriptionType: __Type
directives: [__Directive!]!
defaultErrorBehavior: __ErrorBehavior!
}

type __Type {
Expand Down Expand Up @@ -164,6 +165,12 @@ enum __TypeKind {
NON_NULL
}

enum __ErrorBehavior {
NO_PROPAGATE
PROPAGATE
ABORT
Comment on lines +168 to +171
Copy link
Contributor

@martinbonnin martinbonnin Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving my previous comment here, love this proposal overall but I would love it even more with NULL

enum __ErrorBehavior {
  NULL
  PROPAGATE
  ABORT
}

tldr: I believe this is going to be read and discussed a lot more than it's going to be written by users and I would optimize for the conciseness, pronouncability and memorability of NULL.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To summarize my thoughts on it from the previous thread: NULL is what I originally wrote, I agree it looks and initially feels nicer; but ultimately I discounted it for a number of reasons:

Communication issues

If you tell someone on the phone to set onError: NULL, they're very likely to instead set onError: null.

Ambiguity issues

onError: null and onError: NULL are very similar, but the first means PROPAGATE and the latter means NO_PROPAGATE. This is likely to be the source of many user errors and much frustration (both for newbies, and for the people supporting them including library authors).

Errors already become null in the spec:

[an error] raised [...] is handled as though the response position [...] resolved to null
-- https://spec.graphql.org/draft/#sel-FANVNJCAACGW-qR

onError: NULL doesn't implicitly seem to mean something different from this.

Please suggest other alternatives!

I'm very open to changing the word NO_PROPAGATE to something else, I really don't like NO_PROPAGATE, not least because I always want to spell it propOgate rather than propAgate. But I think changing it to NULL would be a mistake.

I've gone through about 30 alternatives when brainstorming it, and none are sufficiently clear whilst also being shorter than NO_PROPAGATE. My favourite alternatives are things like LOCAL/LOCALIZE/ISOLATE/CONTAIN, but I'm not sure they convey the meaning quite as well as NO_PROPAGATE does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo... Maybe LOCAL_NULL actually is quite compelling. Not thought of that one before.

}

type __Field {
name: String!
description: String
Expand Down Expand Up @@ -238,6 +245,11 @@ Fields\:
must be included in this set.
- `directives` must return the set of all directives available within this
schema including all built-in directives.
- `defaultErrorBehavior` must return the _default error behavior_ of the schema
using one of the values of the `__ErrorBehavior` enum:
- {"NO_PROPAGATE"}
- {"PROPAGATE"}
- {"ABORT"}

### The \_\_Type Type

Expand Down
104 changes: 83 additions & 21 deletions spec/Section 6 -- Execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ A GraphQL service generates a response from a request via execution.

:: A _request_ for execution consists of a few pieces of information:

<!-- https://github.com/prettier/prettier/issues/17286 -->
<!-- prettier-ignore -->
- {schema}: The schema to use, typically solely provided by the GraphQL service.
- {document}: A {Document} which must contain GraphQL {OperationDefinition} and
may contain {FragmentDefinition}.
Expand All @@ -15,13 +17,23 @@ A GraphQL service generates a response from a request via execution.
being executed. Conceptually, an initial value represents the "universe" of
data available via a GraphQL Service. It is common for a GraphQL Service to
always use the same initial value for every request.
- {onError} (optional): The _error behavior_ to apply to the request; see
Copy link
Contributor

@fotoetienne fotoetienne Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about onError (or behavior) as a query directive rather than a new field on the request? Adding a field to the request payload might require changes to lots of GraphQL middleware. also a directive mirrors the behavior schema directive.

Copy link
Contributor

@martinbonnin martinbonnin Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we initially started with @disableErrorPropagation. It's very handy that no middleware is impacted at all but @benjie argumented that it should be a request parameter.

The argument that won me is that the directive would most likely be automatically added by the Apollo/Relay/... compilers. Then it means you're modifying the user query and if the user types the same query in GraphiQL it won't behave the same as what they initially typed, which is surprising.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point that you might want to be able to add it without modifying the query document

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also keep in mind that it being a GraphQL request parameter does not necessarily mean it’s an HTTP request parameter; for example you could use a different endpoint in HTTP to set the param: /graphql_nobubblesplz ;)

[Handling Execution Errors](#sec-Handling-Execution-Errors). If {onError} is
provided and its value is not one of {"PROPAGATE"}, {"NO_PROPAGATE"}, or
{"ABORT"}, then a _request error_ must be raised.
- {extensions} (optional): A map reserved for implementation-specific additional
information.

Given this information, the result of {ExecuteRequest(schema, document,
operationName, variableValues, initialValue)} produces the response, to be
formatted according to the Response section below.

Note: Previous versions of this specification did not define the {onError}
request attribute. Clients can detect support for {onError} by checking for the
presence of the `defaultErrorBehavior` field on the `__Schema` introspection
type. If this field is absent, clients should not include {onError} in the
request and must assume the _error behavior_ is {"PROPAGATE"}.

Implementations should not add additional properties to a _request_, which may
conflict with future editions of the GraphQL specification. Instead,
{extensions} provides a reserved location for implementation-specific additional
Expand Down Expand Up @@ -392,13 +404,24 @@ is explained in greater detail in the Field Collection section below.
</a>

If during {ExecuteSelectionSet()} a _response position_ with a non-null type
raises an _execution error_ then that error must propagate to the parent
response position (the entire selection set in the case of a field, or the
entire list in the case of a list position), either resolving to {null} if
allowed or being further propagated to a parent response position.

If this occurs, any sibling response positions which have not yet executed or
have not yet yielded a value may be cancelled to avoid unnecessary work.
raises an _execution error_, the error must be added to the {"errors"} list in
the _response_ and then handled according to the _error behavior_ of the
request:

<!-- https://github.com/prettier/prettier/issues/17286 -->
<!-- prettier-ignore -->
- {"NO\_PROPAGATE"}: The _response position_ must be set to {null}. (The client
is responsible for interpreting this {null} in conjunction with the {"errors"}
list to distinguish error results from intentional {null} values.)
- {"PROPAGATE"}: The _execution error_ must propagate to the parent _response
position_ (the entire selection set in the case of a field, or the entire list
in the case of a list position). The parent position resolves to {null} if
allowed, or else the error is further propagated to a parent response
position. Any sibling response positions that have not yet executed or have
not yet yielded a value may be cancelled to avoid unnecessary work.
- {"ABORT"}: The entire _request_ must be aborted. The {"data"} entry in the
_response_ must be {null}. Any _response position_ that has not yet executed
or has not yet yielded a value may be cancelled to avoid unnecessary work.

Note: See [Handling Execution Errors](#sec-Handling-Execution-Errors) for more
about this behavior.
Expand Down Expand Up @@ -823,28 +846,61 @@ MergeSelectionSets(fields):
</a>

An _execution error_ is an error raised during field execution, value resolution
or coercion, at a specific _response position_. While these errors must be
reported in the response, they are "handled" by producing partial {"data"} in
the _response_.
or coercion, at a specific _response position_. These errors must be added to
the {"errors"} list in the _response_, and are "handled" according to the _error
behavior_ of the request.

Note: An _execution error_ is distinct from a _request error_ which results in a
response with no {"data"}.

If a _response position_ resolves to {null} because of an execution error which
has already been added to the {"errors"} list in the response, the {"errors"}
list must not be further affected. That is, only one error should be added to
the errors list per _response position_.

:: The _error behavior_ of a request indicates how an _execution error_ is
handled. It may be specified using the optional {onError} attribute of the
_request_. If omitted, the _default error behavior_ of the schema applies. Valid
values for _error behavior_ are {"PROPAGATE"}, {"NO_PROPAGATE"} and {"ABORT"}.

:: The _default error behavior_ of a schema is implementation-defined. For
compatibility with existing clients, schemas should default to {"PROPAGATE"}
which reflects prior behavior. For new schemas, {"NO_PROPAGATE"} is recommended.
The default error behavior is indicated via the `defaultErrorBehavior` field of
the `__Schema` introspection type, or via the `@behavior` schema directive.

Note: {"ABORT"} is not recommended as the _default error behavior_ because it
prevents generating partial responses which may still contain useful data.

Regardless of error behavior, if a _response position_ with a non-null type
results in {null} due to the result of {ResolveFieldValue()} then an execution
error must be raised at that position as specified in {CompleteValue()}.

Note: This is distinct from a _request error_ which results in a response with
no data.
The _error behavior_ of a request applies to every _execution error_ raised
during execution. The following sections describe the behavior of each valid
value:

If an execution error is raised while resolving a field (either directly or
nested inside any lists), it is handled as though the _response position_ at
which the error occurred resolved to {null}, and the error must be added to the
{"errors"} list in the response.
**{"NO_PROPAGATE"}**

<!-- https://github.com/prettier/prettier/issues/17286 -->
<!-- prettier-ignore -->
With {"NO\_PROPAGATE"}, a `Non-Null` _response position_ will have the value
{null} if and only if an error occurred at that position.

Note: Clients must inspect the {"errors"} list and use the {"path"} of each
error result to distinguish between intentional {null} values and those
resulting from an _execution error_.

**{"PROPAGATE"}**

With {"PROPAGATE"}, a `Non-Null` _response position_ must not contain {null} in
the _response_.

If the result of resolving a _response position_ is {null} (either due to the
result of {ResolveFieldValue()} or because an execution error was raised), and
that position is of a `Non-Null` type, then an execution error is raised at that
position. The error must be added to the {"errors"} list in the response.

If a _response position_ resolves to {null} because of an execution error which
has already been added to the {"errors"} list in the response, the {"errors"}
list must not be further affected. That is, only one error should be added to
the errors list per _response position_.

Since `Non-Null` response positions cannot be {null}, execution errors are
propagated to be handled by the parent _response position_. If the parent
response position may be {null} then it resolves to {null}, otherwise if it is a
Expand All @@ -859,3 +915,9 @@ position_ must resolve to {null}. If the `List` type is also wrapped in a
If every _response position_ from the root of the request to the source of the
execution error has a `Non-Null` type, then the {"data"} entry in the response
should be {null}.

**{"ABORT"}**

With {"ABORT"}, execution must cease immediately when the first _execution
error_ is raised. That error must be added to the {"errors"} list, and {"data"}
must be {null}.