What is the recommended approach for business logic errors from GraphQL?

[ryancole]

I've seen this asked before, but I have a particular concern about the recommended approach that I had seen. The question being: how do I get meaningful errors, from GraphQL, to the UI in such a way that I can act upon them (highlight an input red, etc)? Without Relay in the picture, it's pretty straight forward - you can pretty much do whatever you like. Relay needs some additional thought.

I had seen a recommendation that this type of information should be thought of as data and that it should be present in the payload of a query, as opposed to thrown as an Error and thus included in the errors array from GraphQL. This recommendation brought up additional questions for me, though.

1. Specifically, when dealing with Relay, if a mutation fails due to some failed input business logic checks on the GraphQL side of things but instead of throwing an Error we include the failure information as part of a successful payload, won't that circumvent a lot of the built in Relay mutation failure logic? Since we're not throwing errors, we're not telling Relay that the mutation failed.
2. Wouldn't that require us to code a lot of that failure logic into our code, even when Relay already handles it all for us if we were to just throw an Error? Such as adding didFail fields and error message fields to the payload.
3. Wouldn't we have to also make sure to return the original, unchanged data in the mutation response payload so that Relay doesn't update its own local store with invalid data?
4. Now, every mutation needs a REQUIRED_CHILDREN config to explicitly capture the error-related fields, right?

All of this seems like pain that can be avoided if we just make use of the already dealt with errors field from GraphQL. Relay already sees this as a failed mutation and does not update the store. Relay already provides a nice hook to make UI updates in response to a failure.

So, why would we not just use that then? In my recent experience, I've seen the suggestions like that I mentioned at the top of this, which pointed me away from the errors array. There's also that mention in the GraphQL spec that the errors array is meant for code debugging related messages. These are the two reasons why I even began to think about putting error messages in the mutation payloads.

Here's what I'm currently doing in my code. Maybe you can tell me if this is bad. It involves mostly a tweak on GraphQL side of things, but solely to enable a pattern on the Relay and React side. I've created my own custom formatError function that checks for the existence of a fieldName property on the error. If the error has that property, it includes it as part of the object that is subsequently includes in the errors array. Here's that custom formatError code ...

export default function formatError(error) {
  let result = {
    message: error.message,
    locations: error.locations
  };
  if (error.originalError && error.originalError.fieldName) {
    result.fieldName = error.originalError.fieldName;
  }
  return result;
};

Now, with this in place on GraphQL side, I can just throw an error that has this property from inside my mutation code and I'll be able to provide back to Relay an error message AND a field name that the error relates to. On the Relay side, I get to continue using the onFailure callbacks and rest assured knowing that I'm not messing up the store, etc. I can now map specific error messages to specific fields in React/Relay land. Only annoyance is that this all hinges on no typos or anything when throwing these errors (its just strings).

Are my initial concerns anything to actually be concerned about? Is the way I'm doing this good or bad? I'm mostly just lost without a clear direction as to which approach I truly should be striving to use, I guess.

[josephsavona]

This is a great question.

@dschafer probably has the best context on error handling in GraphQL. Dan, any resources on this?

[Globegitter]

Now, with this in place on GraphQL side, I can just throw an error that has this property from inside my mutation code and I'll be able to provide back to Relay an error message AND a field name that the error relates to. On the Relay side, I get to continue using the onFailure callbacks and rest assured knowing that I'm not messing up the store, etc. I can now map specific error messages to specific fields in React/Relay land.

This is the road we have been thinking of going down as well. But how could you handle sending back errors for multiple fields at once? (e.g. form validation)

[ryancole]

But how could you handle sending back errors for multiple fields at once? (e.g. form validation)

At the moment, I don't know. It's obvious the spec allows for multiple errors, because it's an array of errors in the response. I'm not sure how to throw multiple errors and have them appear there, though. At the moment for me it's just a single error each request.

[NevilleS]

I was definitely interested in hearing some community thoughts on this one. I got asked about this a bunch at a recent React meetup and felt like I didn't have a great answer... pinging @dschafer again?

[dschafer]

Ah, sorry about the delay, this slipped through my inbox!

It's obvious the spec allows for multiple errors, because it's an array of errors in the response.

Yep, this is exactly why we allowed it that way. In fact, the reason we ensured that in the spec is because in the original implementation at FB (which we're moving away from), we could only throw one error, and we regretted it. That's also why the spec is incredibly unopinionated on what the shape of an error is; we knew we were changing how errors worked, and didn't want to express an opinion we hadn't tried ourselves.

So that's both good news and bad news. The good news is, we definitely allow and encourage this. The bad news is, I don't really have any recommendations backed by experience.

Based on your post above, what you're doing looks very reasonable; I think returning a list of errors to indicate all of the reasons a mutation failed is exactly the right thing to do, and if you can annotate those errors with useful information to the client, all the better!

[leebenson]

Excellent question. I'm in the same boat.

Currently, I'm handling errors in GraphQL like so:

// This helper 'wraps' a GraphQL schema object and catches `boom` and
// general purpose Error objects so that validation errors can be
// properly reported to the client

import Sequelize from 'sequelize';

// import GraphQL
import * as g from 'graphql';

// import exceptions. We'll use these to determine what type of
// GraphQL error we're dealing with, so we know whether to propagate
// it up to the client
import * as ex from '../exceptions';

// Mark field/type/schema
export const Processed = Symbol();

// Used to identify UserErrors
export const IsUserError = Symbol();

// The default handler that is called to determine what kind of error
// we're looking at.  If it's a validation error (a `Map`), we know to send
// it back to the user as an object of form fields -> error messages.
// Otherwise, we'll read the statusCode and respond appropriately.
const defaultHandler = function (e) {

  switch (e.constructor) {
    case Sequelize.ValidationError:
      return (new ex.SequelizeError(e)).mapToObject();
    case ex.ValidationError:
      return e.mapToObject();
    case ex.DatabaseError:
      return 'Database error';
    case ex.NotAuthorised:
      return 'You are not authorised';
    default:

      // This is a general error that wasn't already captured.  So
      // we'll create a new `AppError`, which will implicitly log
      const err = new ex.ErrorToException(e);
      err.log();
      return 'General error';
  }
};

// Unexported helper function for masking `fields`
function maskField(field, fn) {
  const resolveFn = field.resolve;
  if (field[Processed] || !resolveFn) {
    return;
  }

  field[Processed] = true;
  field.resolve = async function (...args) {
    try {
      return resolveFn.call(this, ...args);
    } catch (e) {
      const newError = new Error();
      newError.message = fn(e);
      throw newError;
    }
  };

  // save the original resolve function
  field.resolve._resolveFn = resolveFn;
}

// Unexported helper function for masking `type`
function maskType(type, fn) {
  if (type[Processed] || !type.getFields) {
    return;
  }

  const fields = type.getFields();
  for (const fieldName in fields) {
    if (!Object.hasOwnProperty.call(fields, fieldName)) {
      continue;
    }

    maskField(fields[fieldName], fn);
  }
}

// Unexported helper function for masking `schema`
function maskSchema(schema, fn) {
  const types = schema.getTypeMap();
  for (const typeName in types) {
    if (!Object.hasOwnProperty.call(types, typeName)) {
      continue;
    }

    maskType(types[typeName], fn);
  }
}

// Exported function that masks graphql schemas, types or individual fields,
// using the helper methods specified above.  An optional second paramater
// can override the default error handler function
export function maskErrors(thing, fn = defaultHandler) {
  if (thing instanceof g.GraphQLSchema) {
    maskSchema(thing, fn);
  } else if (thing instanceof g.GraphQLObjectType) {
    maskType(thing, fn);
  } else {
    maskField(thing, fn);
  }
}

The defaultHandler 'catches' the error/exception thrown, determines its type - a vanilla Error object, something slightly more exotic like a custom exception (that may in turn have logging, etc), or a ValidationError, which is hashed as 'field -> error message' - and then either hides it from public view under the 'General Error' banner, or mutates error to be an object of field/error pairs.

Relay, in turn, can look at the error that's thrown back and determine if it's an object or a plain error. If it's an object, I propagate those errors back to a MobX store and use a form helper outside of Relay to respond to state changes in the UI. As far as Relay is concerned, the mutation failed and no further changes are needed sync the local store. At the same time, error state is handled separately outside of Relay.

It's not ideal, IMO (not least because I now have to care about state in two places) although I think this is probably more for GraphQL proper to fix than Relay, per se. Instead of forcing one error response to be two things, it'd be nice if there was a distinction between error types thrown... e.g. something like throw new GraphQL.RequestError(object | map) to distinguish that an error is an arbitrary object, vs. any other kind which becomes the text string of the errors[].message field.

Relay, in turn, would then have an 'official' way to handle the distinction:

errors = system errors, syntax issues
requestErrors = user/validation stuff

This could somehow feed back to the Relay component and cause a re-render... maybe this.props.relay.requestErrors or similar... outside of the GraphQL response, but still handled in a first-class way by Relay.

[leebyron]

I'm sorry this issue has sat here for so long. In an attempt to clean up our issue queue we're closing some aging or unclear-action issues.

Sounds like there are some good answers above. In general at Facebook we use typical data in GraphQL to represent different product considerations, while errors typically mean that something has gone wrong on the server.