-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error's path on validation errors #166
Comments
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 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 If it was just some other mutation we could rescue from But using |
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, {
: 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. |
Actually the GQL gem's docs say something about it, maybe this is the best approach https://graphql-ruby.org/mutations/mutation_errors |
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 Also, I'd expect the |
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. |
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 |
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:
"path": ["userSignUp"]
. Would be great to have"path": ["userSignUp", "email"]
rescue_from ActiveModel::ValidationError { ... }
in the schema doesn't seem to be catching this.What's the proper way to do it?
Thanks!
The text was updated successfully, but these errors were encountered: