Skip to content
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

Enable Subscription Resolver to return websocket error message #2506

Merged

Conversation

zhixinwen
Copy link
Contributor

@zhixinwen zhixinwen commented Jan 12, 2023

Solve issue #1118

Currently there is no way of returning an error to client, after subscription resolver returns channel.
This is inconvenient because some clients have build-in retry on error message, and that is not going to work with gqlgen servers.

Ideally the subscriotion resolver should return an error channel as well, but this is going to be a breaking change.
Instead we can propagate error with context similar to graphql.AddError. This way the change would have no impact to existing users, and users can choose to opt into this behavior.

Tested on my local with resolver code:

func (r *SubscriptionResolver) Method(ctx context.Context, input models.Input) (<-chan *models.Response, error) {
	outputChan := make(chan *models.Response)

	streamClient, err := r.StreamClient.GetStream(ctx,toProto(input))
	if err != nil {
		return nil, mperror.Wrap(err, "fail to make grpc calls")
	}

	go func() {
		defer close(outputChan)
		for {
			outputProto, err :=  r.StreamClient.Recv()
			if err == io.EOF {
				return
			}

			if err != nil {
				transport.AddSubscriptionError(ctx, gqlerror.Errorf(err.Error()))
				return
			}

			outputModel := toModel(outputProto)

			select {
			case <-ctx.Done():
				return
			case outputChan <- outputModel:
				continue
			}
		}
	}()

	return outputChan, nil
}

![Screen Shot 2023-01-12 at 11 35 45 AM](https://user-images.githubusercontent.com/10200233/212168867-1024138f-a58a-4a56-8bd2-66ca2a02a13d.png)

According to the [spec](https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md#error), there should be no more message after error message, and this change adheres to the spec



I have:
 - [ ] Added tests covering the bug / feature (see [testing](https://github.com/99designs/gqlgen/blob/master/TESTING.md))
 - [ ] Updated any relevant documentation (see [docs](https://github.com/99designs/gqlgen/tree/master/docs/content))

@coveralls
Copy link

coveralls commented Jan 13, 2023

Coverage Status

Coverage: 75.542% (-0.09%) from 75.636% when pulling f1648cd on zhixinwen:zhixin/implement-subscription-error into 43c9a1d on 99designs:master.

@StevenACoffman StevenACoffman changed the title Enanble Subscription Resolver to return websocket error message Enable Subscription Resolver to return websocket error message Jan 13, 2023
Zhixin Wen and others added 2 commits January 13, 2023 11:18
Signed-off-by: Steve Coffman <steve@khanacademy.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants