Skip to content
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

Add basic error overriding #1131

Merged
merged 1 commit into from
Apr 1, 2017
Merged

Add basic error overriding #1131

merged 1 commit into from
Apr 1, 2017

Conversation

Marsup
Copy link
Collaborator

@Marsup Marsup commented Mar 20, 2017

Hey y'all,

Had some free time this week-end, I've been toying in my head with this kind of API for while now, it allows you to override errors. This PR is essentially a reality check to see if it can be useful to some people.

Now, with that said, you asked for it, but here be dragons ! You'll be dealing with the full stack of errors joi is usually manipulating for you to give you those so called ugly errors, you'll have to work a bit to make your own messages, so don't complain if it's too complex (unless you have better ideas), there's no free lunch :)

Roles are now reversed, I'll let you do the review and see where we go from here.

@Marsup
Copy link
Collaborator Author

Marsup commented Apr 1, 2017

Well, so much silence... I'll merge anyway.

@Marsup Marsup merged commit e67c59e into master Apr 1, 2017
@Marsup Marsup deleted the e branch April 1, 2017 18:44
This was referenced Apr 1, 2017
@mattapperson
Copy link

When using this it seems to break the abortEarly option? error returned by validate is just the one error when using this vs being added within err.details

@mattapperson
Copy link

Edit: This is broken when validating an object of keys, it fails as if validating just like a string for example

@Marsup
Copy link
Collaborator Author

Marsup commented Nov 16, 2017

Mind providing an example ?

@mattapperson
Copy link

Sure! So with the following code, not using .error()

Joi.validate(values, {
    name: Joi.string().required().max(10),
    confirmName: Joi.string().required().valid(Joi.ref('name'))
}, {
     abortEarly: false
}, (err, value) => {
     console.log(JSON.stringify(err, false, 2))
})

I get the following in the error of the callback:

{
  ...
  "isJoi": true,
  "name": "ValidationError",
  "details": [
    {
      "message": "\"name\" length must be less than or equal to 10 characters long",
      "path": "name",
      "type": "string.max",
      "context": {
        "limit": 10,
        "value": "matthghgjhg",
        "key": "name"
      }
    },
    {
      "message": "\"confirmName\" is required",
      "path": "confirmName",
      "type": "any.required",
      "context": {
        "key": "confirmName"
      }
    }
  ],
  "_object": {
    "name": "matthghgjhg"
  }
}

however if I use the following:

Joi.validate(values, {
    name: Joi.string().required().max(10),
    confirmName: Joi.string().required().valid(Joi.ref('name')).error(new Error('Was REALLY expecting this to match your name'))
}, {
     abortEarly: false
}, (err, value) => {
     console.log(JSON.stringify(err, false, 2))
})

Then I simply get the js error "Was REALLY expecting this to match your name" returned without the "isJoi":, "name" or "details" keys, and the error is just for the confirmName field

@Marsup
Copy link
Collaborator Author

Marsup commented Nov 16, 2017

That's unrelated to this PR and is working as documented. An actual error will be served as is.

@mattapperson
Copy link

@Marsup so it is expected to not be able to get all errors from an object if one field has a custom error?

@Marsup
Copy link
Collaborator Author

Marsup commented Nov 16, 2017

When using an actual Error, yes.

@mattapperson
Copy link

Hmmm, would you be open to considering a PR to change that? Or else would you mind explaining why this should be as it is because to me this felt very unexpected. I would only expect that if abortEarly was set to true.

@Marsup
Copy link
Collaborator Author

Marsup commented Nov 16, 2017

Not really. Because it's inconsistent and often impossible in several scenarios to combine several types of errors. It's mostly used to throw boom errors anyway. If you don't want that just use the other forms.

@hueniverse hueniverse added support Questions, discussions, and general support and removed discussion labels Sep 19, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement support Questions, discussions, and general support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants