graphql_devise icon indicating copy to clipboard operation
graphql_devise copied to clipboard

Error's path on validation errors

Open l3x4 opened this issue 3 years ago • 7 comments

Question

For validation errors it will be very helpful to specify the field where the validation failed (to highlight it on the form for example).

Currently the situation looks like this:

  1. The path contains just the mutation name, for example for the empty email it's like so: "path": ["userSignUp"]. Would be great to have "path": ["userSignUp", "email"]
  2. The attempt to build the error manually with rescue_from ActiveModel::ValidationError { ... } in the schema doesn't seem to be catching this.

What's the proper way to do it?

Thanks!

l3x4 avatar Apr 14 '21 13:04 l3x4

Hey @l3x4! So yes, the error won't come in the path and I'm not sure if we want that level of detail for the arguments or if even if that is a part of the GQL (I'll check). What we do right now for ActiveRecord errors is send all validation errors in an array as part of the extension key. Take a look at this spec so you can see how the errors are returned in the response.

mcelicalderon avatar Apr 14 '21 17:04 mcelicalderon

@mcelicalderon Thanks for the explanation!

I did have a look at that spec, and tried it out myself on my setup too.

Yes, there's a message that the email is missing for example, but there's no signs even in the extensions what field is it. It seems impossible to automatically tell the empty email error from mismatching passwords error.

If it was just some other mutation we could rescue from ActiveModel::ValidationError and analyse error.model.errors, building GraphQL::ExecutionError with the path containing the attribute and adding it to the context.

But using GraphqlDevise::SchemaPlugin this approach doesn't seem to work.

l3x4 avatar Apr 14 '21 18:04 l3x4

Mmm, you mean something like this:

{
  : errors => [{
    : message => "Argument 'email' on Field 'userSignUp' has an invalid value (1). Expected type 'String!'.",
    : locations => [{
      : line => 2,
      : column => 11
    }],
    : path => ["mutation", "userSignUp", "email"],
    : extensions => {
      : code => "argumentLiteralsIncompatible",: typeName => "Field",: argumentName => "email"
    }
  }]
}

I passed the wrong type to the mutation just to try it, but I guess doing the same for invalid data would be possible. I see how right now the only way to display the errors is on a list, separate from the fields that originated the arguments.

I can think of an easy way to implement something that would help identify the arguments, detailed_errors could look like this:

{
  : data => {
    : userSignUp => nil
  },: errors => [{
    : message => "User couldn't be registered",
    : locations => [{
      : line => 2,
      : column => 11
    }],
    : path => ["userSignUp"],
    : extensions => {
      : code => "USER_ERROR",: detailed_errors => {
        : email => ["can't be blank"]
      }
    }
  }]
}

That would be simple to change, of course we would have to make it optional via an initializer, maybe eventually the default. These error keys come from active_record, so they might not match the arguments in the request, depending on the validations you used.

I will check if there's a way to customize the path on the returned errors, but even then I think it might be hard to match with the actual arguments of the mutation instead of the error keys from active record.

mcelicalderon avatar Apr 15 '21 03:04 mcelicalderon

Actually the GQL gem's docs say something about it, maybe this is the best approach https://graphql-ruby.org/mutations/mutation_errors

mcelicalderon avatar Apr 15 '21 03:04 mcelicalderon

Yep, that's this part in the doc, which adds the attributes to the path:

def resolve(id:, attributes:)
  post = Post.find(id)
  if post.update(attributes)
    {
      post: post,
      errors: [],
    }
  else
    # Convert Rails model errors into GraphQL-ready error hashes
    user_errors = post.errors.map do |attribute, message|
      # This is the GraphQL argument which corresponds to the validation error:
      path = ["attributes", attribute.to_s.camelize(:lower)]
      {
        path: path,
        message: message,
      }
    end
    {
      post: post,
      errors: user_errors,
    }
  end
end

And the result is:

{
  "data" => {
    "createPost" => {
      "post" => nil,
      "errors" => [
        { "message" => "Title can't be blank", "path" => ["attributes", "title"] },
        { "message" => "Body can't be blank", "path" => ["attributes", "body"] }
      ]
    }
  }
}

The path is not exactly as I mentioned above (["userSignUp", "email"]), but the variant from documentation works perfectly well, specifying which attribute had the error.

Also, I'd expect the errors to be on the same level as data, but maybe I'm missing some concepts here.

l3x4 avatar Apr 15 '21 07:04 l3x4

Yes, the author of the gem recommends these kind of errors to be at the same level as data and not make them top level errors, but not sure if that's his personal preference or if that's actually somewhere in the GQL spec. We are still discussing with @00dav00 how to move forward on this one, but we'll definitively implement something that allows to identify the field of the error. Thank you for bringing this up.

mcelicalderon avatar Apr 16 '21 03:04 mcelicalderon

Just as an update, we have decided to implement mutation level errors field, details still under discussion, but we will probably add a setting to disable top-level errors so you can actually query the new errors field.

mcelicalderon avatar May 09 '21 16:05 mcelicalderon