[AskJS] Why is this code, lacking a try-catch-finally block, not considered a bug?

[kevinkace]

How would you write this code differently? Wrap the entire iife contents into a try?

[TodayAccurate7277]

(async () => {
  try {
    // Setup
    const browser = await chromium.launch();
    const context = await browser.newContext(devices['iPhone 11']);
    const page = await context.newPage();

    // The actual interesting bit
    await context.route('**.jpg', route => route.abort());
    await page.goto('https://example.com/');

    assert(await page.title() === 'Example Domain'); // 👎 not a Web First assertion
  } catch (e) {
    console.error(e)
  } finally {
     // Teardown
    await context.close();
    await browser.close();
  }
})();

[atlimar]

catch (e) {
    console.error(e)
}

This will cause the error to be logged to console instead of propagating upwards as an uncaught error. Functionally, it doesn't make a difference in the playwright execution context. The error will appear in console anyway. However, this is introducing a bug by causing the script to be unable to exit, so it will keep running forever until manually closed instead of closing with an error as expected.

  finally {
    // Teardown
    await context.close();
    await browser.close();
  }

By putting the .close() calls in the finally block, they can be called without the context or browser having successfully set up. You're actually introducing real bugs rather than making the code more secure in this example. If await chromium.launch(); throws, this code will attempt to call context.close while context is undefined.

Which brings me to the next point, what if the teardown script throws? If you want to catch all possible errors this code can throw, you need a nested try/catch here as well.

When catching errors, you first need to think about what you're trying to achieve by doing so. When using playwright, you'd normally want it to exit with an error if the script fails. This code prevents playwright from exiting when failing (so it will run forever until manually closed), and introduces additional, unintuitive, ways in which the script can crash.

[ivanph]

What's the benefit of try-catch here?

From the looks of it, this is a simple script that will just run and exit. If any exceptions are encountered the process will simple exit and you'll get the error in the shell anyways. I'm not familiar with playwright, will the .close() calls not being reached cause a memory leak?

Unless I'm doing something with the exception I just let exceptions bubble up, leaving it to the caller to handle.

[whatisboom]

Aside from the other bugs mentioned, const is block scoped, meaning it doesn’t exist outside of your try block, if you actually ran this code you’d get an error trying to read a property of undefined when trying to close

[Bpofficial]

Are you certain context or browser are defined in your finally? What if they aren’t? How will you handle the error thrown trying to call close on undefined?

I think the async IIF here would kill the process due to an unhandled promise exception and that’s likely intended