Wsrtjtyk

In an express + passport + local strategy app, I use bcrypt to hash password, this is working:

var bcrypt = require('bcrypt-nodejs');



familySchema.pre('save', function(next) {

  var family = this;

  var SALT_FACTOR = 14;



  if (!family.isModified('password')) return next();



  bcrypt.genSalt(SALT_FACTOR, function(err, salt) {

    if (err) return next(err);



    bcrypt.hash(family.password, salt, null, function(err, hash) {

      if (err) return next(err);

      family.password = hash;

      next();

    });

  });

});

Then I refactored using promisify and async/await:

const bcrypt = require('bcrypt-nodejs');

const util = require('util');

const bcryptGenSalt = util.promisify(bcrypt.genSalt);

const bcryptHash = util.promisify(bcrypt.hash);



familySchema.pre('save', async function(next) {

  var family = this;

  const SALT_FACTOR = 14;

  if (!family.isModified('password')) return next();

  const salt = await bcryptGenSalt(SALT_FACTOR).catch(next);

  const hash = await bcryptHash(family.password, salt, null).catch(next);

  family.password = hash;

  next();

});

• Is this refactoring actually correct?



• How can I double check that errors in bcryptGenSalt or bcryptHash are caught correctly? Is there a way to somehow "force" bcryptGenSalt into throwing an error, for testing?




• Next step, how can I remove the two .catch(next), using a wrapAsync util function:

wrapAsync.js:

module.exports = fn => (req, res, next) => fn(req, res, next).catch(next);

Following attempt is not working, error: family.isModified() is not a function, (probably because the this is not right anymore).
And what to do with the arguments of wrapAsync, as next should be the 3rd argument?

familySchema.pre(

  'save',

  wrapAsync(async function(req, res, next) {

    var family = this;

    const SALT_FACTOR = 14;

    debugger;

    if (!family.isModified('password')) return next();

    const salt = await bcryptGenSalt(SALT_FACTOR);

    const hash = await bcryptHash(family.password, salt, null);

    family.password = hash;

    next();

  })

);

The refactoring is not correct, because after the .catch statements, the rest of the function will continue to run. So if for instance bcryptGenSalt throws an error, next is called (because of .catch(next)) but it will also continue with the next line of code, until the end of the function (where next is called again).

Typically, in async functions, you use try/catch around statements that may throw errors:

familySchema.pre('save', async function(next) {

  const SALT_FACTOR = 14;

  if (!this.isModified('password')) return next();

  try {

    const salt = await bcryptGenSalt(SALT_FACTOR);

    const hash = await bcryptHash(this.password, salt, null);

    this.password = hash;

    return next();

  } catch(err) {

    return next(err);

  }

});

Is there a way to somehow "force" bcryptGenSalt into throwing an error, for testing?

It depends on which tools you use for testing, but there are packages like sinon that can stub existing functions so you can controllably make then throw errors, which you can then test for.