Skip to content

[Detail Bug] Browser respawn failures cause unhandled promise rejections and potential Node.js crashes #696

@detail-app

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_06887db3-bf54-40ab-976d-46c66ab2b840/bugs/bug_8449ffbe-5be7-4e01-a37e-2f83f9b76fba

Summary

  • Context: The getBrowser() function in packages/browserless/src/index.js detects disconnected browsers and triggers a respawn to restore connectivity.
  • Bug: When getBrowser() calls respawn() at line 68, it doesn't await the returned promise, causing unhandled promise rejections if the respawn fails.
  • Actual vs. expected: If driver.spawn() fails during respawn, the Promise.all in respawn() rejects without a handler, creating an unhandled rejection. Expected behavior is that all promise rejections should be caught and handled.
  • Impact: In strict Node.js environments (--unhandled-rejections=strict), this causes application crashes. In normal environments, it produces spurious error logs that make debugging difficult.

Code with Bug

packages/browserless/src/index.js:

const getBrowser = async () => {
  if (isClosed) return browserProcessPromise
  const browserProcess = await lock(async () => {
    const browserProcess = await browserProcessPromise
    if (browserProcess.isConnected()) return browserProcess
    respawn()  // <-- BUG 🔴 Promise returned by respawn() is not awaited/caught, so failures become unhandled rejections
  })

  return browserProcess || getBrowser()
}
const respawn = () =>
  !isClosed &&
  Promise.all([
    browserProcessPromise.then(driver.close),
    (browserProcessPromise = spawn({ respawn: true }))
  ])

Additional unhandled-rejection path (event handler ignores returned promise):

promise.then(async browser => {
  if (launchOpts.mode !== 'connect') browser.once('disconnected', getBrowser) // <-- BUG 🔴 returned promise is not handled
  // ...
})

Explanation

  • respawn() returns a rejecting promise when spawn({respawn: true}) fails.
  • getBrowser() calls respawn() without await/.catch(), so that rejection is not observed by any handler, producing unhandledRejection events (and process crashes under --unhandled-rejections=strict).
  • The issue can trigger multiple times because respawn() may be invoked both from explicit getBrowser() calls and via the browser disconnected event listener, where getBrowser is used as a fire-and-forget handler.

Recommended Fix

  • In getBrowser(), await respawn() (or explicitly attach .catch() if it must be fire-and-forget).
  • Wrap the disconnected handler so the promise returned by getBrowser() is caught/logged (instead of passing getBrowser directly).

History

This bug was introduced in commit f6b34ac. The commit refactored the locking mechanism from mutexify to superlock, and during this refactor, the await keyword was accidentally removed from the respawn() call inside getBrowser().

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions