Result<Option<T>> doesn't return partial results. (while Option<Result<T>> does.) about async-graphql HOT 7 OPEN

If you wrap your resolver response in Option I believe this works.

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

At least it has been working for my project.

from async-graphql.

Well, today I learned that the "bug" behavior I'm seeing is actually defined in the GraphQL specification:

If an error was raised during the execution that prevented a valid response, the data entry in the response should be null.

And ascync_graphql is advertised as:

a high-performance graphql server library that's fully specification compliant

So, I suppose this is not a bug. It just differs than multiple GraphQL server implementations I've used before, so was surprising.

from async-graphql.

Oops, nevermind, the spec goes on to say:

If a field error is raised, execution attempts to continue and a partial result is produced (see Handling Field Errors). The data entry in the response must be present. The errors entry should include all raised field errors.

and:

If a field error is raised while resolving a field, it is handled as though the field returned null, and the error must be added to the "errors" list in the response.

So, async_graphql is, AFAICT, not currently "fully specification compliant".

from async-graphql.

I tried that as well, and got the same error. Maybe I'm on an older version. I'll update and try again.

from async-graphql.

As @ashmarchington said, if you want to have a null here it would means that the type of foo and the type of bar should be Nullable, but in async-graphql to have a Nullable type, you must return Option<T>.

When you return Result<String> it means that the inferred type should be a Non-Nullable<String> for GraphQL, but it may fail and if it fails, it'll bubble up the error.

So here, if you change it by Option<Result<String>> it will work.

from async-graphql.

Came back to test this. Created a repository where I can reproduce this issue:
NfNitLoop/async-graphql-bug-1449@5563f45

But then I re-read @Miaxos's comment, suggesting Option<Result<T>> and I tried that. Sure enough, it works:
NfNitLoop/async-graphql-bug-1449@e1531a5#diff-42cb6807ad74b3e201c5a7ca98b911c5fa08380e942be6e4ac5807f8377f87fcR20-R23

But -- why would that work and not Result<Option<T>>? That seems to be the more "correct" type in my mind. It also allows you to use ? for error handling, where Option<Result<T>> does not.

IMO it's a bug that these don't work the same way. Or at a minimum, a really big foot-gun.

from async-graphql.