response with both errors and data about graphql HOT 4 CLOSED

I guess attaching the error to the result and sending it would be able to give you more information as to what part of the query or mutation caused the error but that could still be ambigious.
And shouldn't we remove the panic'king or is it better this way (since we get stack traces)

from graphql.

In this issue, I was asking two things.

1. whether the spec allows it?
2. then should we support it.

I don't think it is the responsibility of a library to capture the stack trace during development , and should never expose it to the end user. I use https://github.com/facebookgo/stackerr to wrap my resolvers and logs them. I think currently we are using defer/panic for easy exit from error case then explicitly passing it through the hierarchy as I never seen the stack logged.

from graphql.

Thanks for bringing this up @bsr203 👍

According the specification I think we should include the execution errors along with the result data.

If the data entry in the response is not null, the errors entry in the response may contain any errors that occurred during execution. If errors occurred during execution, it should contain those errors.

To confirm it, I've wrote a small example using the reference implementation - graphql-js, which does include execution errors and result data:

import {
  graphql,
  GraphQLSchema,
  GraphQLObjectType,
  GraphQLString
} from 'graphql';

var schema = new GraphQLSchema({
  query: new GraphQLObjectType({
    name: 'RootQueryType',
    fields: {
      bar: {
        type: GraphQLString,
        resolve() {
          return 'bar';
        }
      },
      foo: {
        type: GraphQLString,
        resolve() {
          throw new Error('foo error');
        }
      }
    }
  })
});

var query = '{ bar foo }';

graphql(schema, query).then(result => {
  console.log(result);
});

$ babel-node index.js
{ data: { bar: 'bar', foo: null },
  errors: [ { [Error: foo error] message: 'foo error' } ] }

I guess attaching the error to the result and sending it would be able to give you more information as to what part of the query or mutation caused the error but that could still be ambiguous.
And shouldn't we remove the panic'king or is it better this way (since we get stack traces)

Thanks for the insights @pyros2097, yes very valid to think it looks ambiguous, but you might want to consider it that it would be very helpful to know the execution errors and that other fields were resolved correctly, so for debugging purposes it's great - also having partial data available might be useful for some clients that indeed would like to implement some strategies when execution error occur but still have access the resolved partial data.

from graphql.

thanks chris for testing out and confirming the behavior. I too agree with @pyros2097 , but was saying this library shouldn't be capturing stack trace to aid debugging and it is trivial to wrap the resolver in a typical defer/panic/recover mechanism. thanks again for quick response from both of you. cheers.

from graphql.