Problems with async await in registerAsync

[feeh27]

Environment info

Validatorjs version: 3.22.1 Node.js version: v12.18.3

Problem

I need the same functionality of issue #345, I want check in my database if the new value is unique, but the validation is not working as it should.

Implementation - Register Async

const Validator = require('validatorjs');
const services = require('../../index');
const NotFoundError = require('../errors/http-error');

Validator.registerAsync('unique', async (value, requirement, param, passes) => {
  if (typeof requirement !== 'string') {
    throw new Error('Invalid unique rule, you must enter the table!');
  }

  const args = requirement.split(',');

  const entityName = args[0];
  const field = args[1] || 'id';

  if (Object.keys(services).includes(entityName) === false) {
    throw new Error(`The entity ${entityName} doesn't exist.`);
  }

  const filters = {};
  filters[field] = value;

  /** @var {object} repository */
  const repository = services[entityName];

  try {
    await repository.getFirst(filters);
  } catch (error) {
    if (error instanceof NotFoundError) {
      passes(false, `The ${param} is already registered.`);
    }

    throw error;
  }

  await passes(true);
  return true;
});

module.exports = Validator;

Implementation - Validation

const Validator = require('../../lib/validatorjs'); // Import previous code 

const data = {
  name: 'Name',
  email: 'non-unique@email.com',
  password: 'password',
};

const rules = {
  name: 'required',
  email: 'required|unique:users,email',
  password: 'required',
};

const validation = new Validator(data, rules);

const passes = () => {};

const fails = () => {
  const message = JSON.stringify(validation.errors.all());
  throw new UserInputError(message);
};

await validation.checkAsync(passes, fails);

Expected behaviour

Wait for await validation.checkAsync(passes, fails); and throw an exception.

Current behavior

The code below is taken from Implementation - Register Async

passes(false, `The ${param} is already registered.`);`

When the logic of the code does not go through the above section everything happens as it should.

When the code logic goes through the above section, the code does not respect the await validation.checkAsync(passes, fails); and the exception is returned asynchronously, breaking the application logic.

When this snippet is inserted before the line with await, the code behaves as expected.

[feeh27]

I solved this problem as follows:

Implementation - Validation

const Validator = require('../../lib/validatorjs'); // Import code  with register

const data = {
  name: 'Name',
  email: 'non-unique@email.com',
  password: 'password',
};

const rules = {
  name: 'required',
  email: 'required|unique:users,email',
  password: 'required',
};

const validation = new Validator(data, rules);

let passes = () => {};
let fails = () => {};

const promise = new Promise((resolve) => {
  passes = () => { resolve(true); };
  fails = () => { resolve(false); };
});

validation.checkAsync(passes, fails);

const result = await promise;

if (result === false) {
  const message = JSON.stringify(validation.errors.all());
  throw new UserInputError(message);
}

It would be nice to have native support from the library for async calls.

[feeh27]

I found new problems using registerAsync, when an exception occurs within the registerAsync callback it is not returned correctly, because of that I stopped using it and had to do the asynchronous validations separately.

By coincidence I started to have the same behavior as registerAsync and it only occurs when I have asynchronous calls inside a loop, I believe this is the same problem that this library has, follow the code I used:

const userId = data['user_id'];
const groupCode = data['code'];

const validators = [
  new UniqueValidator(getFirstGroupCommand, groupCode, 'code', 'This field must be unique');
  new ExistsValidator(getUserByIdCommand, userId, 'user_id', `User with id ${userId} not exists.`);
];

let errors = [];

await validators.every(async (validator) => {
  errors = await validator.validate();
});

To solve I used the following solution with for loop:

const userId = data['user_id'];
const groupCode = data['code'];

const validators = [
  new UniqueValidator(getFirstGroupCommand, groupCode, 'code', 'This field must be unique');
  new ExistsValidator(getUserByIdCommand, userId, 'user_id', `User with id ${userId} not exists.`);
];

let errors = [];

for (let i = 0; i < this.validators.length; i += 1) {
  const validator = this.validators[i];

  errors = await validator.validate();
}

I believe that this is the solution to the problem reported in this issue, that is, there must be some loop that is not doing the correct treatment for asynchronous calls.

I didn't find in the library code the location where the asynchronous calls are made, if they show me I am extremely happy to contribute to the library by doing the correct treatment.

[mikeerickson]

@feeh27 i am about to release version 3.5 (should be ready in the next week or so) but I am going to move forward without a proper for your issue as it is more of an edge case.

After 3.5 release, we can move forward with address this issue.

[feeh27]

I am very grateful for the feedback, as I said earlier, I am willing to contribute, however, as I still do not understand the operation of registerAsync andcheckAsync I do not know where to start.

If you show me the way (when possible), I can contribute to the solution.

[feeh27]

Update:

const userId = data['user_id'];
const groupCode = data['code'];

const validators = [
  new UniqueValidator(getFirstGroupCommand, groupCode, 'code', 'This field must be unique');
  new ExistsValidator(getUserByIdCommand, userId, 'user_id', `User with id ${userId} not exists.`);
];

const errors = [];

for (let i = 0; i < this.validators.length; i += 1) {
  const validator = this.validators[i];

  const validationReponse = await validator.validate();
  errors.push(validationReponse);
}

The above solution works, but it makes asynchronous calls synchronously.

To do asynchronously the best option I found was to use Promise.all(), as we can see in the example below:

const userId = data['user_id'];
const groupCode = data['code'];

const validators = [
  new UniqueValidator(getFirstGroupCommand, groupCode, 'code', 'This field must be unique');
  new ExistsValidator(getUserByIdCommand, userId, 'user_id', `User with id ${userId} not exists.`);
];

const validationPromises = [];

for (let i = 0; i < this.validators.length; i += 1) {
  const validator = this.validators[i];

  validationPromises.push(validator.validate());
}

const errors = await Promise.all(validationPromises);

[mikeerickson]

@feeh27 I can see what you are doing, but am curious why async rules are not working? Without more context as to your implementation, it is difficult for me to advise accordingly.

• What does UniqueValidator and ExistsValidator classes look like?
• I am assuming each of them return promises, but that is independent of validatorjs
• What do the getFirstGroupCommand and getUserByIdCommand methods do?

Since you are doing your own promise processsing (again, this is is independent of what validatorjs is doing), your implementation of using Promise.all would be a viable approach

• Have you consulted the validatorjs tests to see how async validation should be handled?

https://github.com/mikeerickson/validatorjs/blob/master/spec/async-rule.js

[feeh27]

@mikeerickson yes, the classes UniqueValidator and ExistsValidator are independent of validatorjs.

The getFirstGroupCommand and getUserByIdCommand are javascript objects that follow the command design pattern.

See this link to understand more about the command design pattern: https://refactoring.guru/design-patterns/command

In my implementation, both have an asynchronous function exec and their internal implementation doesn't make any difference to the logic of the caller, just put the await when calling the exec funcion.

I use the commands to execute calls to other services, whoever uses it does not know which client it uses (it can be sequelize ORM, elasticsearch client, axios, or any other), abstracting this part of the logic and allowing my validators to be compatible with any third party service communication interface, where I use only the return of the command to perform the validation.

However, each command has its own responsibility, such as a search by id, or search by filters, for this reason we have several commands.

The way presented in the last comment, was how I solved it outside of validatorjs and just to explain the implementation of Promise.all, If the treatment in validatorjs was correct, this would be my implementation:

https://github.com/mikeerickson/validatorjs/issues/418#issue-835367912 (Comment I made when I opened this issue)

I was not aware of where I could perform this consultation, I'll check this test to find out how we can treat it within the validatorjs.

[feeh27]

@mikeerickson I saw the tests and realized that validatorjs doesn't work with promises when making asynchronous calls, it must be one of the reasons why his behavior didn't work in my implementation.

The ideal would be if the callback function was asynchronous to always return a promise.

I would like to see the code snippet that calls this callback, as it is probably in the implementation of this snippet that we will find the problem.

[feeh27]

I may be mistaken, but I believe that the treatment should be done in the code snippet below:

https://github.com/mikeerickson/validatorjs/blob/46928d706054b94821680c089de8b2a185830429/src/validator.js#L85-L143

I understood a little of the logic, a counter was used that is used to wait for the number of validations completed and the number of registered validations to be equal and only after that it returns the validations, as I said in the last comment, I believe that what is missing is to use promises in all the functions involved.

[francsiswanto]

I solved this problem as follows:

Implementation - Validation

const Validator = require('../../lib/validatorjs'); // Import code  with register

const data = {
  name: 'Name',
  email: 'non-unique@email.com',
  password: 'password',
};

const rules = {
  name: 'required',
  email: 'required|unique:users,email',
  password: 'required',
};

const validation = new Validator(data, rules);

let passes = () => {};
let fails = () => {};

const promise = new Promise((resolve) => {
  passes = () => { resolve(true); };
  fails = () => { resolve(false); };
});

validation.checkAsync(passes, fails);

const result = await promise;

if (result === false) {
  const message = JSON.stringify(validation.errors.all());
  throw new UserInputError(message);
}

It would be nice to have native support from the library for async calls.

Very nice @feeh27, this approach works for me.

[wulfsolter]

Cleaning up the approach that @feeh27 did a little bit, this is what I'm using:

    const validation = new Validator(data, rules, customErrorMessages);
    const validatorPromise = new Promise((resolve) => { validation.checkAsync(() => { resolve(true); }, () => { resolve(false); }); });
    const result = await validatorPromise;

    // failed validation
    if (result === false) {
      const allErrors = validation.errors.all();
      ...