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

[WIP] Fix subscriptions errors output #636

Closed

Conversation

lebedev
Copy link

@lebedev lebedev commented Nov 28, 2017

Hi!

I've been using GraphiQL with subscriptions-transport-ws package and discovered, that in case of subscriptions if there're some issues on server side, GraphiQL shows [object Object] in output window, despite a message sent over WebSocket having correct stringified JSON.

This happens because in case of an error (of errors), the error (or error.stack) just gets String casted, and if there's no .stack, an error is effectively swallowed by showing [object Object] instead. And the only way to debug this kind of server issues is to look at WebSocket messages directly, which isn't very convenient.

There're 3 places in code where errors get String casted. Despite me actually tracing this [object Object] issue to only one place, I think that all of them could lead to such inconvenience.

I propose that something should be done with this kind of situations (and with errors just being String casted). My PR isn't merge-ready, it's a matter of discussion.

So, what do you think?

This issue is indirectly mentioned in #17 and #88, and mentioned in apollographql/subscriptions-transport-ws#236 and apollographql/subscriptions-transport-ws#305.

@lebedev
Copy link
Author

lebedev commented Dec 2, 2017

I wasn't sure about the format of errors, but I saw what Github's GraphQL endpoint returns, and judging by that, it returns an array of objects (stringified error messages from the server), even with a single error.

So I think that the corresponding code either receives an instance of Error, if an error happened on client, which should have .stack and can be String casted, or an array of objects came from server, which should be JSON.stringifyed before showing.

@asiandrummer
Copy link
Contributor

Hi @angly-cat - thanks for firing up the discussion. Your observation looks valid except for the case where the server is already serving up a JSON-stringified error message, which GraphiQL then would show a doubly-stringified error message.

I agree that we're probably operating on a non-standard feature such as error.stack (probably Mozilla only - haven't actually checked), which is why we're relying on the server side to provide either a error.stack property or an error message that can be String casted. Perhaps subscriptions-transport-ws needs to be updated with regards to how it serves up errors?

@Neitsch
Copy link
Contributor

Neitsch commented Jul 27, 2019

@lebedev - I'd be fine with just JSON.stringify ing the whole error. Would you mind rebasing and doing so? :)

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.

None yet

4 participants