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

More info on errors #60

Closed
EmperorEarth opened this issue Feb 18, 2017 · 6 comments
Closed

More info on errors #60

EmperorEarth opened this issue Feb 18, 2017 · 6 comments

Comments

@EmperorEarth
Copy link

I've been using Dave Cheney's errors pkg to add stack traces to all my errors. In dev, when I'm testing queries in graphiql/my app, I like to return the error directly in the response. I was wondering if there was any way to return the %+v version of errors back instead of the %s version, because right now, I'm not getting the line numbers of various errors and the rest of the stack trace.

@neelance
Copy link
Collaborator

See #49. I think the same solution should be possible in your case. If yes, then I should add some documentation to the readme since this question came up several times now.

@neelance
Copy link
Collaborator

Oops, I referenced the wrong issue in the commit. Reopening.

@neelance neelance reopened this Mar 11, 2017
@neelance
Copy link
Collaborator

@EmperorEarth Any response to my comment above?

@tonyghita
Copy link
Member

Not to hijack the issue, but the addition of query paths on errors to the specification seems near. See: graphql/graphql-spec#230

At first I was thinking of adding the path value after the query has executed and before we respond to the user, but I think this might be more appropriate in the library.

@EmperorEarth
Copy link
Author

EmperorEarth commented Mar 24, 2017

@neelance, if I understand it right, it doesn't solve what I'm trying to do because the param is still %s in Printf, not %+v. But I don't really understand it at all, so maybe it's better if I try to explain the yak I'm shaving?

One pattern I use in my code is that I log errors once and as late as possible (as high up as possible). Most functions, if they error out, will just wrap them a stack trace and return them (Cheney's errors package). For non-graphql handlers, this is pretty easy, I log errors at the handler level (could also be middleware, but for this discussion, they're the same). If the handlers call any helper functions that error out, these helper functions will just return the raw error for the handler to log/massage.

Given the highly nested nature of graphql resolvers, I want them to simply wrap and return any errors without logging them. Instead, I'd like to aggregate the raw errors at the root graphql handler and log/massage them there. Right now, what I get is a slice of minimally helpful error strings sent back to the client that I inspect in my client's DevTool console.

As a basic example, as I'm rapidly writing code, sometimes I forget to check err == sql.ErrNoRows() before I check err != nil to return a nil resolver. In a regular handler, the helper func that generated the error just returns the wrapped error and at the handler level, I preserve the stack trace when I log with %+v. With your (excellent) library, if a helper function errors out, the resolver silently aggregates error strings stripping the stack trace before returning them back to the client. With any non-trivial graphql query, errors aren't only common, they might even be correct. Without access to the raw errors, rather than the error strings, we have limited context to differentiate between the two.

I guess what I'm looking for is an API to plug into the aggregated, raw slice of errors, right before they are returned to clients, where I can log/massage any/all errors.

@neelance
Copy link
Collaborator

To me this still sounds like you need the same solution as #49. Take the list of errors returned by Schema.Exec and do with them whatever you want before encoding them as JSON and returning them to the client.

dvic pushed a commit to dvic/graphql-go that referenced this issue Feb 5, 2018
Adding an http example for graph-gophers#60

Left to do:
 - [ ] clean up the schema so its only in one place
dvic pushed a commit to dvic/graphql-go that referenced this issue Feb 5, 2018
Adds the simple mutation for graph-gophers#60

Left to do:
 [ ] add an http based example
tinnywang pushed a commit to tinnywang/graphql-go that referenced this issue Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants