From c281de1bf9a1ec6c5cf7086ebca5edadfece39cd Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Thu, 7 Mar 2024 11:33:29 -0300 Subject: [PATCH 1/3] Don't look for for non-top frame locals in node >=18 during locals tests --- test/server.locals.test.js | 98 +++++++++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 27 deletions(-) diff --git a/test/server.locals.test.js b/test/server.locals.test.js index 39036bdb7..a5c4a268e 100644 --- a/test/server.locals.test.js +++ b/test/server.locals.test.js @@ -131,14 +131,19 @@ function verifyThrownError(r) { assert.isTrue(addItemStub.called); var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'node error'); - if (nodeMajorVersion >= 10) { - // Node 10+; locals enabled - var length = data.body.trace_chain[0].frames.length; + var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); + + if (nodeMajorVersion >= 18) { + // Node >=18; locals only in top frame + assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); + assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); + } else if (nodeMajorVersion >= 10) { + // Node >=10; locals enabled assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); assert.equal(data.body.trace_chain[0].frames[length-2].locals.timer, ''); } else { - // Node 8; locals disabled - var length = data.body.trace_chain[0].frames.length; + // Node <=8; locals disabled assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); } @@ -151,13 +156,18 @@ function verifyCaughtError(r) { assert.isTrue(addItemStub.called); var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'caught error'); - if (nodeMajorVersion >= 10) { - // Node 10+; locals enabled - var length = data.body.trace_chain[0].frames.length; + var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); + + if (nodeMajorVersion >= 18) { + // Node >=18; locals only in top frame + assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); + assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); + } else if (nodeMajorVersion >= 10) { + // Node 10..<18; locals enabled assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); assert.equal(data.body.trace_chain[0].frames[length-2].locals.timer, ''); } else { - var length = data.body.trace_chain[0].frames.length; assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); } @@ -171,13 +181,30 @@ function verifyNestedError(r) { var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'test error'); assert.equal(data.body.trace_chain[1].exception.message, 'nested test error'); - if (nodeMajorVersion >= 10) { - // Node 10+; locals enabled - var length = data.body.trace_chain[0].frames.length; + var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); + + if (nodeMajorVersion >= 18) { + // Node >=18; locals only in top frame + assert.equal(data.body.trace_chain[0].frames[length-1].locals.message, 'test error'); + assert.equal(data.body.trace_chain[0].frames[length-1].locals.password, '********'); + assert.equal(data.body.trace_chain[0].frames[length-1].locals.err, ''); + assert.equal(data.body.trace_chain[0].frames[length-1].locals.newMessage, 'nested test error'); + assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); + + length = data.body.trace_chain[1].frames.length; + assert.ok(length > 1); + assert.equal(data.body.trace_chain[1].frames[length-1].locals.nestedMessage, 'nested test error'); + assert.equal(data.body.trace_chain[1].frames[length-1].locals._password, '123456'); + assert.equal(data.body.trace_chain[1].frames[length-1].locals.nestedError, ''); + assert.equal(data.body.trace_chain[1].frames[length-2].locals, undefined); + } else if (nodeMajorVersion >= 10) { + // Node >=10; locals enabled assert.equal(data.body.trace_chain[0].frames[length-1].locals.err, ''); assert.equal(data.body.trace_chain[0].frames[length-2].locals.timer, ''); length = data.body.trace_chain[1].frames.length; + assert.ok(length > 1); assert.equal(data.body.trace_chain[1].frames[length-1].locals.nestedMessage, 'nested test error'); assert.equal(data.body.trace_chain[1].frames[length-1].locals.nestedError, ''); assert.equal(data.body.trace_chain[1].frames[length-2].locals.message, 'test error'); @@ -185,11 +212,10 @@ function verifyNestedError(r) { assert.equal(data.body.trace_chain[1].frames[length-2].locals.err, ''); assert.equal(data.body.trace_chain[1].frames[length-2].locals.newMessage, 'nested test error'); } else { - // Node 8; locals disabled - var length = data.body.trace_chain[0].frames.length; + // Node <=8; locals disabled assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); -} + } addItemStub.restore(); } @@ -199,19 +225,26 @@ function verifyRejectedPromise(r) { assert.isTrue(addItemStub.called); var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'promise reject'); - if (nodeMajorVersion >= 10) { - // Node 10+; locals enabled - var length = data.body.trace_chain[0].frames.length; + var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); + + if (nodeMajorVersion >= 18) { + // Node >=18; locals only in top frame + assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); + assert.equal(data.body.trace_chain[0].frames[length-1].locals.callback, ''); + assert.equal(data.body.trace_chain[0].frames[length-1].locals.rollbar, ''); + assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); + } else if (nodeMajorVersion >= 10) { + // Node >=10; locals enabled assert.equal(data.body.trace_chain[0].frames[length-1].locals.error, ''); assert.equal(data.body.trace_chain[0].frames[length-1].locals.rollbar, ''); assert.equal(data.body.trace_chain[0].frames[length-2].locals.notifier, ''); assert.equal(data.body.trace_chain[0].frames[length-2].locals.r, ''); } else { - // Node 8; locals disabled - var length = data.body.trace_chain[0].frames.length; + // Node <=8; locals disabled assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); -} + } addItemStub.restore(); } @@ -245,18 +278,21 @@ vows.describe('locals') topic: function(_err, r) { r.configure({ locals: { enabled: false }}); var notifier = r.client.notifier; + assert.ok(notifier); r.addItemStub = sinon.stub(notifier.queue, 'addItem'); nodeThrowNested(r, this.callback); }, 'should not include locals': function(_err, r) { var addItemStub = r.addItemStub; + assert.ok(addItemStub); assert.isTrue(addItemStub.called); var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'test error'); assert.equal(data.body.trace_chain[1].exception.message, 'nested test error'); var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); addItemStub.restore(); @@ -299,6 +335,7 @@ vows.describe('locals') var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'caught error'); var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); @@ -445,15 +482,22 @@ vows.describe('locals') assert.isTrue(addItemStub.called); var data = addItemStub.getCall(0).args[3].data; assert.equal(data.body.trace_chain[0].exception.message, 'deep stack error, limit=3'); - if (nodeMajorVersion < 10) { - // Node 8; locals disabled - var length = data.body.trace_chain[0].frames.length; - assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); - } else { - var length = data.body.trace_chain[0].frames.length; + var length = data.body.trace_chain[0].frames.length; + assert.ok(length > 1); + + if (nodeMajorVersion >= 18) { + // Node >=18; locals only in top frame + assert.deepEqual(data.body.trace_chain[0].frames[length-1].locals, { curr: 3, limit: 3 }); + assert.equal(data.body.trace_chain[0].frames[length-2].locals, undefined); + assert.equal(data.body.trace_chain[0].frames[length-3].locals, undefined); + } else if (nodeMajorVersion >= 10) { + // Node >=10; locals enabled assert.deepEqual(data.body.trace_chain[0].frames[length-1].locals, { curr: 3, limit: 3 }); assert.deepEqual(data.body.trace_chain[0].frames[length-2].locals, { curr: 2, limit: 3 }); assert.deepEqual(data.body.trace_chain[0].frames[length-3].locals, { curr: 1, limit: 3 }); + } else { + // Node <=8; locals disabled + assert.equal(data.body.trace_chain[0].frames[length-1].locals, undefined); } addItemStub.reset(); Locals.session = undefined; From a7a467d7e5135953fe4593e263094603b4cdefea Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Thu, 7 Mar 2024 11:38:33 -0300 Subject: [PATCH 2/3] Update CI to test lts nodes from 14 through 20 and non-lts 21 --- .github/workflows/ci.yml | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e58eedb37..601f5414a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,27 +2,42 @@ name: Rollbar.js CI on: push: - branches: [ master ] - tags: [ v* ] + branches: [master] + tags: [v*] pull_request: - branches: [ master ] + branches: [master] jobs: build: runs-on: ubuntu-20.04 + strategy: matrix: - node-version: [10, 12, 14, 16] + include: + - node: 14 + npm: ^8 + - node: 16 + npm: ^8 + - node: 18 + npm: ^9 + - node: 20 + npm: ^10 + - node: latest + npm: latest steps: - - uses: actions/checkout@v2 + - name: Checkout + uses: actions/checkout@v4 with: submodules: recursive - - name: Install node.js - uses: actions/setup-node@v2-beta + - name: Set up node ${{ matrix.node }} + uses: actions/setup-node@v4 with: - node-version: ${{ matrix.node-version }} + node-version: ${{ matrix.node }} + + - name: Update npm + run: npm install -g npm@${{ matrix.npm }} - name: npm install run: npm install From 71b5e3a8d29a07283432b2bce17349afdb4a10ee Mon Sep 17 00:00:00 2001 From: Fujimura Kaito <63333564+fujiyamaorange@users.noreply.github.com> Date: Fri, 8 Mar 2024 00:05:18 +0900 Subject: [PATCH 3/3] [Refactor] Bundle duplicated `replace` function (#1128) --- src/browser/telemetry.js | 9 +-------- src/server/telemetry.js | 15 +-------------- src/utility/replace.js | 9 +++++++++ 3 files changed, 11 insertions(+), 22 deletions(-) create mode 100644 src/utility/replace.js diff --git a/src/browser/telemetry.js b/src/browser/telemetry.js index e4e965781..e4678694e 100644 --- a/src/browser/telemetry.js +++ b/src/browser/telemetry.js @@ -1,5 +1,6 @@ var _ = require('../utility'); var headers = require('../utility/headers'); +var replace = require('../utility/replace'); var scrub = require('../scrub'); var urlparser = require('./url'); var domUtil = require('./domUtility'); @@ -21,14 +22,6 @@ var defaults = { errorOnContentSecurityPolicy: false }; -function replace(obj, name, replacement, replacements, type) { - var orig = obj[name]; - obj[name] = replacement(orig); - if (replacements) { - replacements[type].push([obj, name, orig]); - } -} - function restore(replacements, type) { var b; while (replacements[type].length) { diff --git a/src/server/telemetry.js b/src/server/telemetry.js index 51d396d5d..99264553a 100644 --- a/src/server/telemetry.js +++ b/src/server/telemetry.js @@ -1,6 +1,7 @@ var http = require('http'); var https = require('https'); var _ = require('../utility'); +var replace = require('../utility/replace'); var urlHelpers = require('./telemetry/urlHelpers'); var defaults = { @@ -149,20 +150,6 @@ Instrumenter.prototype.instrumentConsole = function() { }, this.replacements, 'log'); }; -// TODO: These helpers are duplicated in src/browser/telemetry.js, -// and may be candidates for extraction into a shared module. -// It is recommended that before doing so, the author should allow -// for more telemetry types to be implemented for the Node target -// to ensure that the implementations of these helpers don't diverge. -// If they do diverge, there's little point in the shared module. -function replace(obj, name, replacement, replacements, type) { - var orig = obj[name]; - obj[name] = replacement(orig); - if (replacements) { - replacements[type].push([obj, name, orig]); - } -} - function restore(replacements, type) { var b; while (replacements[type].length) { diff --git a/src/utility/replace.js b/src/utility/replace.js new file mode 100644 index 000000000..892bcc25b --- /dev/null +++ b/src/utility/replace.js @@ -0,0 +1,9 @@ +function replace(obj, name, replacement, replacements, type) { + var orig = obj[name]; + obj[name] = replacement(orig); + if (replacements) { + replacements[type].push([obj, name, orig]); + } + } + +module.exports = replace;