jurassix / joi-validation-strategy

Joi validator for react-validation-mixin
MIT License
9 stars 14 forks source link

error.details doesn't always exist #9

Closed ammmze closed 8 years ago

ammmze commented 8 years ago

I'm having an issue when attempting to use the 'error' api

let schema = Joi.string().error(new Error('Was REALLY expecting a string'));
schema.validate(3);

When doing so, the decodeMessages.js throws the following error:

Uncaught TypeError: Cannot read property 'map' of undefined

It does this because error.details is something that joi adds internally when it creates the Error objects. But you may not always have it.

ammmze commented 8 years ago

Looks like other code is dependent on the details.message and details.path. Looks like I have to be sure to specify both of those for it to work.

Edit: I'm not sure there's another way to get that path though...so maybe its gonna just have to be required that we manually attach these details to the Error object.

jurassix commented 8 years ago

You can probably fix this by updating the collectErrors function to understand this new error object.

https://github.com/jurassix/joi-validation-strategy/blob/master/src/utils/collectErrors.js#L5

Throw in a failing test, turn it green, and I'll review it and merge it in. 👯

jurassix commented 8 years ago

Yeah I guess it might get more complicated, like you called out. We may need to also update the decodeMessages function to understand these new objects as well?

https://github.com/jurassix/joi-validation-strategy/blob/master/src/utils/decodeMessages.js#L5

If you are up for prototyping we can move this to a PR. Or if you can at least open a PR with a failing test added I'll try to extend to support your use case. Thx!

ammmze commented 8 years ago

I'm thinking we may need some changes upstream in order for this to fully work. I added a test that has a joi schema with 2 properties that should fail. One with a custom error and one with out.

it('should allow custom error messages', (done) => {
  const schema = {
    a: Joi.string().required(),
    b: Joi.string().required().error(new Error('Custom error'))
  };
  strategy().validate({}, schema, {}, errors => {
    console.log(JSON.stringify(errors));
    expect(errors).to.have.keys(['a', 'b']);
    done();
  })
});

When I log the errors object (after fixing decodeMessages to not blow up on missing details) we don't get anything in there. I think we really just need the upstream library to add the details array to custom errors. Basically by calling the error method in the joi schema, its setting that as the error for the entire schema.

jurassix commented 8 years ago

From the Joi docs here: https://github.com/hapijs/joi/blob/v9.0.4/API.md#anyerrorerr

Note that the provided error will be returned as-is, unmodified and undecorated with any of the normal joi error properties. If validation fails and another error is found before the error override, that error will be returned and the override will be ignored (unless the abortEarly option has been set to false).

Can you do something like this as a way around? (pulled from here: https://github.com/hapijs/joi/blob/master/lib/errors.js#L154)

It seems that the error object you are creating needs to satisfy our internal assumptions.

Something like this may work:

  const customJoiError = (message, path) => 
     const error = new Error(message);
     error.details = [];
     error.path = path;
     return error;
   }
  const schema = {
    a: Joi.string().required(),
    b: Joi.string().required().error(customJoiError('Custom error', 'b'))
  };
jurassix commented 8 years ago

yeah I guess the path field truly belongs on an Object in the details array.

I guess the next question is what is your desired result? A custom message?

jurassix commented 8 years ago

Custom messages can be defined as follows: https://jurassix.gitbooks.io/docs-react-validation-mixin/content/overview/custom.html

Although, I do like the conciseness the .error() api provides.

ammmze commented 8 years ago

Yea...that should work ... thanks for your help

jurassix commented 8 years ago

Awesome! Glad to help.