Async/Await misinformation and bad practice

[Svenskunganka]

The Async/Await section spreads some misinformation and bad practices. It explains that try/catch must be used whenever an await expression is used, but that is not the case. For example, this snippet:

function getGithubUser(username) {
  return new Promise((resolve, reject) => {
    fetch(`https://api.github.com/users/${username}`)
      .then(response => {
        const user = response.json();
        resolve(user);
      })
      .catch(err => reject(err));
  })
}

getGithubUser('mbeaudru')
  .then(user => console.log(user))
  .catch(err => console.log(err));

is the same as:

function getGithubUser(username) {
  return fetch(`https://api.github.com/users/${username}`).then(response => response.json());
}

getGithubUser('mbeaudru')
  .then(user => console.log(user))
  .catch(err => console.log(err));

You don't need to wrap it in another Promise, fetch already returns a promise.

This snippet is really bad, which I'll explain:

async function getGithubUser(username) { // promise + await keyword usage allowed
  try { // We handle async function errors with try / catch
    const response = await fetch(`https://api.github.com/users/${username}`); // Execution stops here until fetch promise is fulfilled.
    const user = response.json();
    return user; // equivalent of resolving the getGithubUser promise with user value.
  } catch (err) {
    throw new Error(err); // equivalent of rejecting getGithubUser promise with err value.
  }
}

getGithubUser('mbeaudru')
  .then(user => console.log(user))
  .catch(err => console.log(err));

You don't need to have a try/catch block every time you have an await expression. You're already catching the error with .catch() on the last line. Any function that getGithubUser calls that throws, and any function the functions getGithubUser() calls that throws, and so on, will be caught by the last .catch(). It should be rewritten:

async function getGithubUser(username) { // promise + await keyword usage allowed
  const response = await fetch(`https://api.github.com/users/${username}`); // Execution stops here until fetch promise is fulfilled.
  const user = response.json();
  return user; // equivalent of resolving the getGithubUser promise with user value.
}

getGithubUser('mbeaudru')
  .then(user => console.log(user))
  .catch(err => console.log(err));

Additionally, the throw new Error(err); is super bad because you lose the original error type and whatever custom stack trace implementation it has. For example:

class TlsHandshakeError extends Error {}

async function throws () {
  throw new TlsHandshakeError("key mismatch");
}

async function doSomething () {
  try {
    await throws()
  } catch (e) {
    throw new Error(e);
  }
}

doSomething.catch(e => console.error(e));

The error written to console will be Error: Error: key mistmatch rather than TlsHandshakeError: key mismatch.

The above could be:

class TlsHandshakeError extends Error {}

async function throws () {
  throw new TlsHandshakeError("key mismatch");
}

async function doSomething () {
  await throws()
}

doSomething.catch(e => console.error(e)); // TlsHandshakeError: key mismatch

The code below will catch the error and not crash the application:

async function throws () {
  throw new Error("error");
}

async function step1 () {
  await throws();
}

async function step2 () {
  await step1();
}

async function step3 () {
  await step2();
}

step3().catch((err) => {
  // Error is caught here
  console.log(err); // Error: error
});

Same with the last snippet:

async function fetchPostById(postId) {
  try {
    const token = await fetch('token_url');
    const post = await fetch(`/posts/${postId}?token=${token}`);
    const author = await fetch(`/users/${post.authorId}`);

    post.author = author;
    return post;
  } catch(e) {
    throw new Error(e);
  }
}

fetchPostById('gzIrzeo64')
  .then(post => console.log(post))
  .catch(err => console.log(err));

should be:

async function fetchPostById(postId) {
  const token = await fetch('token_url');
  const post = await fetch(`/posts/${postId}?token=${token}`);
  const author = await fetch(`/users/${post.authorId}`)

  post.author = author;
  return post;
}

fetchPostById('gzIrzeo64')
  .then(post => console.log(post))
  .catch(err => console.log(err));

[mbeaudru]

@Svenskunganka thank you for the detailed explanation and your effort improving this document 👍.

About this snippet:

function getGithubUser(username) {
  return new Promise((resolve, reject) => {
    fetch(`https://api.github.com/users/${username}`)
      .then(response => {
        const user = response.json();
        resolve(user);
      })
      .catch(err => reject(err));
  })
}

getGithubUser('mbeaudru')
  .then(user => console.log(user))
  .catch(err => console.log(err));

Yes, the goal here was to have a side-by-side syntax equivalent between promises and async / await. It's quite useless indeed, but not that useless because this new promise returns the jsonified response. But I agree that it could be much better.

For the rest, you clearly demonstrated your point and I (or someone with a PR) will adapt the async / await notion accordingly.

However, do we need a try / catch if you are awaiting an async function with async / await ?

async function getGithubUser(username) { // promise + await keyword usage allowed
  const response = await fetch(`https://api.github.com/users/${username}`); // Execution stops here until fetch promise is fulfilled.
  const user = response.json();
  return user; // equivalent of resolving the getGithubUser promise with user value.
}

async function myFunc() {
  try {
    const user = await getGithubUser('mbeaudru');
    return user;
  } catch (err) {
    console.log(err);
  }
}

I would have appreciated a softer tone though, really / super bad is quite condescending, but the quality of the demonstration quite compensates I guess 😅.

[Svenskunganka]

I'm sorry, I did not mean to sound condescending, I apologize for that! Meant it as "bad for your application" because it will be very hard to debug an application that casts all errors into the generic Error type. I've been answering question on Stackoverflow and there are so many users that are using similar structure to async/await as demonstrated in this repo. There's just no end to such questions, so I figured it would be better to set out and attempt to fix the root causes for the misinformation.

About your first point, the snippet I provided in my original response does too return a JavaScript Object parsed from the JSON response:

function getGithubUser(username) {
  return fetch(`https://api.github.com/users/${username}`).then(response => response.json());
}

getGithubUser('mbeaudru')
  .then(user => {
    console.log(typeof user) // Object
  })
  .catch(err => console.log(err));

This is because a Promise's .then()-method returns a Promise. See https://repl.it/LvLJ

However, do you agree that if you are awaiting an async function with async / await then a try / catch is necessary ?

Purely depends on application structure I believe. You should try/catch if you can handle the error, but never to just throw it again, because that already happens behind the scenes so to speak. 😁
For example if you have a HTTP route handler that fetches a user from DB:

async function getUser (id) {
  return await Query.prepare("SELECT name from users WHERE id = ?", id)
}

async function routeHandler (req, res) {
  try {
    let user = await getUser(req.query.id)
    if (!user) return res.sendStatus(404) // Not found
    return res.status(200).json(user)
  } catch (e) {
    switch (e.constructor) {
      case DatabaseConnectionError:
        return res.sendStatus(500) // Internal server error
      case QuerySyntaxError:
        return res.sendStatus(400) // Bad request
      case default:
        return res.sendStatus(418) // I'm a teapot
    }
  }
}

I think this is fine for try/catch because it is the top-most function in the call chain. If Query.prepare throws, or another function that the Query.prepare function calls throws, it will be caught by the try/catch block in the routeHandler function. But I think the below is bad practice.

async function getUser (id) {
  try {
    return await Query.prepare("SELECT name from users WHERE id = ?", id)
  } catch (e) {
    throw e
  }
}

async function routeHandler (req, res) {
  try {
    let user = await getUser(req.query.id)
    if (!user) return res.sendStatus(404) // Not found
    return res.status(200).json(user)
  } catch (e) {
    switch (e.constructor) {
      case DatabaseConnectionError:
        return res.sendStatus(500) // Internal server error
      case QuerySyntaxError:
        return res.sendStatus(400) // Bad request
      case default:
        return res.sendStatus(418) // I'm a teapot
    }
  }
}

[Svenskunganka]

I should also add that try/catch in an async function works just the same as a regular try/catch block, in addition to that it can catch Promises. Here's an example:

function throws () {
  throw new Error("an error")
}

async function doSomething () {
  throws()
}

doSomething().catch(e => console.log(e)) // Error: an error

Calling a non-async function that throws from an async function wraps the thrown value in a rejected Promise and returns it.

[hansenman61]

Could this have to do with dr. Who?

[Svenskunganka]

@mbeaudru I like the changes, however I'd like to point out that what I've lined out in my previous comments holds true for Promises as well, as long as you return them. To illustrate, if we take the updated example:

function getUser() => { // This promise will be rejected!
  return new Promise((res, rej) => rej("User not found !")
};

function getAvatarByUsername(userId) {
  return new Promise((res, rej) =>
    getUser(userId)
      .then(user => res(user.avatar))
      .catch(err => rej(err))
  );
}

function getUserAvatar(username) {
  return new Promise((res, rej) =>
    getAvatarByUsername(username)
      .then(avatar => res({ username, avatar }))
      .catch(err => rej(err))
  );
}

getUserAvatar('mbeaudru')
  .then(res => console.log(res))
  .catch(err => console.log(err)); // "User not found !"

is the same as this:

function getUser() { // This promise will be rejected!
  return new Promise((res, rej) => rej("User not found !"))
}

function getAvatarByUsername(userId) {
  return getUser(userId).then(user => user.avatar)
}

function getUserAvatar(username) {
  return getAvatarByUsername(username).then((avatar) => { 
    return { username, avatar }
  })
}

getUserAvatar('mbeaudru')
  .then(res => console.log(res))
  .catch(err => console.log(err)); // "User not found !"

As long as you return the Promise chain, you don't need to catch or return a new Promise.
See these repls:
https://repl.it/MRJs/2
https://repl.it/MRJs/1

There are also some typos, the first getUser function is an arrow function, while the rest are regular functions. Not a typo per-se, but a bit inconsistent (not sure if this was intended?). The returned Promise is missing a ) at the end as well. 😃

[mbeaudru]

I updated the notion accordingly, thanks for your time 👍 !

[Svenskunganka]

Sorry to be a bother, but res is undefined here:

function getUser() { // This promise will be rejected!
  return new Promise((res, rej) => rej("User not found !"))
};

function getAvatarByUsername(userId) {
  return getUser(userId).then(user => res(user.avatar))
}

function getUserAvatar(username) {
  return getAvatarByUsername(username).then(avatar => res({ username, avatar }))
}

getUserAvatar('mbeaudru')
  .then(res => console.log(res))
  .catch(err => console.log(err)); // "User not found !"

should be:

function getUser() { // This promise will be rejected!
  return new Promise((res, rej) => rej("User not found !"))
}

function getAvatarByUsername(userId) {
  return getUser(userId).then(user => user.avatar)
}

function getUserAvatar(username) {
  return getAvatarByUsername(username).then(avatar => ({ username, avatar }))
}

getUserAvatar('mbeaudru')
  .then(res => console.log(res))
  .catch(err => console.log(err)); // "User not found !"

[mbeaudru]

Indeed, I did the change too fast 😅. I don't understand, why don't you submit a PR instead, since you've taken the time to write the fix ? You could be in the contributors list 👍.

I wait for your answer before fixing 😉

[bartduisters]

@mbeaudru you did not wait for the answer!