[DI] Remove source map cache in Node.js 18 (#5302)

* [DI] Remove source map cache for Node.js 18

This is the second attempt at a workaround for a bug in Node.js core,
that sometimes triggers when, among other things, a timer is created.
This makes the call to `setTimeout` throw an un-catchable error from
within `AsyncLocalStorage._propagate` with the following TypeError:

    Cannot read properties of undefined (reading 'Symbol(kResourceStore)')

* Adjust cache timings

Author: watson

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

@@ -4,10 +4,11 @@ const { join, dirname } = require('path')
 const { readFileSync } = require('fs')
 const { readFile } = require('fs/promises')
 const { SourceMapConsumer } = require('source-map')
+const { NODE_MAJOR } = require('../../../../../version')
 
 const cache = new Map()
 let cacheTimer = null
-let cacheTimerLastSet = 0
+let cacheTime = null
 
 const self = module.exports = {
   async loadSourceMap (dir, url) {
@@ -34,28 +35,32 @@ const self = module.exports = {
   }
 }
 
-// TODO: Remove if-statement around `setTimeout` below once it's safe to do so.
-//
-// 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 error message:
+// The version check inside this function is to guard against a bug Node.js version 18, in which calls to `setTimeout`
+// might throw an uncatchable error from within `AsyncLocalStorage._propagate` with the following error message:
 //
 //     TypeError: Cannot read properties of undefined (reading 'Symbol(kResourceStore)')
-//
-// Source: https://github.com/nodejs/node/blob/v18.20.6/lib/async_hooks.js#L312
 function cacheIt (key, value) {
-  const now = Date.now()
-  if (now > cacheTimerLastSet + 1_000) {
-    clearTimeout(cacheTimer)
-    cacheTimer = setTimeout(function () {
+  if (NODE_MAJOR < 20) return value
+  cacheTime = Date.now()
+  setCacheTTL()
+  cache.set(key, value)
+  return value
+}
+
+function setCacheTTL () {
+  if (cacheTimer !== null) return
+
+  cacheTimer = setTimeout(function () {
+    cacheTimer = null
+    if (Date.now() - cacheTime < 2_500) {
+      // If the last cache entry was added recently, keep the cache alive
+      setCacheTTL()
+    } else {
       // Optimize for app boot, where a lot of reads might happen
       // Clear cache a few seconds after it was last used
       cache.clear()
-    }, 10_000).unref()
-    cacheTimerLastSet = now
-  }
-  cache.set(key, value)
-  return value
+    }
+  }, 5_000).unref()
 }
 
 function loadInlineSourceMap (data) {

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

@@ -1,6 +1,7 @@
 'use strict'
 
 require('../../setup/mocha')
+const { NODE_MAJOR } = require('../../../../../version')
 
 const parsedSourceMap = {
   version: 3,
@@ -94,13 +95,62 @@ describe('source map utils', function () {
     })
   })
 
+  describe('no cache', function () {
+    function setup () {
+      readFileSync = sinon.stub().returns(rawSourceMap)
+      readFile = sinon.stub().resolves(rawSourceMap)
+
+      const sourceMaps = proxyquire('../src/debugger/devtools_client/source-maps', {
+        fs: { readFileSync },
+        'fs/promises': { readFile }
+      })
+
+      loadSourceMap = sourceMaps.loadSourceMap
+      loadSourceMapSync = sourceMaps.loadSourceMapSync
+    }
+
+    before(function () {
+      if (NODE_MAJOR > 18) this.skip()
+    })
+
+    describe('loadSourceMap', function () {
+      before(setup)
+
+      it('should read from disk on the fist call', async function () {
+        const sourceMap = await loadSourceMap(dir, sourceMapURL)
+        expect(sourceMap).to.deep.equal(parsedSourceMap)
+        expect(readFile.callCount).to.equal(1)
+      })
+
+      it('should read from disk on the second call', async function () {
+        const sourceMap = await loadSourceMap(dir, sourceMapURL)
+        expect(sourceMap).to.deep.equal(parsedSourceMap)
+        expect(readFile.callCount).to.equal(2)
+      })
+    })
+
+    describe('loadSourceMapSync', function () {
+      before(setup)
+
+      it('should read from disk on the fist call', function () {
+        const sourceMap = loadSourceMapSync(dir, sourceMapURL)
+        expect(sourceMap).to.deep.equal(parsedSourceMap)
+        expect(readFileSync.callCount).to.equal(1)
+      })
+
+      it('should read from disk on the second call', function () {
+        const sourceMap = loadSourceMapSync(dir, sourceMapURL)
+        expect(sourceMap).to.deep.equal(parsedSourceMap)
+        expect(readFileSync.callCount).to.equal(2)
+      })
+    })
+  })
+
   describe('cache', function () {
     let clock
 
     function setup () {
-      clock = sinon.useFakeTimers({
-        toFake: ['setTimeout']
-      })
+      clock = sinon.useFakeTimers()
       readFileSync = sinon.stub().returns(rawSourceMap)
       readFile = sinon.stub().resolves(rawSourceMap)
 
@@ -117,6 +167,10 @@ describe('source map utils', function () {
       clock.restore()
     }
 
+    before(function () {
+      if (NODE_MAJOR < 20) this.skip()
+    })
+
     describe('loadSourceMap', function () {
       before(setup)
 
@@ -134,8 +188,8 @@ describe('source map utils', function () {
         expect(readFile.callCount).to.equal(1)
       })
 
-      it('should clear cache after 10 seconds', async function () {
-        clock.tick(10_000)
+      it('should clear cache after 5 seconds', async function () {
+        clock.tick(5_000)
         const sourceMap = await loadSourceMap(dir, sourceMapURL)
         expect(sourceMap).to.deep.equal(parsedSourceMap)
         expect(readFile.callCount).to.equal(2)
@@ -159,8 +213,8 @@ describe('source map utils', function () {
         expect(readFileSync.callCount).to.equal(1)
       })
 
-      it('should clear cache after 10 seconds', function () {
-        clock.tick(10_000)
+      it('should clear cache after 5 seconds', function () {
+        clock.tick(5_000)
         const sourceMap = loadSourceMapSync(dir, sourceMapURL)
         expect(sourceMap).to.deep.equal(parsedSourceMap)
         expect(readFileSync.callCount).to.equal(2)