[Detail Bug] Adblock initialization permanently fails after a single transient asset load error

[detail-app]

Detail Bug Report

https://app.detail.dev/org_06887db3-bf54-40ab-976d-46c66ab2b840/bugs/bug_3fa6d67e-5f82-4312-9f6a-e9607c198a4f

Summary

• Context: The packages/goto/src/adblock.js module lazy-loads adblocker assets (engine.bin and autoconsent.playwright.js) and caches the loading promises to avoid redundant file reads.
• Bug: If the initial file load fails (due to filesystem errors, missing files, or deserialization errors), the rejected promise is cached permanently, preventing all future adblock operations from succeeding.
• Actual vs. expected: Once a load failure occurs, all subsequent calls to getEngine() or getAutoconsentPlaywrightScript() return the same cached rejected promise instead of retrying the file load.
• Impact: After a single transient filesystem error or missing file, the adblock functionality becomes permanently broken for the entire lifetime of the Node.js process, requiring a full process restart to recover.

Code with Bug

let enginePromise

const getEngine = () => {
  if (enginePromise) return enginePromise  // <-- BUG 🔴 Returns cached rejected promise (no retry)

  enginePromise = fs.readFile(path.resolve(__dirname, './engine.bin')).then(buffer => {
    const engine = PuppeteerBlocker.deserialize(new Uint8Array(buffer))
    engine.on('request-blocked', ({ url }) => debug('block', url))
    engine.on('request-redirected', ({ url }) => debug('redirect', url))
    return engine
  })
  // <-- BUG 🔴 No .catch() to clear enginePromise on failure

  return enginePromise
}

let autoconsentPlaywrightScriptPromise

const getAutoconsentPlaywrightScript = () => {
  if (autoconsentPlaywrightScriptPromise) return autoconsentPlaywrightScriptPromise  // <-- BUG 🔴 Returns cached rejected promise (no retry)

  autoconsentPlaywrightScriptPromise = fs.readFile(
    path.resolve(
      path.dirname(require.resolve('@duckduckgo/autoconsent')),
      'autoconsent.playwright.js'
    ),
    'utf8'
  )
  // <-- BUG 🔴 No .catch() to clear autoconsentPlaywrightScriptPromise on failure

  return autoconsentPlaywrightScriptPromise
}

Explanation

Both functions cache a promise in a module-level variable and return it on subsequent calls. If the first readFile() fails, the promise is rejected and remains stored in the cache variable. Since the code only checks whether the cached value is truthy (not whether it fulfilled), every later call returns the same rejected promise and never retries the read/deserialization.

Failing Test

const path = require('path')
const Module = require('module')

// Simulate a failing file read on first call, then fix it
let callCount = 0
const originalRequire = Module.prototype.require

Module.prototype.require = function(id) {
  if (id === 'fs/promises') {
    const fs = originalRequire.call(this, id)
    return {
      ...fs,
      readFile: async (...args) => {
        callCount++
        if (callCount === 1) {
          throw new Error('ENOENT: no such file or directory')
        } else {
          return fs.readFile(...args)  // Fixed now
        }
      }
    }
  }
  return originalRequire.call(this, id)
}

async function test() {
  const adblockPath = path.resolve(__dirname, 'packages/goto/src/adblock.js')
  delete require.cache[adblockPath]
  const adblock = require(adblockPath)

  // First call fails
  try {
    await adblock.enableBlockingInPage(mockPage, mockRun, 5000)
  } catch (err) {
    console.log('First call failed:', err.message)
  }

  // Second call should succeed (file is "fixed"), but doesn't
  try {
    await adblock.enableBlockingInPage(mockPage, mockRun, 5000)
    console.log('Second call succeeded')
  } catch (err) {
    console.log('Second call ALSO failed:', err.message)  // Bug!
  }
}

Output:

First call failed: ENOENT: no such file or directory
Second call ALSO failed: ENOENT: no such file or directory

Recommended Fix

Clear the cached promise on rejection so subsequent calls retry:

enginePromise = fs.readFile(path.resolve(__dirname, './engine.bin'))
  .then(buffer => {
    const engine = PuppeteerBlocker.deserialize(new Uint8Array(buffer))
    engine.on('request-blocked', ({ url }) => debug('block', url))
    engine.on('request-redirected', ({ url }) => debug('redirect', url))
    return engine
  })
  .catch(err => {
    enginePromise = undefined
    throw err
  })

Apply the same pattern to autoconsentPlaywrightScriptPromise.

History

This bug was introduced in commit 0b1ff2d. The commit converted adblock asset loading from synchronous fs.readFileSync() calls (executed immediately at module load) to asynchronous lazy loading with promise caching to improve startup performance. However, the promise caching implementation forgot to include error handling that clears the cached promise on failure, causing rejected promises to be permanently cached and preventing all future retry attempts.