products

From callbacks to Tasks

Refactoring common auth flow from callbacks into Tasks.

I recently have looked at a new password hashing library Argon2 and saw the following example in their announcement

1
2
3
4
5
6
7
8
9
10
11
12
13
14
argon.generateSalt((err, salt) => {
  if (err) {
    throw err;
  }

  argon.hash('some-user-password', salt, (err, hash) => {
    if (err) {
      throw err;
    }

    console.log('Successfully created Argon2 hash:', hash);
    // TODO: store the hash in the user database
  });
});

The library itself seems to be very good - it won the recent Password hashing competition, but this particular code is far from the best when using the library. Let me explain.

To better show the problems with this callback-based code, let me mock argon object, and create an actual "user" function around the salt generation and password hashing.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
var argon = {
  generateSalt: function (cb) {
    setTimeout(() => cb(null, '-salt'), 0)
  },
  hash: function (password, salt, cb) {
    var err
    if (!password) {
      err = new Error('Password cannot be empty')
    }
    setTimeout(() => cb(err, password + salt))
  }
}
function savePassword(password) {
  argon.generateSalt((err, salt) => {
    if (err) {
      throw err;
    }
    argon.hash('some-user-password', salt, (err, hash) => {
      if (err) {
        throw err;
      }

      console.log('Successfully created Argon2 hash:', hash);
    });
  });
}

The success work-flow

If everything goes well, the code works. For example

1
2
savePassword('test1234')
// Successfully created Argon2 hash: test1234-salt

Great.

Empty password example

Let us see how the code handles empty string as a password. Initially, everything looks ok - we do get an exception thrown.

1
savePassword('')
1
2
3
4
5
6
$ node index.js 
/index.js:21
        throw err;
        ^
Error: Password cannot be empty
    at Object.argon.hash (/index.js:8:13)

We need to tell user that the password cannot be empty string. So let us catch the error and log it to the console.

1
2
3
4
5
6
try {
  savePassword('')
} catch (err) {
  console.error(err)
  console.error('User, enter valid password')
}
1
2
3
4
5
6
$ node index.js 
/index.js:22
        throw err;
        ^
Error: Password cannot be empty
    at Object.argon.hash (/index.js:8:13)

Hmm, we did not catch the error! The error was thrown asynchronously inside a setTimeout callback, and thus the try - catch block around savePassword('') never catches it.

How can we grab the errors?

Adding more callbacks

Let us pass a callback to savePassword instead of wrapping the code in try - catch block. We are going to use it like this

1
2
3
4
5
6
savePassword('', (err) => {
  if (err) {
    console.error(err)
    console.error('User, enter valid password')
  }
})

We cannot use try - catch inside savePassword for the same reason, we could not use it outside it. Thus we need to avoid throwing errors at all. Instead pass the error into the callback function and return right away.

1
2
3
4
5
6
7
8
9
10
11
12
13
function savePassword(password, cb) {
  argon.generateSalt((err, salt) => {
    if (err) {
      return cb(err);
    }
    argon.hash(password, salt, (err, hash) => {
      if (err) {
        return cb(err);
      }
      console.log('Successfully created Argon2 hash:', hash);
    });
  });
}

Now the empty password leads to the meaningful error message

1
2
3
$ node index.js 
[Error: Password cannot be empty]
User, enter valid password

Still, there is a subtle problem - we forgot to execute the callback on success. For example, a valid password will not show "success" message

1
2
3
4
5
6
7
8
savePassword('test1234', (err) => {
  if (err) {
    console.error(err)
    console.error('User, enter valid password')
  } else {
    console.log('Password saved')
  }
})
1
2
$ node index.js 
Successfully created Argon2 hash: test1234-salt

Let us make sure to execute the callback on success

1
2
3
4
5
function savePassword(password, cb) {
  ...
  console.log('Successfully created Argon2 hash:', hash);
  cb()
}
1
2
3
$ node index.js 
Successfully created Argon2 hash: test1234-salt
Password saved

Making sure ALL success and error execution paths run the callback is very hard. Can we do better?

Promises to the rescue

Let us move away from callbacks to promises. We can still use the original mock argon object, but let us first make a couple of promise-returning functions.

1
2
3
var promisify = require('bluebird').promisify
var genSalt = promisify(argon.generateSalt)
var genHash = promisify(argon.hash)

The best feature for us is the clear error handling and success path. Notice that we have NO error logic inside the savePassword at all.

1
2
3
4
5
function savePassword(password) {
  return genSalt()
    .then(salt => genHash(password, salt))
    .then(hash => console.log('Successfully created Argon2 hash:', hash))
}

The caller code clearly defines separate path for successful save vs error

1
2
3
4
5
6
7
8
savePassword('test1234')
  .then(() => console.log('Password saved'))
  .catch(err => {
    console.error(err)
    console.error('User, enter valid password')
  })
// Successfully created Argon2 hash: test1234-salt
// Password saved

The errors are handled

1
2
3
4
5
6
// savePassword('')
$ node index.js 
{ [Error: Password cannot be empty]
  cause: [Error: Password cannot be empty],
  isOperational: true }
User, enter valid password

Taking it to the next level

The above promise-based code is nice, yet it has several shortcomings, in my opinion

  1. The getHash function should be partially applied, since we know the first argument password right away!
1
2
3
4
5
6
function savePassword(password) {
  const genHashFor = genHash.bind(null, password)
  return genSalt()
    .then(salt => genHashFor(salt))
    .then(hash => console.log('Successfully created Argon2 hash:', hash))
}

Now we don't need the extra function just to pass salt to the genHash

1
2
3
4
5
6
function savePassword(password) {
  const genHashFor = genHash.bind(null, password)
  return genSalt()
    .then(genHashFor)
    .then(hash => console.log('Successfully created Argon2 hash:', hash))
}

This point-free code is simpler - no extra variables to distract us!

Which brings us to my second pet peeve

  1. The promise chain has different types of the first and second links

Look at the value returned. What is the type of the genSalt() and genHashFor?

1
2
return genSalt()     // Promise instance
  .then(genHashFor)  // function

This always bothers me - in order to combine promise steps together, I need to execute the first promise-returning function, get a Promise object and then add all promise-returning functions!

If we have a Promise object, that means the action has already started, which means our promise chain cannot be created as easily wihout side effects, because creating it means the action is already executing.

This is where the Tasks come in handy - they allow us to compose multiple actions (async or not) into a single pipeline and then start the first one.

Tasks to the rescue

A Task is just like a Promise, except the action has to be started separately by calling .fork() method. Let us "taskify" callback returning functions instead of "promisifying" them. I wrote a tiny utility to convert a Node style, callback expecting function into a function that returns a data.Task instead. Let us apply it here

1
2
3
var taskify = require('@bahmutov/taskify')
var genSalt = taskify(argon.generateSalt)
var genHash = taskify(argon.hash)

By default, genSalt() returns a Task, but nothing gets executed. In order to actually run the function, we must call .fork method

1
2
3
4
5
6
7
const saltTask = genSalt()
// nothing is running yet!
saltTask()
  .fork( // saltTak runs now
    err => console.error(err),
    salt => console.log('salt generated', salt)
  )

.fork() expects two callbacks, just like a Promise, but in reverse order - the error callback is first, while success callback function is second. This is done to make sure the errors are not silently ignored when there is no callback.

Tasks can be composed easily to run one after another using .chain method. To be explicit, we can write

1
2
3
4
function savePassword(password) {
  const saltTask = genSalt()
  return saltTask.chain(salt => genHash(password, salt))
}

Notice that we start with a Task (saltTask instance) and then chain new Tasks from functions passed to .chain. Yet nothing runs until someone calls .fork!

1
2
3
4
5
6
7
8
9
savePassword('test1234')
  .fork(
    err => {
      console.error(err)
      console.error('User, enter valid password')
    },
    () => console.log('Password saved')
  )
// Password saved

The function savePassword is so small and side effect free, that we can even make it a one-liner. Our goal is to make salt => genHash(password, salt) point-free. We can curry genHash, because we know it expects just two arguments.

1
2
3
4
5
var R = require('ramda')
var genHash = R.curryN(2, taskify(argon.hash))
// now we can genHash separately for first and last argument
// genHash(password)(salt) same as argon.hash(password, salt) without callback
var savePassword = password => genSalt().chain(genHash(password))

Nice! The final code is very short, yet side effect free, and both success and error paths are available to avoid silent failures. This is a big improvement over code that uses callbacks.

Links