NonNullable List returns null on field error

[ebenoist]

The return type for a non-nullable list is inconsistent with schema when a field error is present. This makes guaranteeing the type via the non-nullable contract inconsistent.

Given the following schema, it is my understanding that I should always receive an array typed object (not a null).

type Image {
  url: String!
}

type Product {
  images: [Image]!
}

images = [] // valid
images = null // invalid
images = [null] // valid

However, if the resolver for images encounters an error, the returned type is Null.

Recieved

{
  "errors": [
    {
      "message": "'foozles' is not a supported image transformation.",
      "path": [
        "listing",
        "images"
      ],
      "extensions": {
        "code": "CLIENT_ERROR"
      }
    }
  ],
  "data": {
    "listing": {
      "images": null
    }
  }
}

This is explicitly tested for in execution/tests/lists-test.js, but seems inconsistent with the declared type contract.

Expected

{
  "errors": [
    {
      "message": "'foozles' is not a supported image transformation.",
      "path": [
        "listing",
        "images"
      ],
      "extensions": {
        "code": "CLIENT_ERROR"
      }
    }
  ],
  "data": {
    "listing": {
      "images": []
    }
  }
}

Non-nullable types (for certain fields) allows a higher level of type safety and is especially useful while handling partial failures.

Is this behavior intended? Is there a way to support the behavior I would have expected given the current type system and error handling behavior? There is a reference to handling this in the parent field in the spec, but I'm unsure that would be consistently implemented in my resolver or parent resolver?

[yaacovCR]

Try:

type Product {
  images: [Image!]!
}

Note the inner exclamation point

:)

[ebenoist]

@yaacovCR That's not issue I am running into. Changing this to [Image!]! makes the following invalid [null], but doesn't fix the issue of allowing null to be returned when images returns an error.

This issue is specifically the interaction with this line of code:

function handleFieldError(rawError, fieldNodes, path, returnType, exeContext) {
  const error = locatedError(
    asErrorInstance(rawError),
    fieldNodes,
    pathToArray(path),
  );

  // If the field type is non-nullable, then it is resolved without any
  // protection from errors, however it still properly locates the error.
  if (isNonNullType(returnType)) {
    throw error;
  }

  // Otherwise, error protection is applied, logging the error and resolving
  // a null value for this field if one is encountered.
  exeContext.errors.push(error);
  return null;
}

[yaacovCR]

Sorry, read too fast!

[danielrearden]

If images has a non-null type and an error is thrown inside its resolver, then its parent field will instead return null because images can never be null. If the parent also has a non-null type, then its parent will be null, and so on until we hit either a nullable field or data (which is always nullable).

In other words, assuming listing is nullable, if an error is thrown inside images, the expected response would be { data: { listing: null } }. If listing is also non-null, the expected response would be { data: null }.

images returning null when it's a non-null field would be a spec-breaking behavior and a bug. However, I cannot reproduce this behavior on my end.

If you want images to return an empty array when an error is encountered during the execution of its resolver, you'd need to wrap the resolver logic in a try/catch and return the empty array inside the catch block.

@ebenoist I'm closing this issue since I can't seem to reproduce it. If you are still seeing this behavior, it would be great if you could provide a code snippet or code sandbox to reproduce it. If you have additional questions about GraphQL, please post them on SO since we use issues only for tracking bugs and feature requests.