async.auto & the curious case of try ... catch

[trvra]

What version of async are you using?
v2.5.0
Which environment did the issue occur in (Node version/browser version)
v6.10.3
What did you do? Please include a minimal reproducable case illustrating issue.

async.auto({
  returnOne: (callback) => callback(null, 1),
  tryCatch: ["returnOne", (results, callback) => {
    try {
      assert(results.returnOne === 1);
      return callback(null);
    } catch (err) {
      console.log(`Caught an error: ${err.message}`);
      return callback(err);
    }
  }],
}, (err, results) => {
  assert(results.returnOne !== 1);
  console.log(`All Done!`);
});

What did you expect to happen?
The assert inside the final aync.auto callback fails and throws an error which is not caught and stops the Node.js process. In my real world example of this, we are testing a block of code within Mocha, which handles the error and reports that the test failed with information about the assertion that caused the failure.

What was the actual result?
Try / Catch from the previous auto step catches the error and calls the tryCatch callback a second time, resulting in a different error:

async/dist/async.js:903
        if (fn === null) throw new Error("Callback was already called.");
                         ^
Error: Callback was already called.

[aearly]

try {
  //...
  return callback(null, result)
} catch (e) {
  //..
  return callback(e)
}

Is a huge anti-pattern because if something throws later in the callback chain (e.g. your assert), it will be caught in the try block, causing a double callback in the catch. Correct way:

let err, result
try {
  result = doSomething()
} catch (e) {
  err = e
}
callback(err, result)

[trvra]

@aearly Makes sense, thanks for the tip!

(Also, hello! 👋)

[aearly]

👋😊