Search code examples
javascriptnode.jsasync-awaites6-promise

Should the inner function or the calling function be wrapped within a try-catch block for correct error handling?


Should the inner function or the calling function be wrapped within a try-catch block for correct error handling?

Example 1

// Async function definition
const getLatestRecords = async() => {
    try {
        const records = await Record.find({})
        return records
    } catch(e) {
        console.error(e)
    }
}

// Async function call without error handling - will this suffice?
let records

const callAsyncFunction = async() => {
    records = await getLatestRecords()
}

callAsyncFunction()

// Same async function call with error handling - or should it be like this?
let records

const callAsyncFunctionWithTryCatch = async() => {
    try {
        records = await getLatestRecords()
    } catch(e) {
        console.error(e)
    }   
}

callAsyncFunctionWithTryCatch()

Q1) In the example above, the async function definition getLatestRecords() is already wrapped in a try-catch block for error handling. In spite of this, is the calling function as defined by callAsyncFunction() enough or should the async function call also be wrapped in a try-catch block as demonstrated by callAsyncFunctionWithTryCatch()?

Example 2

// Test without try catch block - will this suffice?
test('Should delete records with valid id', async() => {
    const record = await new Record().save()

    const res = await request(app)
        .delete(`/records/${record._id}`)
        .expect(200)
    
    expect(await Records.countDocuments()).toBe(0)
})

// Test with try catch block - should all tests with await in them be wrapped in a try catch block?
test('Should delete records with valid id', async() => {
    try {
        const record = await new Record().save()

        const res = await request(app)
            .delete(`/records/${record._id}`)
            .expect(200)
        
        expect(await Records.countDocuments()).toBe(0)
    } catch(e) {
        console.error(e)
    }
})

Q2) Similarly, for tests, should all tests with await calls in them be wrapped in a try catch block? Keep in mind, that one wants a test to fail if something goes wrong so I am not totally sure about this one.


Solution

  • You should only enclose them in try/catch if you know you can handle everything regarding the error in that function. Otherwise, you should let the consumer of the function decide what to do with errors. For example, with getLatestRecords, if there are various different points around the script where it's used, it would probably make the most sense to let those different points handle errors, depending on their context. For example:

    const getLatestRecords = async () => {
      const records = await Record.find({})
      return records
    }
    // could also just do: `return Record.find({})`
    
    getLatestRecords()
      .then(doStuff)
      .catch((err) => {
        res.status(500).send('Could not retrieve records for ' + user);
      });
    

    If you put the try/catch inside the inner getLatestRecords function, on the other hand, consumers will have to check if an error occurred by checking whether the return value exists, rather than .catch, which is a bit strange.

    If all awaits are inside try/catch, the containing async function will never reject. So, having the caller of that async function put it inside try/catch won't accomplish anything, because the inner function will never reject; the catch will never be entered. That is, for this example:

    const getLatestRecords = async() => {
        try {
            const records = await Record.find({})
            return records
        } catch(e) {
            console.error(e)
        }
    }
    const callAsyncFunctionWithTryCatch = async() => {
        try {
            records = await getLatestRecords()
        } catch(e) {
            console.error(e)
        }   
    }
    
    callAsyncFunctionWithTryCatch()
    

    it's superfluous; simplify it to

    const getLatestRecords = async() => {
        try {
            const records = await Record.find({})
            return records
        } catch(e) {
            console.error(e)
        }
    }
    const callAsyncFunctionWithTryCatch = async() => {
        records = await getLatestRecords()
    }
    
    callAsyncFunctionWithTryCatch()
    

    (or catch in callAsyncFunctionWithTryCatch only - but don't catch in both)