Allow propagation of errors from Subscriptions channels into Request.Errs

[abourget]

Hi there,

From what I can gather in the current source, it's not possible to gather errors that happened on the producer side of the chan that is used to stream subscription responses. Having only a channel, you can write responses, or close the channel. If, while streaming one of the messages, your backend database fails, you can't send a proper error message, saying why you are closing the subscription.

I propose adding an interface, like:

type SubscriptionError interface {
    SubscriptionError() error
}

and if implemented, would be checked before resolving all the fields. If such a function returns an error, then it would be AddError()'d, and return null data immediately.

With such a mechanism, the top-level objects could gather errors in a field on that object before pushing it to the chan, and expose it through SubscriptionError().

Feedback welcome, I was thinking my team could implement it and propose a PR, but wanted to open the discussion first. There are probably two dozens better solutions, please share :)

[CreatCodeBuild]

May you give an example from the user side of the lib?

[abourget]

Injecting an object in the channel kicks off the Resolver on that object.

At this line: https://github.com/graph-gophers/graphql-go/blob/master/internal/exec/subscribe.go#L101
where the resp object is the received object within graphql-go, we could do a check for the SubscriptionError on resp, and call it if it fails, and return something like:

if err := subscriberErrorObject.SubscriberError(); err != nil {
	c <- &Response{Errors: []*errors.QueryError{errors.Errorf("%s", err)}}
	continue
}

The user would simply need to return an object to be resolved that implements that method, and returns an error object that it has kept within.

Ex:

type Person struct {
    Name string
    Email string
    err error
}
func (p *Person) SubscriptionError() error {
    return err
}

and while fetching that Person, if you get any failure, you can return that in the message you send over... and decide, or not, to close the channel after that error.

This makes it so subscriptions can avoid changing error handling in front-end code: messages always contains errors in the message.

Otherwise, there's no way to specify why you're about to close a subscription.. if something fatal occurred.

Does that make sense?

[pavelnikolov]

If you treat the errors as any other data in your struct, then why does the library even need to know about this? It'll just be part of the response, right?

[abourget]

Because in the subscription code, a Response is crafted passing your object as Data. You can't pass anything for Errors

[pavelnikolov]

You can always treat errors as data and add them to your schema. Otherwise, how do you imagine subscriptions?

…

Sent from my phone

On 9 May 2019, at 11:22, Alexandre Bourget ***@***.***> wrote: Because in the subscription code, a Response is crafted passing your object as Data. You can't pass anything for Errors — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[abourget]

Hmm, not sure I get that. GraphQL already has an error model. I simply want to reuse that. My data model is the same in queries as in subscriptions. I don't want to craft my own custom error handling in my schema. Like a UserWithError type?!

I've never seen an API do that.

[abourget]

Subscriptions return standard GraphQL responses, composed of a "data" field, and an optional "errors" field, with the structure in the "errors" package (in this repo).

[abourget]

Check the PR referenced above ^^

[pavelnikolov]

I'm looking into the PR. I'm sorry I didn't understand the problem earlier.

[pavelnikolov]

There is a pending conflict and question... 👀

[abourget]

Sorry for the delay.. I've gone through and tried to address the questions.

I know there are 2 features in there.. tell me what it takes to bring those things in :)

[pavelnikolov]

As a beginning, send the subscription error propagation first in a separate PR.

[pavelnikolov]

@abourget I think that #420 solve this issue, right? And I believe that this unit test demonstrates the ability to propagate subscription errors to the user. Let me know if more documentation is needed to explain this better.