Skip to content

Commit 77abd1e

Browse files
authored
[DI] Workaround bug in AsyncLocalStorage which would otherwise throw (#5290)
This is a workaround for, what seems like a bug in Node.js core, that seems to trigger when, among other things, a lot of timers are being created very rapidly. This makes the call to `setTimeout` throw an error from within `AsyncLocalStorage._propagate` with the following TypeError: Cannot read properties of undefined (reading 'Symbol(kResourceStore)')
1 parent 172bc66 commit 77abd1e

File tree

4 files changed

+27
-11
lines changed

4 files changed

+27
-11
lines changed

.github/workflows/system-tests.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
uses: DataDog/system-tests/.github/workflows/compute-workflow-parameters.yml@main
3232
with:
3333
library: nodejs
34-
scenarios_groups: essentials,appsec_rasp
34+
scenarios_groups: essentials,appsec_rasp,debugger
3535

3636
system-tests:
3737
runs-on: ${{ contains(fromJSON('["CROSSED_TRACING_LIBRARIES", "INTEGRATIONS"]'), matrix.scenario) && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}

packages/dd-trace/src/debugger/devtools_client/breakpoints.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ async function addBreakpoint (probe) {
3131
probe.nsBetweenSampling = BigInt(1 / snapshotsPerSecond * 1e9)
3232
probe.lastCaptureNs = 0n
3333

34-
// TODO: Inbetween `await session.post('Debugger.enable')` and here, the scripts are parsed and cached.
35-
// Maybe there's a race condition here or maybe we're guraenteed that `await session.post('Debugger.enable')` will
36-
// not continue untill all scripts have been parsed?
34+
// Warning: The code below relies on undocumented behavior of the inspector!
35+
// It expects that `await session.post('Debugger.enable')` will wait for all loaded scripts to be emitted as
36+
// `Debugger.scriptParsed` events. If this ever changes, we will have a race condition!
3737
const script = findScriptFromPartialPath(file)
3838
if (!script) throw new Error(`No loaded script found for ${file} (probe: ${probe.id}, version: ${probe.version})`)
3939
const { url, scriptId, sourceMapURL, source } = script

packages/dd-trace/src/debugger/devtools_client/source-maps.js

+20-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const { SourceMapConsumer } = require('source-map')
77

88
const cache = new Map()
99
let cacheTimer = null
10+
let cacheTimerLastSet = 0
1011

1112
const self = module.exports = {
1213
async loadSourceMap (dir, url) {
@@ -33,13 +34,26 @@ const self = module.exports = {
3334
}
3435
}
3536

37+
// TODO: Remove if-statement around `setTimeout` below once it's safe to do so.
38+
//
39+
// This is a workaround for, what seems like a bug in Node.js core, that seems to trigger when, among other things, a
40+
// lot of timers are being created very rapidly. This makes the call to `setTimeout` throw an error from within
41+
// `AsyncLocalStorage._propagate` with the following error message:
42+
//
43+
// TypeError: Cannot read properties of undefined (reading 'Symbol(kResourceStore)')
44+
//
45+
// Source: https://github.com/nodejs/node/blob/v18.20.6/lib/async_hooks.js#L312
3646
function cacheIt (key, value) {
37-
clearTimeout(cacheTimer)
38-
cacheTimer = setTimeout(function () {
39-
// Optimize for app boot, where a lot of reads might happen
40-
// Clear cache a few seconds after it was last used
41-
cache.clear()
42-
}, 10_000).unref()
47+
const now = Date.now()
48+
if (now > cacheTimerLastSet + 1_000) {
49+
clearTimeout(cacheTimer)
50+
cacheTimer = setTimeout(function () {
51+
// Optimize for app boot, where a lot of reads might happen
52+
// Clear cache a few seconds after it was last used
53+
cache.clear()
54+
}, 10_000).unref()
55+
cacheTimerLastSet = now
56+
}
4357
cache.set(key, value)
4458
return value
4559
}

packages/dd-trace/test/debugger/devtools_client/source-maps.spec.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ describe('source map utils', function () {
9898
let clock
9999

100100
function setup () {
101-
clock = sinon.useFakeTimers()
101+
clock = sinon.useFakeTimers({
102+
toFake: ['setTimeout']
103+
})
102104
readFileSync = sinon.stub().returns(rawSourceMap)
103105
readFile = sinon.stub().resolves(rawSourceMap)
104106

0 commit comments

Comments
 (0)