Validations for multiple fields in Apollo Server 2.0

[vespertilian]

Thanks for adding better error handling to V2. I have just run into some issues and wanted to share my thoughts and get some feedback from the community.

Current v2 errors documentation from the website

v2 errors documentation

const { 
  ApolloServer,
  BadUserInputError,
  gql,
} = require('apollo-server');

const typeDefs = gql`
  type Mutation {
    userInputError(input: String): String
  }
`;

const resolvers = {
  Mutation: {
    userInputError: (parent, args, context, info) => {
      if (args.input !== 'expected') {
        throw BadUserInputError('Form Arguments invalid', {
          invalidArgs: Object.keys(args),
        });
      }
    },
  },
};

A more complex case

The above solution works for a single variable mutation resolver, as soon as you have a multi variable input if falls down a bit. It would be nice if your documented example included what to do with multiple variables.

The issue I noticed is that with the current system you can only return one error message object from the resolver, I feel that it would be nice if you could return multiple BadUserInputErrors.

Something like this link

const { 
  ApolloServer,
  BadUserInputError,
  gql,
} = require('apollo-server');

const typeDefs = gql`
  type Mutation {
    userInputError(input: {name: String, age: Int}): String
  }
`;

const resolvers = {
  Mutation: {
    userInputError: (parent, {input}, context, info) => {
      const {name, age} = input;

      // The issue with the current solution is that that age will never get 
      // checked if there is an error on name as the flow will stop as soon as we throw
      // if (name.length < 3 ) {
      //   flow stops here
      //   throw BadUserInputError('Name is less than 3 characters', { invalidArgs: "name" });
      // }

      // Potential solution, have some type of errors array we can throw. 
      let errors = []
      if (name.length < 3 ) {
        errors.push(new BadUserInputError('Name is less than 3 characters', { invalidArgs: "name" }))
      }
      if(age < 18) {
        errors.push(new BadUserInputError("Age is less than 18"), { invalidArgs: "age"})
      }

      if(errors.length > 0) {
        // this does not currently work as it just ends up as one error with the messages concatenated
        throw errors
      }

      // We could also just append args and error message together 
      // It's just nicer IMHO to have the seperate errors so if you were 
      // going to show them to a user its easier to match them to ui fields 
      const combinedErrors = new BadUserInputError("Invalid Name, Invalid Age", {invalidArgs: ["Name", "Age"]})
    },
  },
};

If you allowed the resolver to return multiple independent errors, this would then be consistent with how validation errors (like required) are currently returned, as each field is returned as it's own error.

Current validation errors sample

{
  "errors": [
    {
      "message": "Field CreateCoreProfileInput.emailAddress of required type String! was not provided.",
      "locations": [
        {
          "line": 3,
          "column": 9
        }
      ],
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED"
      }
    },
    {
      "message": "Field CreateCoreProfileInput.biography of required type String! was not provided.",
      "locations": [
        {
          "line": 3,
          "column": 9
        }
       ],
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED"
      }
     }
   ]
}

Also maybe the validation errors could have a "invalid arg" property added, to better match BadUserInputError?

One more thing to note which I have brought up here is that validation errors happen before the resolver is called, so if you have missing fields and validation errors you have to do a minimum of 2 requests even if the validation error is not on a missing field. This may be the desired outcome, it's just not obvious so it might be worthwhile documenting.

Happy to help out in any way I can if you know which direction this is going to go.

[evans]

@vespertilian This could be possible with a GraphQL extension that takes a combine error inside of the GraphQL response and splits it up multiple errors in the errors array. The current formatError function is created with this in mind

apollo-server/packages/apollo-server-core/src/formatters.ts

Line 14 in 1947892

public willSendResponse(o: {

. It currently only returns a single error, since it's use case is masking errors. It could split errors up, however that would mean that reporting would not get the enriched errors, since it occurs before formatError.

We're currently documenting the extensions and this seems like a great use case!

[vespertilian]

@evans

Thank's for getting back to me.

What do you mean by a masking error?

I am not sure I fully grasp your solution yet, but I am keen to try this out as soon as you document it, and as long as I can throw multiple validation errors for inputs and ideally have them look similar to the inbuilt graphql validation errors that would be a huge win.

Thanks you!

[gengjiawen]

Looks like now support for multi error return is not supported now. Maybe apollo-server should consider this as a basic feature.

[treybrisbane]

It sounds like a potentially better solution to this problem would be the ability to supply a custom validation function for input types, no?

Something like this:

// Adapted from `GraphQLTypeResolver` in `graphql/type/definition.d.ts`
export type GraphQLTypeValidator<TSource, TContext> = (
    value: TSource,
    context: TContext,
    info: GraphQLResolveInfo
) => MaybePromise<GraphQLError[]>;

// Copy-pasted from `graphql-tools/dist/Interfaces.d.ts`
export interface IResolverOptions<TSource = any, TContext = any> {
    fragment?: string;
    validate?: GraphQLTypeValidator<TSource, TContext>; // Added this
    resolve?: IFieldResolver<TSource, TContext>;
    subscribe?: IFieldResolver<TSource, TContext>;
    __resolveType?: GraphQLTypeResolver<TSource, TContext>;
    __isTypeOf?: GraphQLIsTypeOfFn<TSource, TContext>;
}

The idea being that the validate function (if supplied) is called after GraphQL's builtin validation, but before the resolve function is called. The resolve function would only be called if validate was not supplied, or was supplied and returned [].

Thoughts?

[vespertilian]

@treybrisbane

I think just being able to return an array of errors from your resolver would be the easiest.

@evens

I just came back to this. I tried to look into format error but the 2 errors just appear in the message extracted already. So I don't believe there is an easy way to throw multiple validation errors. I agree with @gengjiawen, that it would be great if you would consider adding this as a core feature.

I get something like this out of formatError:
message: "UserInputError: This email is already associated with an account. "

• you can see the UserInputError is treated as the message, I returned it as an array from the resolver.

Is there some way you could add some type of check to the code that catches the error, something like:

if(typeof errors = Array) {
 errors = [...errros, ...anyOtherErrors]
} else {
 errors = errors
}

For even more context here is the GraphQL playground output where I return three Apollo UserInputErrors from my CoreProfile creation function

• emailAddress
• timezone
• currency

You can see how that are concatenated together in the message field.

Thanks for all your hard work!

[vadimshvetsov]

I've searched a lot about valid implementation of error response to the client and I'm still trying to dig deeper. What I've learned at this point:

• When we process input validation we can return only 1 error for 1 field. Often 1 input is 1 field and we don't need to add multiple errors to the same field resolver.
  https://facebook.github.io/graphql/draft/#sec-Errors-and-Non-Nullability
  From GraphQL spec: only one error should be added to the errors list per field
• The only one GraphQL library where I was able to find example of handling multiple validation errors is graphql-ruby. The approach is in treat with errors as data and define types. @leebyron accepted this approach and opened an issue for providing an example for Best practice section at graphql.org
• The most approved StackOverflow answer about multiple validation is also about treating error as data

The main question for me with apollo-server is:
Why we should use UserInputError when we need to treat with errors as a data?
graphql/graphql-spec#135 (comment)
graphql/graphql-spec#117 (comment)

[steida]

I put fields errors into schema itself because I believe it belongs there.

https://github.com/este/este/blob/master/server/api/model.graphql#L28

Maybe I am wrong, but I don't know what's the point of UserInputError.

[vespertilian]

This is where I have got to with my solution.

From this code

import { Txn } from "dgraph-js";
import { UserInputError } from "apollo-server-micro";

export async function validateInput(
  { timezoneName, currencyCode, emailAddress }: any,
  txn: Txn
): Promise<{ timezoneUid: string | null; currencyUid: string | null }> {
  const query = `
    query timezoneAndCurrency($timezoneName: string, $currencyCode: string, $emailAddress: string){
      timezone(func: eq(timezoneName, $timezoneName)){
        uid
      }
      currency(func: eq(currencyCode, $currencyCode)) {
        uid
      }
      {
      email(func: eq(emailAddress, $emailAddress)) {
         uid
      }
    }
  }`;

  const queryVars = {
    $timezoneName: timezoneName,
    $currencyCode: currencyCode,
    $emailAddress: emailAddress
  };

  const inputQuery = await txn.queryWithVars(query, queryVars);
  const { timezone, currency, email } = inputQuery.getJson();
  const [timezoneUid, timezoneError] = uidFound(
    timezone,
    "Invalid timezoneName submitted. "
  );
  const [currencyUid, currencyError] = uidFound(
    currency,
    "Invalid currencyCode submitted. "
  );

  let errors = [];
  if (timezoneError) {
    errors.push(
      new UserInputError(timezoneError, { invalidArgs: ["timezone"] })
    );
  }

  if (currencyError) {
    errors.push(
      new UserInputError(currencyError, { invalidArgs: ["currency"] })
    );
  }

  if (email.length > 0) {
    errors.push(
      new UserInputError("This email is already associated with an account. ", {
        invalidArgs: ["emailAddress"]
      })
    );
  }

  if (errors.length > 0) {
    // this is the key part
    const error = new UserInputError('Error on user input', errors);
    throw error;
  }

  return { timezoneUid, currencyUid };
}

function uidFound(value: [{ uid: string }], errorMessage: string) {
  if (value[0] && value[0].uid) {
    return [value[0].uid, null];
  } else {
    return [null, errorMessage];
  }
}

[jbaxleyiii]

👋 I'll close this since this doesn't appear to be a bug with Apollo Server, but rather a question about how to use it or one of its components.

Rather than asking it here in GitHub Issues — where efforts are focused on fixing bugs and adding new features — I'd ask that you take this question to the Apollo Server channel within the Apollo community on Spectrum.chat where there are community members who might be able to relate to a similar problem, or might be able to help you out more interactively. Thanks for your understanding!

[newlands89]

I was thinking I might just concatenate and serialize the message and dig it out using 'invalidArgs' as a key on fields with a matching name.

What really annoyed me is that 'error' seems to be one of these JS basket case objects which is an object but does not readout like an object (maybe just a chrome thing), I wanted to find the contents of 'invalidArgs' and to do so, you do this:

error.graphQLErrors[0].extensions.invalidArgs

Pretty dumb right? If anyone has a prettier way, please let me know.