error handling

[oonxt]

The doc says: "Responses can contain both data and errors, for example when some fields where resolved successfully, but other fields had errors. A field with an error is set to null, and an error is added to the errors block."

but the data is null in my case. Am I missing something here?

graphqls:

type Query {
    getA: Int!

    getB: Int!
}

code:

@DgsComponent
public class DgsHandler {

    @DgsQuery
    public Integer getA() {
        return 1;
    }

    @DgsQuery
    @DgsEnableDataFetcherInstrumentation
    public Integer getB() throws MyException {
        throw new MyException();
    }

}


@Component
public class CustomDataFetchingExceptionHandler implements DataFetcherExceptionHandler {
    private final DefaultDataFetcherExceptionHandler defaultHandler = new DefaultDataFetcherExceptionHandler();

    @Override
    public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) {
        if(handlerParameters.getException() instanceof MyException) {
            Map<String, Object> debugInfo = new HashMap<>();
            debugInfo.put("somefield", "somevalue");

            GraphQLError graphqlError = TypedGraphQLError.INTERNAL.message("This custom thing went wrong!")
                    .debugInfo(debugInfo)
                    .path(handlerParameters.getPath()).build();
            return DataFetcherExceptionHandlerResult.newResult()
                    .error(graphqlError)
                    .build();
        } else {
            return defaultHandler.onException(handlerParameters);
        }
    }
}

query:

query {
    getA
    getB
}

response:

{
    "errors": [
        {
            "message": "This custom thing went wrong!",
            "locations": [],
            "path": [
                "getB"
            ],
            "extensions": {
                "errorType": "INTERNAL",
                "debugInfo": {
                    "somefield": "somevalue"
                }
            }
        }
    ],
    "data": null
}

[oonxt]

expected response:

{
    "errors": [
        {
            "message": "This custom thing went wrong!",
            "locations": [],
            "path": [
                "getB"
            ],
            "extensions": {
                "errorType": "INTERNAL",
                "debugInfo": {
                    "somefield": "somevalue"
                }
            }
        }
    ],
    "data":  {              ### this should be not null
        "getA": 1          
    }
}

[swargolet]

I'm having the exact same issue and have yet to solve it. I have tried custom exception handlers as well as exceptions that implement GraphQLError. I'm in the process of working through debugger to see exactly what is going on but still have yet to figure out why data is always null when an error occurs.

[swargolet]

I figured out why this is happening, but don't yet know how to solve it. This is happening because the field erroring out (in your case getB) is marked as non-nullable in the schema but when an error occurs, that field will end up being null. This ends up breaking the full query. I'm not too sure why there isn't an error being logged or returned in the response like a typical null being returned for a non-null field. Secondly, I feel like if a field errors out and an error is returned in the response, the null check for that field should be ignored.

I confirmed this in two separate ways. In my datafetcher I wrapped everything in a try/catch and then returned back a default data value and the error in the DataFetcherResult. This is not a solution as a default data value doesn't make sense most of the time. I also removed the '!' from the field in the schema to allow nulls. This is also not a solution as in an ideal situation this field would not be null.

Hoping someone has ideas around this.

[swargolet]

Hmm. Looks like Netflix DGS is following spec exactly, so this isn't technically an issue.

I understand why this is done as the user expected a non-null result, but it also makes the partial result + error feature somewhat useless in many instances. A focus on schema design and usage of DataFetcherResult where it makes sense can help with this.

[oonxt]

Hmm. Looks like Netflix DGS is following spec exactly, so this isn't technically an issue.

I understand why this is done as the user expected a non-null result, but it also makes the partial result + error feature somewhat useless in many instances. A focus on schema design and usage of DataFetcherResult where it makes sense can help with this.

OK...
In other words, the graphql definition can NOT be non-null, if the query may cause an exception.

type Query {
    getA: Int!

    getB: Int           # not getB: Int!  
}

It works!!!

But it's so weird because every query may cause an exception.......

[swargolet]

Agreed that this will be somewhat difficult to workaround. My plan is as follows:

• All queries which return a single item may be nullable
• All queries which return more than one item (a list) will return an empty list if an error happens and will also remove null elements from the list. This will either happen in the data fetcher or possibly instrumentation which can handle it across all list fields.
• I have yet to figure out how to handle non-nullable fields within a query as according to spec, the null will bubble up and "break" each parent field until it reaches a nullable field. I'm leaning towards making the "wrapper" field nullable and internal fields for that type non-nullable where applicable. It isn't ideal but better than the whole query erroring out.

Hoping others have suggestions on how they tackled this concern.

[MazizEsa]

So how you usually handle error in data (with your own custom data structure)?
You handle it within the @DGSQuery?

Any more centralize way to do this (ala Controller advice)?

On Spring graphql you also have an option to use WebGraphQlInterceptor, which allow you to modify the response (add error in data, etc.)

[swargolet]

We ended up switching to Spring GraphQL recently, but still had to come up with a solution for this. So we created a custom annotation to handle it and then overrode the execution strategy to pull the default if the fetched value was null.

Annotation definition

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
 * This annotation provides the ability to define a default value to use for a fetched result when no result is returned by a data fetcher. This may be
 * beneficial when resolving a non-nullable field and an exception occurs.  It allows GraphQL errors to be returned and a default value.  The
 * {@link GraphQLDefaultAnnotationInterceptor} intercepts and adds the default value onto the {@link graphql.GraphQLContext} which the
 * {@link DefaultHandlingExecutionStrategy} uses to apply the default value.
 * <p>
 * <b>This annotation only applies on functions that have a {@link graphql.schema.DataFetchingEnvironment} or {@link graphql.GraphQLContext} as an argument.</b>
 */
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface GraphQLDefault {

  enum DefaultType {
    LIST,
    OPTIONAL
  }

  DefaultType value();
}

Aspect

import graphql.GraphQLContext;
import graphql.execution.MergedField;
import graphql.schema.DataFetchingEnvironment;
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.stereotype.Component;

/**
 * @see GraphQLDefault
 */
@Aspect
@Component
@Order(Ordered.HIGHEST_PRECEDENCE + 1)
public class GraphQLDefaultAnnotationInterceptor {
  /**
   * Matching DataFetchingEnvironment argument in any order
   **/
  @Before("@annotation(graphQLDefault) && args(dfe,..)")
  public void interceptAsDfeFirstParam(GraphQLDefault graphQLDefault, DataFetchingEnvironment dfe) {
    setDefaultToContext(graphQLDefault, dfe);
  }

  @Before("@annotation(graphQLDefault) && args(..,dfe)")
  public void interceptAsDfeLastParam(GraphQLDefault graphQLDefault, DataFetchingEnvironment dfe) {
    setDefaultToContext(graphQLDefault, dfe);
  }

  @Before("@annotation(graphQLDefault) && args(*,dfe,..)")
  public void interceptAsDfeMiddleParam(GraphQLDefault graphQLDefault, DataFetchingEnvironment dfe) {
    setDefaultToContext(graphQLDefault, dfe);
  }

  private void setDefaultToContext(GraphQLDefault graphQLDefault, DataFetchingEnvironment dfe) {
    GraphQLContext graphQlContext = dfe.getGraphQlContext();
    if (!graphQlContext.hasKey(DEFAULT_ANNOTATION_KEY)) {
      graphQlContext.put(DEFAULT_ANNOTATION_KEY, new HashMap<>());
    }
    MergedField parentField = dfe.getExecutionStepInfo().getParent() != null ? dfe.getExecutionStepInfo().getParent().getField() : null;
    String fieldIdentifier = GraphQLDefaultAnnotationHelper.getFieldIdentifier(dfe.getExecutionStepInfo().getField(), parentField);
    GraphQLDefaultAnnotationHelper.putDefaultValue(dfe.getGraphQlContext(), fieldIdentifier, getDefaultValue(graphQLDefault.value()));
  }

  private Object getDefaultValue(DefaultType defaultType) {
    if (defaultType == DefaultType.LIST) {
      return List.of();
    } else if (defaultType == DefaultType.OPTIONAL) {
      return Optional.empty();
    } else {
      throw new IllegalArgumentException("Unexpected defaultType " + defaultType);
    }
  }
}

Helper class

import graphql.GraphQLContext;
import graphql.execution.MergedField;
import java.util.Map;

/**
 * @see GraphQLDefault
 * @see GraphQLDefaultAnnotationInterceptor
 */
public final class GraphQLDefaultAnnotationHelper {
  public static final String DEFAULT_ANNOTATION_KEY = "graphqlDefaultValue";

  private GraphQLDefaultAnnotationHelper() {

  }

  public static boolean hasDefaultValue(GraphQLContext graphQlContext, String fieldIdentifier) {
    if (graphQlContext.get(DEFAULT_ANNOTATION_KEY) != null) {
      Map<String, Object> defaultMap = graphQlContext.get(DEFAULT_ANNOTATION_KEY);
      return defaultMap.containsKey(fieldIdentifier);
    }
    return false;
  }

  public static Object getDefaultValue(GraphQLContext graphQlContext, String fieldIdentifier) {
    Map<String, Object> defaultMap = graphQlContext.get(DEFAULT_ANNOTATION_KEY);
    return defaultMap.get(fieldIdentifier);
  }

  public static void putDefaultValue(GraphQLContext graphQlContext, String fieldIdentifier, Object defaultValue) {
    Map<String, Object> defaultMap = graphQlContext.get(DEFAULT_ANNOTATION_KEY);
    defaultMap.put(fieldIdentifier, defaultValue);
  }

  public static String getFieldIdentifier(MergedField curField, MergedField parentField) {
    String parentName = parentField != null ? parentField.getName() : "";
    String fieldName = curField != null ? curField.getName() : "";
    return parentName + "." + fieldName;
  }
}

Custom execution strategy which uses the default value as specified by the annotation if the fetched value is null.

import graphql.GraphQLContext;
import graphql.execution.AsyncExecutionStrategy;
import graphql.execution.DataFetcherExceptionHandler;
import graphql.execution.DataFetcherResult;
import graphql.execution.ExecutionContext;
import graphql.execution.ExecutionStrategyParameters;
import graphql.execution.FetchedValue;
import graphql.execution.MergedField;

public class DefaultHandlingExecutionStrategy extends AsyncExecutionStrategy {
  public DefaultHandlingExecutionStrategy(DataFetcherExceptionHandler exceptionHandler) {
    super(exceptionHandler);
  }

  @Override
  protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Object result) {

    if (result instanceof DataFetcherResult<?> dataFetcherResult) {
      executionContext.addErrors(dataFetcherResult.getErrors());

      Object localContext = dataFetcherResult.getLocalContext();
      if (localContext == null) {
        // if the field returns nothing then they get the context of their parent field
        localContext = parameters.getLocalContext();
      }
      Object value = executionContext.getValueUnboxer().unbox(dataFetcherResult.getData());

      // Default value handling
      GraphQLContext graphQlContext = executionContext.getGraphQLContext();
      MergedField parentField = parameters.getParent() != null ? parameters.getParent().getField() : null;
      String fieldIdentifier = GraphQLDefaultAnnotationHelper.getFieldIdentifier(parameters.getField(), parentField);
      if (value == null && GraphQLDefaultAnnotationHelper.hasDefaultValue(graphQlContext, fieldIdentifier)) {
        value = GraphQLDefaultAnnotationHelper.getDefaultValue(graphQlContext, fieldIdentifier);
      }
      return FetchedValue.newFetchedValue()
          .fetchedValue(value)
          .rawFetchedValue(dataFetcherResult.getData())
          .errors(dataFetcherResult.getErrors())
          .localContext(localContext)
          .build();
    } else {
      return FetchedValue.newFetchedValue()
          .fetchedValue(executionContext.getValueUnboxer().unbox(result))
          .rawFetchedValue(result)
          .localContext(parameters.getLocalContext())
          .build();
    }
  }
}

This will allow you to annotate any datafetcher function with either @GraphQLDefault(DefaultType.LIST) or @GraphQLDefault(DefaultType.OPTONAL). That way you can throw exceptions inside your code as needed, let the default or custom data fetcher exception handling handle turning the exceptions into GraphQL errors and then let the custom execution strategy above handle the null so it doesn't propagate up.

[MazizEsa]

Thank you so much.
Got it.

[MazizEsa]

@swargolet Another option (seem workaround, but I notice default implementation uses it to trigger exception and others) is https://netflix.github.io/dgs/advanced/instrumentation/.

Based on the instrumentation we can change the structure of what to include in the data or error.