Error propagation

[Fanneyyy]

We're working on moving from Juniper to async-graphql and we noticed some issues we're having with error propagation that we didn't have in Juniper.

The issue is that Juniper handles errors so if an error is returned from a non-null field, the null value is propagated up to the first nullable parent field, or the root data object if there are no nullable fields. E.g. the juniper code below still returns data from the me query, even though the produce_error field returns an error because some_obj is nullable.

The same doesn't seem to work for us with async-graphql where the whole query always returns an error if any field does. We can't find any examples in the book or docs so wondering if there's something obvious we're doing wrong?

Basically we're looking for a way for nested fields to return errors without affecting the whole query, is there a good way to do that?

Also, could that solution include a way to return all errors from multiple fields?

Any help would be amazing 🙏

Example from Juniper

Code:

struct SomeObj {}

#[juniper::object()]
impl SomeObj {
    async fn produce_error(&self) -> Result<i32, juniper::FieldError> {
        Err(juniper::FieldError::new(
            "Error",
            graphql_value!({
                "type": "SomeError"
            }),
        ))
    }
}

#[juniper::object(Context = Context)]
impl Query {
    fn some_obj() -> Option<SomeObj> {
        Some(SomeObj {})
    }

    fn me(context: &Context) -> Result<Option<User>, juniper::FieldError> {
        get_me(context)
    }
}

Query:

query MyQuery {
  someObj {
    produceError
  }
  me {
    name
  }
}

Response:

{
  "data": {
    "someObj": null,
    "me": {
      "name": "Some name"
    }
  },
  "errors": [
    {
      "message": "Error",
      "locations": [
        {
          "line": 3,
          "column": 5
        }
      ],
      "path": [
        "someObj",
        "produceError"
      ],
      "extensions": {
        "type": "SomeError"
      }
    }
  ]
}

Async Graphql with similar approach as with Juniper

Code:

struct SomeObj {}

#[Object]
impl SomeObj {
    async fn produce_error(&self) -> FieldResult<i32> {
        Err(FieldError::new("Some error"))
    }
}

#[Object]
impl QueryRoot {
    async fn some_obj(&self) -> Option<SomeObj> {
        Some(SomeObj {})
    }

    async fn me(&self, context: &Context<'_>) -> Option<User> {
        get_me(context)
    }
}

Query:

query MyQuery {
  someObj {
    produceError
  }
  me {
    name
  }
}

Response:

{
    "data": null,
    "errors": [
      {
        "message": "Some error",
        "locations": [
          {
            "line": 6,
            "column": 5
          }
        ],
        "path": [
          "someObj",
          "produceError"
        ]
      }
    ]
  }

[sunli829]

The current implementation of Async-graphql is that the entire query will fail after any error occurs. In subsequent versions, I may allow the query to continue to execute when the error occurs.

[Fanneyyy]

Thank you so much for your answer 🙏 That’s too bad. We’ve tried to follow the graphql spec as much as possible, and this could be very confusing for external users of our api since the spec specifies that errors should propagate until a parent field is nullable and no further, Errors and Non-Nullability.

Would supporting this be a major change? Maybe an extension could solve this? But more importantly maybe, is this a direction you would be interested in going? 😊

[sunli829]

After I finish the work at hand, I will come to achieve this, and it will be done quickly when I start.

[sunli829]

Released in 2.9.0

[sunli829]

@Fanneyyy I hope you can help me test the new version and give me some feedback.

[Fanneyyy]

Hey, yes of course and thank you so much for this.
This looks awesome and works super nicely in the examples that I've tried and also in our server. There are some features that we're not using yet, and I'm not very familiar with e.g. field guards that I haven't tested.

One thing to note regarding the implementation which can break schemas.
I.e. the graphql spec specifies that errors are propagated up to the next nullable field and return "null" for that field.
E.g. for this schema:

type QueryRoot {
  nestedErrorTest: ParentObject!
}

type ParentObject {
  id: String!
  nestedOne: MiddleChild!
}

type MiddleChild {
  id: String!
  nestedTwo: YoungestChild
}

type YoungestChild {
  id: String!
  name: String!
}

and this query:

{
  nestedErrorTest {
    id
    nestedOne {
      id
      nestedTwo {
        id
        name <- returns error
      }
    }
  }
}

If name returns an error, the query should return the following payload:
(because name is required and cannot be null the error is propagated to the next nullable field which is nestedTwo)

{
  "data": {
    "nestedErrorTest": {
      "id": "I'm the parent object",
      "nestedOne": {
        "id": "I'm a middle child",
        "nestedTwo": null
      }
    }
  },
  "errors": [
    {
      "message": "YoungestChild failure",
      "locations": [
        {
          "line": 8,
          "column": 9
        }
      ],
      "path": [
        "nestedErrorTest",
        "nestedOne",
        "nestedTwo",
        "name"
      ]
    }
  ]
}

The current implementation however returns the payload below, where name is null which could break clients that trust the field to be present.

{
  "data": {
    "nestedErrorTest": {
      "id": "I'm the parent object",
      "nestedOne": {
        "id": "I'm a middle child",
        "nestedTwo": {
          "id": "I'm the youngest child",
          "name": null
        }
      }
    }
  },
  "errors": [
    {
      "message": "YoungestChild failure",
      "locations": [
        {
          "line": 8,
          "column": 9
        }
      ],
      "path": [
        "nestedErrorTest",
        "nestedOne",
        "nestedTwo",
        "name"
      ]
    }
  ]
}

However, since the error is present this can easily be handled client side. And for most use cases making sure that fields that can return error (be null) are optional will circumvent this issue.

[sunli829]

Thank you for your feedback. I will think about how to propagate null to the next nullable field. 🙂

[sunli829]

@Fanneyyy I released v2.9.1, looking forward to your feedback. 😁

[Fanneyyy]

Great, thank you so much 🙏

This looks really good and the schema is always type safe.
One thing I noticed with the error propagation which could be confusing when coding.

I.e. when I have an optional value from an external request I would usually create the field like so (opt_description):

struct ParentObject;

#[Object]
impl ParentObject {
    async fn child_opt(&self) -> Option<ChildObject> {
        Some(ChildObject)
    }
}

struct ChildObject;

#[Object]
impl ChildObject {
    async fn opt_description(&self) -> Result<Option<String>> {
        let maybe_description = Some("some description".to_owned());
        Ok(maybe_description)
    }
}

struct QueryRoot;

#[Object]
impl QueryRoot {
    async fn parent(&self) -> ParentObject {
        ParentObject
    }
}

This field would be optional in the graph but if it returns an error the error is still propagated to the next optional field in the graph, i.e. the query:

{
    parent {
        childOpt {
            optDescription <- returns error
        }
    }
}

Returns:

{
  "data": {
    "parent": {
      "childOpt": null
    }
  },
  "errors": [
    {
      "message": "description error",
      "locations": [
        {
          "line": 4,
          "column": 7
        }
      ],
      "path": [
        "parent",
        "childOpt",
        "optDescription"
      ]
    }
  ]
}

Even though opt_description is an optional field in the graph, i.e. you would assume that the response would be:

{
  "data": {
    "parent": {
      "childOpt": {
        "optDescription": null
      }
    }
  },
  "errors": [
    {
      "message": "description error",
      "locations": [
        {
          "line": 4,
          "column": 7
        }
      ],
      "path": [
        "parent",
        "childOpt",
        "optDescription"
      ]
    }
  ]
}

You can however achieve this result by creating the resolver like so:

    async fn opt_description(&self) -> Option<Result<Option<String>>> {
        let _maybe_description = Some("some description".to_owned());
        Some(Err("myerror".into()))
    }

And the response would be correct but the code is a bit confusing 😅

[sunli829]

@Fanneyyy
🙂I think you should do this:

async fn opt_description(&self) -> Option<Result<String>> {
    Some(Err("myerror".into()))
}

[bram209]

I would prefer Result<Option<String>> since you specify that when the operation is successful the result could be either nothing or a string.

If it is not successful it may ONLY be an error.

[Fanneyyy]

@sunli829
Yes definitely, you are correct.

My only concern is that both return types, Option<Result<String>> and Result<Option<String>> will be represented the same in the schema, i.e. both optional types. But one will return null on an error but the other will propagate the error to the next optional type. I.e. as a user of the schema I wouldn’t know the difference in code.
This could be explained in docs/book to avoid devs expecting it to work differently, and causing the schema to be inconsistent.

[sunli829]

@sunli829
Yes definitely, you are correct.

My only concern is that both return types, Option<Result<String>> and Result<Option<String>> will be represented the same in the schema, i.e. both optional types. But one will return null on an error but the other will propagate the error to the next optional type. I.e. as a user of the schema I wouldn’t know the difference in code.
This could be explained in docs/book to avoid devs expecting it to work differently, and causing the schema to be inconsistent.

@Fanneyyy
If you are free, I hope you can explain this in the book. 🙂

After Rust supports generic specialization, I will be able to unify the behavior of these two definitions.