javascript - Should I avoid try catch in every single asyncawait on Node js? - Stack Overflow

admin管理员组

文章数量:1297071

This is a design question that came up to me while unit testing. Let's dive into the example:

Imagine this:

async function foo() {
    try {
        return apiCall()
    }
    catch (e) {
        throw new CustomError(e);
    } 
}



async function bar() {
    return foo()
}



async function main() {
    try {
        await bar()
    }catch(e) {
        console.error(e)
    }
}

main()

What do we see here? That the only function that hasn't got a try-catch block is bar. But if foo fails, it should get catched by the main catch.

While unittesting this like

describe('testing bar', () => {
    it('foo should throw', () => {
        foo.mockImplementantion(() => { throw new CustomError('error')});
        bar()
        .then((result) => console.log(result))
        .catch((err) => { exepect(err).toBeInstanceOf(CustomError)}) // this is what we are testing
    })
})

The output we see is that an Unhandled promise rejection is logged in the console.

So, my question is... even if I know that the main() will catch the error, should I use try-catch block inside all async functions?

This is a design question that came up to me while unit testing. Let's dive into the example:

Imagine this:

async function foo() {
    try {
        return apiCall()
    }
    catch (e) {
        throw new CustomError(e);
    } 
}



async function bar() {
    return foo()
}



async function main() {
    try {
        await bar()
    }catch(e) {
        console.error(e)
    }
}

main()

What do we see here? That the only function that hasn't got a try-catch block is bar. But if foo fails, it should get catched by the main catch.

While unittesting this like

describe('testing bar', () => {
    it('foo should throw', () => {
        foo.mockImplementantion(() => { throw new CustomError('error')});
        bar()
        .then((result) => console.log(result))
        .catch((err) => { exepect(err).toBeInstanceOf(CustomError)}) // this is what we are testing
    })
})

The output we see is that an Unhandled promise rejection is logged in the console.

So, my question is... even if I know that the main() will catch the error, should I use try-catch block inside all async functions?

• javascript
• node.js
• async-await
• jestjs

Share Improve this question Follow edited Aug 4, 2020 at 22:07 skyboyer 23.8k77 gold badges6262 silver badges7171 bronze badges asked Aug 4, 2020 at 16:55 Julian MendezJulian Mendez 3,38033 gold badges1616 silver badges3939 bronze badges 5

• Because a node.js process can/will terminate from an unhandled promise rejection, it is advisable that you always use try/catch with await, and always add a .catch handler to the end of Promise chains. – Jared Smith Commented Aug 4, 2020 at 16:59
• what you are telling me is that I should wrap the function bar() inside a try catch block, right? – Julian Mendez Commented Aug 4, 2020 at 17:02
• The async function bar returns a promise right? The async function foo can throw an exception in it's catch block which causes an Unhandled promise rejection in bar, so........ – gforce301 Commented Aug 4, 2020 at 18:00
• Your test case doesn't return the promise or use async/await nor does it have a done callback, so it's likely that expect is throwing an exception about being called after the test has ended. What's the error message of the unhandled rejection? – Bergi Commented Aug 4, 2020 at 19:24
• @Bergi (node:44872) UnhandledPromiseRejectionWarning: CustomError – Julian Mendez Commented Aug 4, 2020 at 20:55

Add a ment  | 

3 Answers 3

Sorted by: Reset to default 3

try..catch may be necessary if a function is able to recover from an error, do a side effect like logging, or re-throw a more meaningful error.

If CustomError is more preferable than an error that apiCall can throw then try..catch necessary, otherwise it doesn't. Also the problem with foo is that it handles only synchronous errors. In order to handle rejected promises, it should be return await apiCall(), this is a known pitfall of async.

Uncaught rejections are unwanted, they currently result in UnhandledPromiseRejectionWarning and are expected to crash Node in future versions. It's preferable to handle an error in a meaningful way at top level, so main needs to catch the error. This can be delegated to process uncaughtRejection event handler but it may be beneficial for it to stay extra level of error handling that should be never reached.

The output we see is that an Unhandled promise rejection is logged in the console.

This shouldn't happen. A rejection needs to be handled by the test. One possible point of failure is explained above, foo can return original error from apiCall instead of CustomError in case it wasn't correctly mocked, this will fail the expectation and result in unhandled rejection in catch(). Another point of failure is that the test has unchained promise because it wasn't returned, the test always passes.

Asynchronous test that uses promises should always return a promise. This can be improved by using async..await. foo is async, it's expected to always return a promise:

it('foo should throw', async () => {
    foo.mockImplementantion(() => { return Promise.reject(new CustomError('error')) });
    await expect(bar()).rejects.toThrow(CustomError);
})

Now even if foo mock fails (foo mock won't affect bar if they are defined in the same module as shown) and bar rejects with something that is not CustomError, this will be asserted.

No. You don't need to use try/catch in every async/await. You only need to do it at the top level. In this case your main function which you are already doing.

Weather you should is a matter of opinion. The go language designers feel strongly enough about this that is has bee the standard in go to always handle errors at each function call. But this is not the norm in javascript or most other languages.

Unhandled promise rejection

Your unhandled promise rejection is thrown by your it() function because you are not telling it to wait for the promise to plete.

I assume you are using something like mocha for the unit test (other frameworks may work differently). In mocha there are two ways to handle asynchronous tests:

1. Call the done callback - the it() function will always be called with a done callback. It is up to you weather you want to use it or like in your posted code to not use it:

    describe('testing bar', () => {
        it('foo should throw', (done) => {
            foo.mockImplementantion(() => { throw new CustomError('error')});
            bar()
            .then((result) => {
                console.log(result);
                done(); // ------------- THIS IS YOUR ACTUAL BUG
             })
            .catch((err) => {
                exepect(err).toBeInstanceOf(CustomError);
                done(); // ------------- THIS IS YOUR ACTUAL BUG
            })
        })
    })
   

2. Return a Promise. If you return a promise to the it() function mocha will be aware that your code is asynchronous and wait for pletion:

    describe('testing bar', () => {
        it('foo should throw', (done) => {
            foo.mockImplementantion(() => { throw new CustomError('error')});
   
            return bar() // <----------- THIS WOULD ALSO FIX IT
            .then((result) => {
                console.log(result);
             })
            .catch((err) => {
                exepect(err).toBeInstanceOf(CustomError);
            })
        })
    })
   

In short, there is nothing wrong with your code. But you have a bug in your unit test.

As @Bergi told me I will post some solutions right here

I wrap the function in a try catch block

1.

async function bar() {
    try{
       return foo()
    } catch (e) {
       throw e
    }
}

2. Rewrite the test

describe('testing bar', () => {
    it('foo should throw', (done) => {
        foo.mockImplementantion(() => { throw new CustomError('error')});
        bar()
        .then((result) => { throw result }) // this is because we are expecting an error, so if the promise resolves it's actually a bad sign.
        .catch((err) => { 
          exepect(err).toBeInstanceOf(CustomError)}) // this is what we are testing
          done();
    })
})

3. Use return in the test case

describe('testing bar', () => {
    it('foo should throw', () => {
        foo.mockImplementantion(() => { throw new CustomError('error')});
        return bar()
        .then((result) => { throw result })
        .catch((err) => { exepect(err).toBeInstanceOf(CustomError)}) // this is what we are testing
    })
})

本文标签： javascriptShould I avoid try catch in every single asyncawait on Node jsStack Overflow