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

"GraphQL error" text string in error messages #1812

Closed
SachaG opened this issue Jun 22, 2017 · 28 comments
Closed

"GraphQL error" text string in error messages #1812

SachaG opened this issue Jun 22, 2017 · 28 comments
Assignees

Comments

@SachaG
Copy link

SachaG commented Jun 22, 2017

I've noticed that all error messages passed on from apollo-client include the string "GraphQL error":

message += `GraphQL error: ${errorMessage}\n`;

I'm wondering if this is really necessary? It's not a big deal (I can remove the string myself manually) but in some cases you might want to show an error message from the server when (for example) a mutation fails, and users who know nothing about GraphQL might get confused (I've had reports of "some kind of OpenGL error"… ;) )

@stubailo
Copy link
Contributor

You can retrieve the actual error message from inside the ApolloError object, the message is mostly intended to be a summary for developers and differentiates between a graphql error and a network error.

@SachaG
Copy link
Author

SachaG commented Jun 23, 2017

Is that supposed to work inside errors resulting from mutations, too? When I log out error.message it still has GraphQL error: ....

@stale
Copy link

stale bot commented Jul 15, 2017

This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@jbaxleyiii
Copy link
Contributor

@SachaG you can retrieve all of the GraphQL errors like this:

client
  .query({
    query: gql`
      query {
        viewer {
          login
        }
      }
    `
  })
  .then(data => console.error("TEST RES =>", data))
  .catch(error => {
    console.error("TEST ERR =>", error.graphQLErrors.map(x => x.message));
  });

If you take a look here and open the console to the Frame, you can see the actual issue reported by the server

@SachaG
Copy link
Author

SachaG commented Jul 17, 2017

@jbaxleyiii I actually ended up using https://github.com/thebigredgeek/apollo-errors instead.

@runk
Copy link

runk commented Jul 18, 2017

+1

When you have server-side validation error it's intuitive to use just error.message - it feels wrong that there's implementation/technical detail being exposed to the user. Not sure there's a real value. IMO it'd be far better to swap that deep error with real message with modified one.

Or make the prefix configurable and set it to "GraghQL Error" by default. Happy to send a PR for it you it's something that's going to be accepted / not wasted.

@vmasto
Copy link

vmasto commented Sep 24, 2017

Not sure why this is closed, I've been dazzled by this for the better part of the day trying to remove the prefix.

The most common of use cases:

I fire a mutation from within a component in a try/catch statement. I just want to setState({ error: error.message }) in my catch. My GQL errors are already optimized for user display.

It's not hard to str.replace or introspect and get the original error for a couple of components but on a big app it's unmaintainable to have to do it everywhere manually.

@RDGthree
Copy link

RDGthree commented Oct 9, 2017

Gonna throw in another vote for removing this string, it results in GraphQL error: [Object object] if you have errors with multiple fields. This is broken, bizarre, unnecessary functionality.

@pcjmfranken
Copy link

For some reason mutations over websockets (subscriptions) don't return anything so I'm forced to run the mutation inside a try/catch block and parse the error from there. Unfortunately, this includes the silly 'Graphql error:' which I have to remove manually.

@asotog
Copy link

asotog commented Feb 14, 2018

Same thing here, :(

@joepuzzo
Copy link

I am also having this issue.. Would it not make more sense to simply set error to the actual error?

@diegocouto
Copy link

I can see the value in differentiating GraphQL errors from network errors, but having to filter error.messages is very disappointing. 😞

@gregblass
Copy link

Google'd 'GraphQL error get message' and arrived here. Definitely unintuitive with the whole map thing. Error.message should return the actual message without the 'GraphQL Error:' part.

@danielmahon
Copy link

I agree this should be configurable/overridden somewhere (maybe through apollo-link-error?). I find myself writing message.replace('GraphQL error:', '').trim() alot...

@flux627
Copy link

flux627 commented Mar 26, 2018

Currently replacing the string myself manually too. This needs to be configurable.

@ropaillet
Copy link

ropaillet commented Apr 12, 2018

Made a client side helper method to remove the string... Can anyone with the back knowledge push so they remove it ? User doesn´t care about GraphQl and having to make such implementations either on the client side or a custom server side error code makes no sense.

export function serverError(errorMsg) {
return errorMsg.replace('GraphQL error:', ' ').trim();
}

Specially important for login and signup mutations where you use server side REGEX to validate inputs and throw errors if it fails.

@Undeadlol1
Copy link

Seriously, please fix.

@JustinPlute
Copy link

+1

@xenovon
Copy link

xenovon commented May 3, 2018

At first i thought this one coming from backend (i'm frontend guy). I get some complain from the user why error message contain this cryptic string that they don't understand (obviously they don't know what graphql is). I told backend team to change it, until they send link of this issues.

Okay, then my fate has been sealed to fix this issues.

I think this one need to be removed. If there some need to add this string for debugging process, we can always add additional variable for it.

@ghost
Copy link

ghost commented May 17, 2018

+1

@k00k
Copy link

k00k commented May 17, 2018

I think the ability to customize/overwrite the error message would be well-placed into apollo-link-error. Currently adding apollo-link-error to the client simply logs the error to the console, it doesn't propagate through to the error callback. Perhaps there's a way to do this already?

@venkat4541
Copy link

I think this is more of a GraphQL issue rather than Apollo but I may be wrong. I would try to get response from GraphQL team rather. But this issue is causing a headache on front-end.

Anyone found any way around to change the string to an object?

@giraudvalentin
Copy link

Why this is closed ?

@sunstorymvp
Copy link

At least "GraphQL error: " prefix should be added before apollo-link-error so it is possible to get rid of it in one place...

@stolinski
Copy link
Contributor

stolinski commented Jul 17, 2018

I'm also wondering why this was closed.

@hwillson hwillson reopened this Jul 17, 2018
@hwillson hwillson self-assigned this Jul 17, 2018
@hwillson
Copy link
Member

I think it's safe to say that the community has spoken on this one, and that we should start looking into a different approach here. I've re-opened this issue - thanks all!

@lorensr
Copy link
Contributor

lorensr commented Aug 26, 2018

[Object object]

I've seen GraphQL error: [Object object] before, but I don't remember why. @RDGthree—you said it's caused by multiple fields with errors, but I'm unable to replicate that. I'm seeing error.message as:

GraphQL error: message from field1
GraphQL error: message from field2

https://codesandbox.io/s/4xn6pyl1r9

One thing that does cause it is creating an error with an object message, but that's how JS works:

image

You'd have to stringify first:

image

Stripping prefix

I don't imagine devs wanting to show network errors, only graphql errors? In which case we could leave in the network prefix if we wanted? So error.message would be:

  • One graphql error:
message from field1
  • Two graphql errors:
message from field1
message from field2
  • Two graphql errors and one network error (is this possible? the code at least currently covers this case)
message from field1
message from field2
Network error: sorry, your cable got cut

Or we could prefer consistency and strip Network error: as well:

message from field1
message from field2
sorry, your cable got cut

Thoughts?

@hwillson
Copy link
Member

hwillson commented Sep 5, 2018

To help provide a more clear separation between feature requests / discussions and bugs, and to help clean up the feature request / discussion backlog, Apollo Client feature requests / discussions are now being managed under the https://github.com/apollographql/apollo-feature-requests repository.

Migrated to: apollographql/apollo-feature-requests#46

@hwillson hwillson closed this as completed Sep 5, 2018
@apollographql apollographql locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests