absinthe-socket: Error on config results in `Error: [object Object]`

The issue is here: https://github.com/absinthe-graphql/absinthe-socket/blob/master/packages/socket/src/pushRequest.js#L43

It seems this should check to see if the response is a map or a string, and call gqlErrorsToString in case of the former.

Possibly related to #20

👍 I guess we have to ping the maintainers? @mgtitimoli @tlvenn @bruce

However regarding to this specific topic, the spec says the message must be string, and there is an extensions field to provide additional context about the error.

The spec does say that message should be a string, but errors should be a list of maps:

The errors entry in the response is a non‐empty list of errors, where each error is a map.

This is actually what Absinthe Socket should be returning, something like:

{errors: [{locations: [{column: 0, line: 2}], message: "unauthorized"}]}

I don’t think we need extensions for that because the received error is already properly formatted by Absinthe. Absinthe Socket just has to forward the errors map as-is, without converting it to a string. This is how queries and mutations already work, for example.

Currently, the error is lost and there is no way for applications to get it (e.g. to log it or send it to an error tracking platform like Sentry).

WDYT?

Below is the comment I wrote in #39:

Message must be string by spec. We could consider adding support to extensions.

The spec says:

The errors entry in the response is a non‐empty list of errors, where each error is a map.

If no errors were encountered during the requested operation, the errors entry should not be present in the result.

[…]

Every error must contain an entry with the key message with a string description of the error intended for the developer as a guide to understand and correct the error.

If an error can be associated to a particular point in the requested GraphQL document, it should contain an entry with the key locations with a list of locations, where each location is a map with the keys line and column, both positive numbers starting from 1 which describe the beginning of an associated syntax element.

Which is what the parent sends (assuming atoms are serialized into strings):

{:error, %{errors: [%{locations: [%{column: 0, line: 2}], message: :unauthorized}]}}

So this looks like a bug in Absinthe Socket. This prevents proper error handling as there is no way to access the original error data later in the pipeline.

I cannot think of a workaround to avoid that as createAbsintheSocketLink returns a terminating link so there is no way to change the error format on front side before it is parsed by Absinthe Socket.

Also, Apollo Client’s Http Link handles these errors properly so it’s surprising for users adding support for subscriptions to their app: GraphQL errors over HTTP are handled properly but the get an [object Object] over websocket.

I just ran into this error as well, after quite a bit of digging and debugging, I don’t think it looks like this should be returning just an Error string. The errorMessage variable doesn’t actually contain a message, it contains a response body, and the name responseData or something might be better. It looks like the logic here affects socket-apollo-link and causes an incompatibility with the API that the apollo-link libraries use.

I discovered this when I attempted to manage errors within my application by using the pretty standard { errors: [{...}] } object that gets returned from a query, and it turns out that’s not possible. Similarly, I attempted to use apollo-link-error, and my onError method is actually being passed a big string with the error message casted to [object Object] within it instead of a result object as expected.

This should likely be returning a payload of { errors: [{...}] } instead of a stringified version of it in the case of a graphQL error, to allow consumers of the socket data to properly digest the error messages.

In the case of apollo, other apollo-link libraries take in data like this. apollo-link-error handles their return values here. The graphQLErrors field in apollo-link-error relies on the error returned having a value of: { result: { errors: [{...}] } }

Similarly, this logic takes this object and uses it to populate the response field, which passes down and allows for the errors to be returned to the user application for further processing.

I made a quick adjustment just for the purposes of local testing to pushRequestUsing.js line 42.

if (errorMessage && typeof errorMessage === 'object') {
   return abortNotifier(absintheSocket, notifier, { result: errorMessage });
 } else {
   return abortNotifier(absintheSocket, notifier, createRequestError(errorMessage));
}

This works as expected with apollo, successfully returning the data to the application while being compatible with apollo-link. I feel like other non-apollo clients would also be expecting the socket to return a standard { errors: [{...}] } response. Perhaps this can return the { errors: [{...}] }, and then socket-apollo-link could wrap it in a { result: { errors: [{...}] } } structure? Would this cause any issues that I haven’t identified?