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

Custom errors - chaining breaks default functionality #2158

Closed
rvy-brillio opened this issue Sep 29, 2019 · 10 comments
Closed

Custom errors - chaining breaks default functionality #2158

rvy-brillio opened this issue Sep 29, 2019 · 10 comments
Assignees
Labels
documentation Non-code related changes support Questions, discussions, and general support

Comments

@rvy-brillio
Copy link

rvy-brillio commented Sep 29, 2019

Hi team, really appreciate the effort that has been into this validator to make it so dynamic and easy to use.

Describe the problem you are trying to fix (provide as much context as possible)

I'm using 16.1.4 version of the validator.

The problem I'm facing is with custom error messages. I've gone through #1739, and that did not help (in the sense that it does not accomplish the functionality for multiple keys).

My schema is:

const joiSchema = Joi.object({
  a: Joi.string()
    .error(new Error('a should be a type of text'))
    .min(2)
    .error(new Error('a should be a min of 2'))
    .max(10)
    .required(),
  b: Joi.string()
    .error(new Error(`b should be a type of text`))
    .valid('INBOUND', 'OUTBOUND')
    .error(new Error(`Incorrect input provided for field: b`))
    .min(1)
    .error(new Error(`b should at least have a length of 1`))
    .max(100)
    .required()
});

My validation code is: joiSchema.validate(payload, { abortEarly: false });

and my input payload is:

{
  "a": 1,
  "b": 2
}

Expected result:
{ message: "a should be a type of text, b should be a type of text" }

Actual result:
{ message: "a should be a min of 2" }


There are 3 concerns here:

  1. Only the last error is thrown. While validating, either all the failure messages should be returned (in this scenario, 'a should be a type of text, a should be a min of 2'), or at least the first validation failure (in this scenario: 'a should be a type of text').
  2. Validation does not move to other keys: Since the option abortEarly is set to false, the validation should proceed and additionally provide the error for 'b'. And similar to 'a', it should show at least the first encountered error, or all the errors encountered.
  3. Default messages are skipped when custom messages are used: When using custom message for one of the validation, let's say type check (string), and not using custom message for other validation, let's say .required(), the default message of other validation never generates. So, in this case, if 'a' is not even passed, and .required() does not have a custom message, the expected message: 'a is required' never appears.

I did a little test, and found out that if I use the custom error feature (any.error), even once anywhere, then it breaks the functionality.

By functionality I mean that:

  • abortEarly: false is ignored. Other schema keys are not validated.
  • Even for the same key ('a'), other non-custom errors are not considered. So, if I remove 'a' from input payload, then .required should trigger (which is not happening) and the error message should be '"a" is required.' (since this is the joi-defined error for any.required).

and by even once I mean:

const joiSchema = Joi.object({
  a: Joi.string()
    .error(new Error('a should be a type of text'))
    .min(2)
    .max(10)
    .required(),
  b: Joi.string()
    .valid('INBOUND', 'OUTBOUND')
    .min(1)
    .max(100)
    .required()
});

In this changed schema, the custom error is used only once.

Which API (or modification of the current API) do you suggest to solve that problem ?

I'm not sure yet, but I can look around the code if required.

Are you ready to work on a pull request if your suggestion is accepted ?

Yes (but it might take some time, as I've barely gone through the code yet).

@triron2018
Copy link

triron2018 commented Sep 30, 2019

what version of @hapi/joi are you using ?
I'm using 16.1.4 it gives NO Errors ; below is the output

{
"value": {
"a": 1,
"b": 2
},
"error": {}
}

@rvy-brillio
Copy link
Author

rvy-brillio commented Sep 30, 2019

Sorry forgot to mention that. I'm also using the same 16.1.4. (I've updated the ticket with a little more details)

@hueniverse hueniverse self-assigned this Oct 2, 2019
@hueniverse hueniverse added documentation Non-code related changes support Questions, discussions, and general support labels Oct 2, 2019
@hueniverse
Copy link
Contributor

You are using the wrong method. .error() is not meant to override error messages. I've updated the documentation.

@hugebdu
Copy link

hugebdu commented Oct 2, 2019

@hueniverse trying to use .message() however getting an error:

Error: Cannot apply rules to empty ruleset or the last rule added does not support rule properties
    at new module.exports (/Users/daniels/sources/wix-node-platform/node_modules/@hapi/hoek/lib/error.js:21:15)
    at module.exports (/Users/daniels/sources/wix-node-platform/node_modules/@hapi/hoek/lib/assert.js:20:11)

this is my validation code:

result.valid(...LANGUAGE_CODES).message({'any.only': 'invalid language'})

@hugebdu
Copy link

hugebdu commented Oct 2, 2019

UPD: found how to tackle it with .prefs(...) looking into tests.
IMHO documentation should mention that for .valid and other non-ruleset based

@rvy-brillio
Copy link
Author

@hueniverse Thanks.

@hugebdu I tried this:

const joiSchema = Joi.object({
  a: Joi.string()
    .messages({ 'any.only': `a should be a type of 'text'` })
    .min(2)
    .max(10)
    .required(),
  b: Joi.string()
    .messages({ 'any.only': `b should be a type of 'text'` })
    .valid('I', 'O')
    .messages({ 'any.only': `b could either be 'I' or 'O'` })
    .min(1)
    .max(100)
    .required()
});

with input as:

{
    a: 1,
    b: 2
}

Expected output: "a should be a type of 'text'. b should be a type of 'text'"
Actual output: ""a" must be a string. "b" must be one of [I, O]. "b" must be a string"

Can you please tell me what I'm doing wrong? (I went through the documentation: https://hapi.dev/family/joi/?v=16.1.5#anymessagesmessages, but could not find what is the correct usage)

Additional details:
After looking at tests and some code, I've tried:

Joi.string()
    .prefs({
      messages: {
        english: { 'any.type': `a should be a type of 'text'` }
      }
    })

and

Joi.string()
    .prefs({
      messages: {
        root: `a should be a type of 'text'`
      }
    })

But they all seem to fail to respond with custom message.

@rvy-brillio
Copy link
Author

rvy-brillio commented Oct 4, 2019

Update:
Ok, so after going through more code/documentation and tests/examples, I figured that out.
The correct schema to do this is:

Joi.string()
    .min(2)
    .max(10)
    .required()
    .messages({
      'string.base': `"a" should be a type of 'text'`,
      'string.empty': `"a" cannot be an empty field`,
      'any.required': `"a" is a required field`
    }),

You can have the .messages() at any level (as it is any.messages()). My preference is to keep it all together. The only pain is to find the keys for messages, i.e., in this example, 'string.base', 'string.empty', 'any.required' etc. I'm sure there must be a repo for all of these, but my current approach was to let my validation fail and find what error is returned. In the error object, the type is specified for the failure, then use that type in schema (under messages).

For not having to hard-code values, like in min/max, you can also use references:

    'string.min': `"a" should have a minimum length of {#limit}`

Output: ""a" should have a minimum length of 2."

Thanks @hueniverse and @hugebdu.

@hueniverse
Copy link
Contributor

The documentation is pretty detailed about which error codes go with which method.

@rvy-brillio
Copy link
Author

Yes @hueniverse. I found the list of errors (along with description) here: https://github.com/hapijs/joi/blob/master/API.md#list-of-errors. Thanks again.

justsml added a commit to justsml/joi that referenced this issue Oct 22, 2019
* 'master' of github.com:hapijs/joi:
  Update API.md
  Improve compile version conflict error message. Closes hapijs#2173
  Closes hapijs#2172
  Fix docs malformed code block ending at section `object.pattern.match`
  End code block
  Fix docs missing code block ending at section `date.less(date)`
  Fix function signature. Fixes hapijs#2170.
  Delete .npmrc
  Delete .editorconfig
  Delete feature_request.md
  Delete bug_report.md
  Delete CONTRIBUTING.md
  16.1.7
  Fix date format validation. Closes hapijs#2168
  16.1.6
  Closes hapijs#2165
  16.1.5
  Clarify error(). Closes hapijs#2158
  Closes hapijs#2161
  Fix handling of shadow values. Closes hapijs#2156
@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
documentation Non-code related changes support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

4 participants