More customizable errors would be nice

[pcarrier]

It's currently impossible to report errors whose message does not start with "Exception while fetching data: ".

Overriding ExecutionStrategy.resolveField is a bad workaround (and see #208).

[dminkovsky]

"Exception while fetching data" is a hard-wired string that is logged. It is not returned to clients. Do you mean you want to be able to customize this log message?

[pcarrier]

For: query throws { error(message:"hello") } with:

rootQueryBuilder.field(newFieldDefinition()
  .argument(newArgument()
    .name("message")
    .type(Scalars.GraphQLString)
    .build())
  .type(Void)
  .dataFetcher(env -> {
    final String message = env.getArgument("message").toString();
    if (message == null) {
      throw new GraphQLException();
    } else {
      throw new GraphQLException(message);
    }
  })
  .build())

I get:

{
  "data": {
    "error": null
  },
  "errors": [
    {
      "exception": {
        "cause": null,
        "stackTrace": [
          {
            "methodName": "lambda$queryType$19",
            "fileName": "OpticsSchema.java",
            "lineNumber": 776,
            "className": "apollo.optics.graphql.OpticsSchema",
            "nativeMethod": false
          },
          {
            "methodName": "resolveField",
            "fileName": "ExecutionStrategy.java",
            "lineNumber": 40,
            "className": "graphql.execution.ExecutionStrategy",
            "nativeMethod": false
          },
          {
            "methodName": "execute",
            "fileName": "ExecStrategy.java",
            "lineNumber": 35,
            "className": "apollo.optics.graphql.exec.ExecStrategy",
            "nativeMethod": false
          },
          {
            "methodName": "executeOperation",
            "fileName": "Execution.java",
            "lineNumber": 60,
            "className": "graphql.execution.Execution",
            "nativeMethod": false
          },
          {
            "methodName": "execute",
            "fileName": "Execution.java",
            "lineNumber": 33,
            "className": "graphql.execution.Execution",
            "nativeMethod": false
          },
          {
            "methodName": "execute",
            "fileName": "GraphQL.java",
            "lineNumber": 78,
            "className": "graphql.GraphQL",
            "nativeMethod": false
          },
          {
            "methodName": "lambda$null$1",
            "fileName": "GraphQLHandler.java",
            "lineNumber": 57,
            "className": "apollo.optics.graphql.GraphQLHandler",
            "nativeMethod": false
          },
          {
            "methodName": "lambda$op$7",
            "fileName": "Blocking.java",
            "lineNumber": 231,
            "className": "ratpack.exec.Blocking",
            "nativeMethod": false
          },
          {
            "methodName": "lambda$get$0",
            "fileName": "Blocking.java",
            "lineNumber": 70,
            "className": "ratpack.exec.Blocking$1",
            "nativeMethod": false
          },
          {
            "methodName": "intercept",
            "fileName": "Blocking.java",
            "lineNumber": 244,
            "className": "ratpack.exec.Blocking",
            "nativeMethod": false
          },
          {
            "methodName": "access$000",
            "fileName": "Blocking.java",
            "lineNumber": 35,
            "className": "ratpack.exec.Blocking",
            "nativeMethod": false
          },
          {
            "methodName": "get",
            "fileName": "Blocking.java",
            "lineNumber": 68,
            "className": "ratpack.exec.Blocking$1",
            "nativeMethod": false
          },
          {
            "methodName": "get",
            "fileName": "Blocking.java",
            "lineNumber": 61,
            "className": "ratpack.exec.Blocking$1",
            "nativeMethod": false
          },
          {
            "methodName": "run",
            "fileName": "CompletableFuture.java",
            "lineNumber": 1590,
            "className": "java.util.concurrent.CompletableFuture$AsyncSupply",
            "nativeMethod": false
          },
          {
            "methodName": "runWorker",
            "fileName": "ThreadPoolExecutor.java",
            "lineNumber": 1142,
            "className": "java.util.concurrent.ThreadPoolExecutor",
            "nativeMethod": false
          },
          {
            "methodName": "run",
            "fileName": "ThreadPoolExecutor.java",
            "lineNumber": 617,
            "className": "java.util.concurrent.ThreadPoolExecutor$Worker",
            "nativeMethod": false
          },
          {
            "methodName": "lambda$newThread$0",
            "fileName": "DefaultExecController.java",
            "lineNumber": 136,
            "className": "ratpack.exec.internal.DefaultExecController$ExecControllerBindingThreadFactory",
            "nativeMethod": false
          },
          {
            "methodName": "run",
            "fileName": "DefaultThreadFactory.java",
            "lineNumber": 144,
            "className": "io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator",
            "nativeMethod": false
          },
          {
            "methodName": "run",
            "fileName": "Thread.java",
            "lineNumber": 745,
            "className": "java.lang.Thread",
            "nativeMethod": false
          }
        ],
        "message": "hello",
        "localizedMessage": "hello",
        "suppressed": []
      },
      "message": "Exception while fetching data: graphql.GraphQLException: hello",
      "locations": null,
      "errorType": "DataFetchingException"
    }
  ]
}

[pcarrier]

I guess my point is that it'd be nice to add errors to the env that don't necessarily even have an exception, that have an errorType other than DataFetchingException, and it does look indeed like the message has a prefix I can't control.

[dllx]

In our HTTP endpoint getting application/json GrapQL requests and providing the JSON responses, we are extracting the Exceptions (incl. DataFetchingExceptions) from the errors in the ExecutionResult obtained from GraphQL.execute. Depending on the exceptions' causes, we populate the "errors" part in the JSON response, inventing some additional fields to the error structure.

To some degree, this works, but we'd like to better associate an error with a certain mutation in the request (if there are multiple mutations). So maybe this can be considered here.

[laurentpellegrino]

💯 : I am facing the same issue. It would be really convenient to be able to define an error formatting strategy (e.g. I would like to exclude the stacktrace in my case).

[dminkovsky]

Oh hey, I was misunderstanding this too. Sorry I was so slow about this. Yikes.

Anyway, yes, we should improve this.

[dminkovsky]

FWIW, here's a Jackson serializer (if anyone is using Jackson) for ExecutionResult that will skip printing the stacks if an error is an ExceptionWhileDataFetching:

Implementation:

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.ser.std.StdSerializer;
import graphql.ExecutionResult;
import graphql.GraphQLError;
import graphql.language.SourceLocation;

import java.io.IOException;
import java.util.List;

public class ExecutionResultSerializer extends StdSerializer<ExecutionResult> {

    public ExecutionResultSerializer() {
        this(null);
    }

    public ExecutionResultSerializer(Class<ExecutionResult> t) {
        super(t);
    }

    @Override
    public void serialize(ExecutionResult executionResult, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
        Object data = executionResult.getData();
        jsonGenerator.writeStartObject();
        if (data != null) {
            jsonGenerator.writeObjectField("data", data);
        }
        List<GraphQLError> errors = executionResult.getErrors();
        if (errors != null && !errors.isEmpty()) {
            jsonGenerator.writeArrayFieldStart("errors");
            for (GraphQLError error : errors) {
                jsonGenerator.writeStartObject();
                jsonGenerator.writeFieldName("message");
                jsonGenerator.writeString(error.getMessage());
                List<SourceLocation> locations = error.getLocations();
                if (!locations.isEmpty()) {
                    jsonGenerator.writeArrayFieldStart("locations");
                    for (SourceLocation location : locations) {
                        jsonGenerator.writeObject(location);
                    }
                    jsonGenerator.writeEndArray();
                }
                jsonGenerator.writeEndObject();
            }
            jsonGenerator.writeEndArray();
        }
        jsonGenerator.writeEndObject();
    }
}

Test:

import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.databind.module.SimpleModule
import graphql.ErrorType
import graphql.ExecutionResult
import graphql.ExecutionResultImpl
import graphql.GraphQLError
import graphql.language.SourceLocation
import org.skyscreamer.jsonassert.JSONAssert
import spock.lang.Specification


class ExecutionResultSerializerTest extends Specification {

    def mapper = new ObjectMapper()

    def setup() {
        SimpleModule module = new SimpleModule()
        module.addSerializer(ExecutionResult.class, new ExecutionResultSerializer())
        mapper.registerModule(module)
    }

    def "data and errors null"() {
        given:
        def result = new ExecutionResultImpl(null, null)

        when:
        def serialized = mapper.writeValueAsString(result)

        then:
        JSONAssert.assertEquals('{}', serialized, false)
    }

    def "just data"() {
        given:
        def result = new ExecutionResultImpl([field: 'value'], null)

        when:
        def serialized = mapper.writeValueAsString(result);

        then:
        JSONAssert.assertEquals('{ "data": { "field": "value" } }', serialized, true)
    }

    def "just errors"() {
        given:

        def result = new ExecutionResultImpl(null, [
          new GraphQLError() {

              @Override
              String getMessage() {
                  return "message"
              }

              @Override
              List<SourceLocation> getLocations() {
                  return [new SourceLocation(1, 1)]
              }

              @Override
              ErrorType getErrorType() {
                  return null
              }
          }
        ])

        when:
        def serialized = mapper.writeValueAsString(result)

        then:
        JSONAssert.assertEquals('{ "errors": [ { "message": "message", "locations": [ { "column": 1, "line": 1 } ] } ] }', serialized, true)
    }

    def "data and errors"() {

        def result = new ExecutionResultImpl([field: 'value'], [
          new GraphQLError() {

              @Override
              String getMessage() {
                  return "message"
              }

              @Override
              List<SourceLocation> getLocations() {
                  return [new SourceLocation(1, 1)]
              }

              @Override
              ErrorType getErrorType() {
                  return null
              }
          }
        ])

        when:
        def serialized = mapper.writeValueAsString(result)

        then:
        JSONAssert.assertEquals('{ "data": { "field": "value" }, "errors": [ { "message": "message", "locations": [ { "column": 1, "line": 1 } ] } ] }', serialized, true)

    }
}

[jonerer]

@dminkovsky thanks! I was looking for something similar. I won't want risk exposing internals by showing a stacktrace. Your code worked fine except for one thing:
Had to change
if (!locations.isEmpty()) {
into
if (locations != null && !locations.isEmpty()) {

Or I'd get NPEs while serializing (in my case a dataFetcher threw an IllegalArgumentException), and "location" was null

[dminkovsky]

@lulzmachine thanks for the bugfix :)

[yeyuexia]

That's my code to custom exception. use Json to operate json format. you can do more thing in errorHandle and toJson

    private Throwable errorHandle(GraphQLError error) {
        if (error.getErrorType() == ErrorType.DataFetchingException) {
            return ((ExceptionWhileDataFetching) error).getException();
        } else {
            return new CustomException();
        }
    }

    private Json toJson(Throwable throwable) {
        final Json json = Json.read(json(throwable));
        json.delAt("stackTrace");
        json.delAt("localizedMessage");
        json.delAt("suppressed");
        return json;
    }

    private List<Json> customError(ExecutionResult executionResult) {
        return executionResult.getErrors()
            .stream()
            .map(this::errorHandle)
            .map(this::toJson)
            .collect(Collectors.toList());
    }

[bbakerman]

There is a PR for this in progress right now. I dont have it handy to link sorry

[bbakerman]

#387