Fix the ability of websockets to get errors

[telemenar]

For WebSockets, the flow and contexts involved were such that the errors in marshaling were always dropped rather than sent as part of the response.

This happened because in a new subscription:

• The subscribe transport handler creates an operation context which is then passed to:
  • DispatchOperation then creates tempResponseContext, which is passed into Exec (to get the responses function)
    • Exec then uses tempResponseContext in _Subscription to (to get the next function)
      • _Subscription will then use tempResponseContext in the generated handlers
      • Which will return a function that uses tempResponseContext as the only context passed into marshaling functions.
    • Exec wraps the next() function inside of another function that is returned.
  • The returned function from exec is the responses function which is then wrapped in another function that accepts context but always creates a new Response context. Then retrieves the errors from that context before returning the response.
• The subscribe transport handler also calls the return from DistpatchOperation response and when using it passes in the context returned by DispatchOperation.
• Because the inner function bound into next() in exec captured the tempResponseContext, none of the errors generated when calling next() are ever populated on the context inside the function returned by DispatchOperation that collects the errors.

To solve this I added a context to the next() call so that the response context from the DispatchOperation returned function is passed all the way down and can appropriately accumulate errors as expected.

Added a unit test for this as well.

For reference in the content of subscriptions the caller of DispatchOperation and its returned function is here:
https://github.com/99designs/gqlgen/blob/master/graphql/handler/transport/websocket.go#L352

I have:

• Added tests covering the bug / feature (see testing)
• Updated any relevant documentation (see docs)

[StevenACoffman]

@RobinCPel would you mind reviewing this websocket PR?

[coveralls]

Coverage decreased (-0.05%) to 74.443% when pulling 0c1149a on observeinc:websocketsShouldSendErrorsMaster into 5a49764 on 99designs:master.

[telemenar]

Are the golangcli-lint check failures something I need to look at?

First glance looks like an infra failure to me rather than something relating to the PR?

But if I’m mid-interpreting the results happy to make changes.

[StevenACoffman]

I think I fixed the golangci-lint errors on master, so you might just rebase against that and see if it fixes it for this PR.

[telemenar]

Some important pointers into the code that might help on review here.

Picking a random generated file:
https://github.com/99designs/gqlgen/blob/master/codegen/testserver/singlefile/generated.go#L2085

• The context on this line passed into ec._Subscription(ctx, rc.Operation.SelectionSet)
• Becomes captures in the returned function saved into next()
• That context is what accumulates errors on lines like this:
  • https://github.com/99designs/gqlgen/blob/master/codegen/testserver/singlefile/generated.go#L19555

Because the captured context accumulates the errors rather than the context which is passed into the function returned from Exec then this line here:
https://github.com/99designs/gqlgen/blob/master/graphql/executor/executor.go#L110

Doesn't see those errors.

For reference in the content of subscriptions the caller of DispatchOperation and its returned function is here:
https://github.com/99designs/gqlgen/blob/master/graphql/handler/transport/websocket.go#L352

[StevenACoffman]

Thank you for the additional pointers. This looks correct to me and is a very nice improvement. I'm going to merge this, but if @RobinCPel or other WebSocket devs have any review feedback or comments even after it is merged, I'm happy to address those in a follow-up PR.

[RobinCPel]

Thank you for the additional pointers. This looks correct to me and is a very nice improvement. I'm going to merge this, but if @RobinCPel or other WebSocket devs have any review feedback or comments even after it is merged, I'm happy to address those in a follow-up PR.

I don't know much about the code generation of GqlGen, but everything that I do understand looks fine! 🚀

[StevenACoffman]

Thanks for looking it over!