Use Try/Catch Ideally

Question

I have two function, the controller and the service. Here is the service code.

const getVersion = async (type) => {
  const version = await Version.findOne({ TYPE: type }, { _id: false, VERSION: true })

  return version
}

And the controller code call the getVersion function that exist in service

const getVersion = async (req, res) => {
  try {
    ......
    const version = await Version.findOne({ TYPE: type }, { _id: false, VERSION: true })
    ......
  } catch (error) {
    ......
  }
}

So my question is, in getVersion() function, there's an asynchronous call. Should I wrap the function in try catch, so it would look like this:

const getVersion = async (type) => {
  try {
    const version = await Version.findOne({ TYPE: type }, { _id: false, VERSION: true })
    return version
  } catch (error) {
    return error
  }
}

Or should i leave as it to be like the original one that use the try/catch in the root of the function? What are the advantages and the disadvantages of those two method? Thank you.

Mulan · Accepted Answer · 2020-04-14T04:52:16.150

This is an anti-pattern -

const getVersion = async (type) => {
  try {
    const version = await Version.findOne({ TYPE: type }, { _id: false, VERSION: true })
    return version
  } catch (error) {
    return error
  }
}

The reason being that your function is labeled async it already returns a Promise. So it will either resolve version or it will reject the error.¹

This is the idiomatic way to write it -

const getVersion = type =>
  Version.findOne({ TYPE: type }, { _id: false, VERSION: true })

Now when you call it, a valid version response will be resolved, or some error will be rejected -

getVersion("foo").then(console.log, console.error)

^1.In your getVersion you actually resolve both the successful case and the error case. This is effectively silencing the error instead of letting it bubble up to the caller. By rejecting the error, you allow the caller to handle it appropriately.

This is a similar anti-pattern -

function foo (s = "") {
  if (s.length > 5)
    return true
  else
    return false
}

Which is the less idiomatic version of -

function foo (s = "") {
  return s.length > 5
}

Or as an arrow function -

const foo = (s = "") =>
  s.length > 5

score 0 · Answer 2 · answered Apr 14 '20 at 04:35

I suggest you leave as it is. There is no need to add try catch at different places to handle same exception. Suppose assume you are logging exception message to database. If catch is at 2 places, you will end up writing 2 logs into Db. It gives wrong perception that 2 exceptions occurred!

score 0 · Answer 3 · answered Apr 14 '20 at 04:39

You should only wrap around to the code that's actually asynchronously fetching data (i.e, liable to be successful or fail). For example, this line - const version = await Version.findOne({ TYPE: type }, { _id: false, VERSION: true }). So, that if that's fails Catch block would run. You should wrap the whole function with try/catch

score -1 · Answer 4 · answered Apr 14 '20 at 04:40

Should I wrap the function in try catch

You need to wrap async functions, whenever a promise is being resolved.

The await statement means there's an asynchronous call that could potentially fail and reject. If it fails and it's not within a trycatch block, then it will create an Unhandled Promise Rejection.

TLDR: If you use the await keyword, then wrap it with a try catch.

This function requires a trycatch block

async function DoSomething() {
  try {
    const result = await MakeRequest(); // failure must be handled here
    return result;
  } catch (error) {
    // Handle error
  }
}

This function doesn't require a trycatch block

// This requires a trycatch block
async function DoSomething() {
  return MakeRequest(); // failure can be handled by parent function
}

Use Try/Catch Ideally

4 Answers4