[gql_websocket_link/gql_link] SubscriptionError information is not propagated to the WebSocketLinkServerException

[shyndman]

Hi there,

I'm in a situation where a bad query provided to a WebSocketLink is resulting in a SubscriptionError from the server. The SubscriptionError itself is quite descriptive, and provides enough information to debug the issue. However, that information is lost when parsing into a Response.

A bit of digging suggests that response_parser.dart in the gql_link package doesn't handle SubscriptionError messages appropriately.

To reproduce:
Send an invalid GQL document through a websocket link.

Attached is a screenshot of the JSONified message the response parser is dealing with:

Required for nhost/nhost-dart#24

[juancastillo0]

Related, the SubscriptionError just returns the complete payload in the toJson method, without any parsing into errors or extensions. The default ResponseParser always returns an empty map. Maybe we could expect something different like this?

  @override
  Map<String, dynamic> toJson() {
    final Object? payload = this.payload;
    Object? extensions;
    List<Object?>? errors;
    if (payload is List) {
      errors = payload;
    } else if (payload is Map) {
      extensions = payload["extensions"];
      if (payload["errors"] is List) {
        errors = payload["errors"] as List;
      } else if (payload is Map) {
        errors = [payload];
      }
    }
    return <String, dynamic>{
      "type": type,
      "id": id,
      "payload": payload,
      if (extensions != null) "extensions": extensions,
      if (errors != null) "errors": errors,
    };
  }

graphql-transport-ws expects a list of GraphQL errors. graphql-ws from what I can see in the code is an object with a message property, in the PROTOCOL it just says and Error. I can see that this isn't standardized, but maybe it would reduce the need to implement a ResponseParser which checks for errors when a "payload" field is in the json and parsing it from that.

I thinks it makes sense to try to expect a payload like the one shown by @shyndman, since it's similar to a data Message and allows for multiple validation errors.

Maybe be could implement a similar toJson but with the errors variable as List<Map<String, Object?>>? errors; so that the ResponseParser doesn't throw when a different format is provided. Are you open for a pull request for this?

[agent3bood]

@juancastillo0 Thanks for taking a look at this.
I want to suggest something on you proposal, change the ConnectionError signature, this way there will be no logic in the toJson method, and the caller of ConnectionError must provide all the needed data ConnectionError(payload, errors, extensions);.

Do you think you can make a PR for this change?

PS: Will this be fine ConnectionError(errors, extensions)?