109

I like the flatness of the new Async/Await feature available in Typescript, etc. However, I'm not sure I like the fact that I have to declare the variable I'm awaiting on the outside of a try...catch block in order to use it later. Like so:

let createdUser
try {
    createdUser = await this.User.create(userInfo)
} catch (error) {
    console.error(error)
}

console.log(createdUser)
// business
// logic
// goes
// here

Please correct me if I'm wrong, but it seems to be best practice not to place multiple lines of business logic in the try body, so I'm left only with the alternative of declaring createdUser outside the block, assigning it in the block, and then using it after.

What is best practice in this instance?

Bergi
  • 572,313
  • 128
  • 898
  • 1,281
freedomflyer
  • 2,161
  • 3
  • 24
  • 37
  • 1
    "Best practice" is to use what works and is understandable, maintainable, etc.. How could we answer this question "correctly"? I'd just use `var`, knowing the variable would be hoisted. Is that "wrong"? – Heretic Monkey Jun 20 '17 at 22:23
  • 4
    try/catch should enclose exactly what you want to capture an exception for. If you're looking explicitly for errors coming from `this.User.create()` then you wouldn't put anything else inside the try/catch. But, it's also a perfectly reasonable design to put a whole bunch of logic inside a try block. It all depends upon how/where you want to handle an error and how you want to design your exception handling code and what makes sense for a given operation. There is no generic best practice. The ONE generic best practice is to make sure you catch and handle all errors in some appropriate way. – jfriend00 Jun 20 '17 at 22:23
  • 1
    `async/await` is part of **ES2017** (this year's release), not ES6 (which was released two years ago). – Felix Kling Jun 20 '17 at 22:50
  • To add to @jfriend00's comment, if you put your business logic inside the `try` block, and that code `Error`s, (`TypeError`, `ReferenceError`, etc), that will be `catch`ed, which could produce unexpected behavior if you're expecting to only `catch` promise rejections. – KFunk Aug 13 '21 at 21:44

5 Answers5

86

It seems to be best practice not to place multiple lines of business logic in the try body

Actually I'd say it is. You usually want to catch all exceptions from working with the value:

try {
    const createdUser = await this.User.create(userInfo);

    console.log(createdUser)
    // business logic goes here
} catch (error) {
    console.error(error) // from creation or business logic
}

If you want to catch and handle errors only from the promise, you have three choices:

  • Declare the variable outside, and branch depending on whether there was an exception or not. That can take various forms, like

    • assign a default value to the variable in the catch block
    • return early or re-throw an exception from the catch block
    • set a flag whether the catch block caught an exception, and test for it in an if condition
    • test for the value of the variable to have been assigned
      let createdUser; // or use `var` inside the block
      try {
          createdUser = await this.User.create(userInfo);
      } catch (error) {
          console.error(error) // from creation
      }
      if (createdUser) { // user was successfully created
          console.log(createdUser)
          // business logic goes here
      }
    
  • Test the caught exception for its type, and handle or rethrow it based on that.

      try {
          const createdUser = await this.User.create(userInfo);
          // user was successfully created
          console.log(createdUser)
          // business logic goes here
      } catch (error) {
          if (error instanceof CreationError) {
              console.error(error) // from creation
          } else {
              throw error;
          }
      }
    

    Unfortunately, standard JavaScript (still) doesn't have syntax support for conditional exceptions.

    If your method doesn't return promises that are rejected with specific enough errors, you can do that yourself by re-throwing something more appropriate in a .catch() handler:

      try {
          const createdUser = await this.User.create(userInfo).catch(err => {
              throw new CreationError(err.message, {code: "USER_CREATE"});
          });
          …
      } …
    

    See also Handling multiple catches in promise chain for the pre-async/await version of this.

  • Use then with two callbacks instead of try/catch. This really is the least ugly way and my personal recommendation also for its simplicity and correctness, not relying on tagged errors or looks of the result value to distinguish between fulfillment and rejection of the promise:

      await this.User.create(userInfo).then(createdUser => {
          // user was successfully created
          console.log(createdUser)
          // business logic goes here
      }, error => {
          console.error(error) // from creation
      });
    

    Of course it comes with the drawback of introducing callback functions, meaning you cannot as easily break/continue loops or do early returns from the outer function.

Bergi
  • 572,313
  • 128
  • 898
  • 1,281
  • 2
    Your last example uses `.then()` to resolve the promise and provide a callback, so perhaps `await` has no effect there. – dcorking Jan 24 '18 at 14:57
  • 2
    @dcorking It is `await`ing the promise returned by the `.then(…)` call. – Bergi Jan 24 '18 at 20:32
  • do the lambdas returned by the `.then()` call need the async keyword? – dcorking Jan 25 '18 at 07:56
  • @dcorking Only if they use `await` inside. – Bergi Jan 25 '18 at 08:20
  • 2
    I have seen people attaching catch handler directly to await . Is it a good Idea to do that or wrap it inside try/catch? – Saroj Feb 25 '19 at 17:44
  • 7
    @Saroj `const result = await something().catch(err => fallback);` is simpler than `let result; try { result = await something(); } catch(err) { result = fallback; }` so yes in that case I consider it a good idea. – Bergi Feb 25 '19 at 18:08
  • @Bergi any downsides to this syntax? For example, how would one break out of the surrounding scope? Attaching the handler seems to be the most concise yet flexible approach. – Slaiyer Dec 09 '19 at 20:08
  • @Slaiyer What do you mean by "*break out of the surrounding scope*"? Sure, you can't place a `break` keyword in the `onreject` callback of `then`/`catch`, if you need to break from a loop go with a traditional `try` block. If you refer to returning a value to the surrounding scope, just `return` it from the callback and `await` the promise chain. – Bergi Dec 09 '19 at 23:29
28

Another simpler approach is to append .catch to the promise function. ex:

const createdUser = await this.User.create(userInfo).catch( error => {
// handle error
})
nevf
  • 4,188
  • 6
  • 30
  • 31
  • I had never thought of this, but I tried it out, and it has an interesting side effect: you can `return` a result in the `.catch()` callback to set the value. Otherwise, it returns undefined. – Joe Sadoski May 10 '21 at 23:55
  • 1
    This is mentioned in the [docs](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function#rewriting_a_promise_chain_with_an_async_function) – shmuels Jan 26 '22 at 19:35
1

I usually use the Promise's catch() function to return an object with an error property on failure.

For example, in your case i'd do:

const createdUser = await this.User.create(userInfo)
          .catch(error => { error }); // <--- the added catch

if (Object(createdUser).error) {
    console.error(error)
}

If you don't like to keep adding the catch() calls, you can add a helper function to the Function's prototype:

Function.prototype.withCatcher = function withCatcher() {
    const result = this.apply(this, arguments);
    if (!Object(result).catch) {
        throw `${this.name}() must return a Promise when using withCatcher()`;
    }
    return result.catch(error => ({ error }));
};

And now you'll be able to do:

const createdUser = await this.User.create.withCatcher(userInfo);
if (Object(createdUser).error) {
    console.error(createdUser.error);
}


EDIT 03/2020

You can also add a default "catch to an error object" function to the Promise object like so:

Promise.prototype.catchToObj = function catchToObj() {
    return this.catch(error => ({ error }));
};

And then use it as follows:

const createdUser = await this.User.create(userInfo).catchToObj();
if (createdUser && createdUser.error) {
    console.error(createdUser.error);
}
Arik
  • 4,602
  • 1
  • 23
  • 23
  • I use the last approach and it gives me `'catchToObj' is not a function` error. – newguy May 17 '20 at 06:53
  • @newguy `catchToObj` will exist on every `Promise` object after you call the first code segment in my answer. If your function doesn't return a `Promise` it won't work – Arik May 17 '20 at 10:13
  • I am using the Sequelize's `create` method which returns a `Promise`. definition is : `public static async create(values: object, options: object): Promise` – newguy May 17 '20 at 12:44
0

@Bergi Answer is good, but I think it's not the best way because you have to go back to the old then() method, so i think a better way is to catch the error in the async function

async function someAsyncFunction(){
    const createdUser = await this.User.create(userInfo);

    console.log(createdUser)
}

someAsyncFunction().catch(console.log);
  • But what if we have many await in the same function and need to catch every error?

You may declare the to() function

function to(promise) {
    return promise.then(data => {
        return [null, data];
    })
    .catch(err => [err]);
}

And then

async function someAsyncFunction(){
    let err, createdUser, anotherUser;

    [err, createdUser] = await to(this.User.create(userInfo));

    if (err) console.log(`Error is ${err}`);
    else console.log(`createdUser is ${createdUser}`);


    [err, anotherUser] = await to(this.User.create(anotherUserInfo));

    if (err) console.log(`Error is ${err}`);
    else console.log(`anotherUser is ${anotherUser}`);
}

someAsyncFunction();

When reading this its: "Wait to this.User.create".

Finally you can create the module "to.js" or simply use the await-to-js module.

You can get more information about to function in this post

Ivan
  • 11
  • 1
  • 2
    `then` isn't worse than `await` because it older. It's just different, and suited for other things. This "`await to(…)` style" on the other hand is reminiscent of the nodeback style with all its disadvantages. – Bergi Feb 25 '19 at 19:54
  • Btw, for better performance and simplicity you should use `promise.then(data => [null, data], err => [err, null]);` – Bergi Feb 25 '19 at 20:02
  • Exactly "It's just different, and suited for other things" `await` is used for create a code with a "synchronous" like syntax, the use of `then` and it's callback is more asynchronous syntax. Btw thanks for the code simplicity recommendation :) – Ivan Feb 27 '19 at 04:07
0

Cleaner code

for async/await error handling with Promise catch handler.

From what I see, this has been a long-standing problem that has bugged (both meanings) many programmers and their code.

ES6 Promise's catch handler provides a proper solution:

this.User.create(userInfo).then(createdUser => {
    console.log(createdUser)
    // business
    // logic
    // goes
    // here
}).catch(err => {
    //handle the error
})

But that looks like a removal of async/await altogether. Not true.

The idea is to use Promise style and catch for top level caller. Otherwise, continue to use async/await.

Example, besides creating user (this.User.create), we can push notification (this.pushNotification) and send email (this.sendEmail). All are async operations. There are no catch handlers required. Just async/await.

this.User.create

this.User.create = async(userInfo) => {

    // collect some fb data and do some background check in parallel
    const facebookDetails = await retrieveFacebookAsync(userInfo.email)
    const backgroundCheck = await backgroundCheckAsync(userInfo.passportID)

    if (backgroundCheck.pass !== true) throw Error('Background check failed')

    // now we can insert everything
    const createdUser = await Database.insert({ ...userInfo, ...facebookDetails })

    return createdUser
}

this.pushNotifcation and this.sendEmail

this.pushNotification = async(userInfo) => {
    const pushed = await PushNotificationProvider.send(userInfo)
    return pushed
})

this.sendEmail = async(userInfo) => {
    const sent = await mail({ to: userInfo.email, message: 'Welcome' })
    return sent
})

When all the async operations are composed together, we can use Promise-style with a #catch handler attached:

this.User.create(userInfo).then(createdUser => {

    console.log(createdUser)

    // business logic here

    return Promise.all([
        this.pushNotification(userInfo),
        this.sendEmail(userInfo)
    ])
    
}).catch(err => {
    // handle err
})

If we were to do this with try/catch, one will have to wrap everything in try/catch (not advisable), or set up many try/catches:

var createdUser
try {
    createdUser = await this.User.create(userInfo)
} catch (err) {
    //handle err
}

console.log(createdUser)

// business logic here

if (createdUser) {
    try {
        await this.pushNotification(userInfo)
        await this.sendEmail(userInfo)
    } catch (err) {
        // handle err
    }
}
Calvintwr
  • 7,624
  • 5
  • 26
  • 37