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

Validation with custom followed by withMessage always uses thrown message. #1047

Closed
JHawkley opened this issue Jun 9, 2021 · 2 comments · Fixed by #1049
Closed

Validation with custom followed by withMessage always uses thrown message. #1047

JHawkley opened this issue Jun 9, 2021 · 2 comments · Fixed by #1049
Assignees
Labels

Comments

@JHawkley
Copy link
Contributor

JHawkley commented Jun 9, 2021

Describe the bug

There seems to be a regression of #203 from the v2 days or the documentation for withMessage is incorrect.

If you try to override the default message of a custom validator using withMessage, it ignores the provided message.

According to the documentation of withMessage:

.withMessage(message)
Sets the error message for the previous validator.
This will have precedence over errors thrown by a custom validator.

And, according to the Dynamic Messages guide:

If you're using a custom validator, then it may very well throw or reject promises to indicate an invalid value.
In these cases, the error gets reported with a message that's equal to what was thrown by the validator:

...which a colleague initially read as it will always use the message from the error. So, we're not sure which it is supposed to be and it's been so long since #203 that it's hard to say if the intended behavior had been altered since then.

But having withMessage override a default error message from a custom validator to provide better context is a handy tool to have in a validation library.

To Reproduce

A partial reproduction:

const { body, validationResult } = require('express-validator');

const customValidation = () => {
  throw new Error("default message");
};

router.post("/test", [
  body("someValue")
    .custom(customValidation).withMessage("custom message"),
  (req, res) => {
    try {
      validationResult(req).throw();
      res.send("OK");
    }
    catch (err) {
      // Expect to find "default message" in the output for `someValue`.
      console.log(err.mapped());
      res.status(400).send("FAIL");
    }
  }
]);

Expected behavior

I would like it to use the custom error message provided by withMessage
...or the documentation updated to be more clear about how custom validator error messages are intended to be presented.

Current behavior

Always uses the thrown error message; checked in code for src/context-items/custom-validation.js and it indeed preferentially uses err.message or err over this.message when catching an exception from the custom validator.

Express-validator version:

  • Version: Discovered on 6.10.1, verified that it still affects 6.11.1
@fedeci
Copy link
Member

fedeci commented Jun 10, 2021

Thanks for the report, would you like to work on this?

@JHawkley
Copy link
Contributor Author

I can probably submit a quick PR for review this weekend. Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants