[Detail Bug] Adblock initialization permanently fails after a single transient asset load error
# 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
```javascript
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
}
```
```javascript
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
```javascript
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:
```javascript
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 0b1ff2d2. 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.
0 条评论