From bc11b3152b7334881d76d4ce77eb2316234e1ebd Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Tue, 10 Sep 2024 13:10:02 -0400 Subject: [PATCH] feat: Added support for `express@5` --- lib/shim/shim.js | 2 +- test/unit/shim/shim.test.js | 24 ++++++++------ test/versioned/express-esm/package.json | 14 +-------- test/versioned/express-esm/segments.tap.mjs | 22 +++++++++---- .../express-esm/transaction-naming.tap.mjs | 3 +- test/versioned/express/async-error.tap.js | 3 +- test/versioned/express/async-handlers.tap.js | 6 ++-- .../versioned/express/express-enrouten.tap.js | 3 +- test/versioned/express/package.json | 31 ++----------------- test/versioned/express/segments.tap.js | 18 ++++++++--- test/versioned/express/utils.js | 3 +- 11 files changed, 57 insertions(+), 72 deletions(-) diff --git a/lib/shim/shim.js b/lib/shim/shim.js index 54a7fca45a..9505a28872 100644 --- a/lib/shim/shim.js +++ b/lib/shim/shim.js @@ -1300,7 +1300,7 @@ function getName(obj) { * @returns {boolean} True if the object is an Object, else false. */ function isObject(obj) { - return obj instanceof Object + return obj != null && (obj instanceof Object || (!obj.constructor && typeof obj === 'object')) } /** diff --git a/test/unit/shim/shim.test.js b/test/unit/shim/shim.test.js index 7e89320a46..226bf507f7 100644 --- a/test/unit/shim/shim.test.js +++ b/test/unit/shim/shim.test.js @@ -2545,16 +2545,20 @@ tap.test('Shim', function (t) { t.beforeEach(beforeEach) t.afterEach(afterEach) t.test('should detect if an item is an object', function (t) { - t.ok(shim.isObject({})) - t.ok(shim.isObject([])) - t.ok(shim.isObject(arguments)) - t.ok(shim.isObject(function () {})) - t.notOk(shim.isObject(true)) - t.notOk(shim.isObject(false)) - t.notOk(shim.isObject('foobar')) - t.notOk(shim.isObject(1234)) - t.notOk(shim.isObject(null)) - t.notOk(shim.isObject(undefined)) + t.equal(shim.isObject({}), true) + t.equal(shim.isObject([]), true) + t.equal(shim.isObject(arguments), true) + t.equal( + shim.isObject(function () {}), + true + ) + t.equal(shim.isObject(Object.create(null)), true) + t.equal(shim.isObject(true), false) + t.equal(shim.isObject(false), false) + t.equal(shim.isObject('foobar'), false) + t.equal(shim.isObject(1234), false) + t.equal(shim.isObject(null), false) + t.equal(shim.isObject(undefined), false) t.end() }) }) diff --git a/test/versioned/express-esm/package.json b/test/versioned/express-esm/package.json index 4232eb1421..924c057779 100644 --- a/test/versioned/express-esm/package.json +++ b/test/versioned/express-esm/package.json @@ -11,7 +11,7 @@ }, "dependencies": { "express": { - "versions": ">=4.6.0 && <5.0.0", + "versions": ">=4.6.0", "samples": 5 } }, @@ -19,18 +19,6 @@ "segments.tap.mjs", "transaction-naming.tap.mjs" ] - }, - { - "engines": { - "node": ">=18" - }, - "dependencies": { - "express": "5.0.0-beta.3" - }, - "files": [ - "segments.tap.mjs", - "transaction-naming.tap.mjs" - ] } ], "dependencies": {} diff --git a/test/versioned/express-esm/segments.tap.mjs b/test/versioned/express-esm/segments.tap.mjs index 5751a09893..f497e87f7c 100644 --- a/test/versioned/express-esm/segments.tap.mjs +++ b/test/versioned/express-esm/segments.tap.mjs @@ -20,8 +20,7 @@ const assertSegmentsOptions = { // const pkgVersion = expressPkg.version import { readFileSync } from 'node:fs' const { version: pkgVersion } = JSON.parse(readFileSync('./node_modules/express/package.json')) -// TODO: change to 5.0.0 when officially released -const isExpress5 = semver.gte(pkgVersion, '5.0.0-beta.3') +const isExpress5 = semver.gte(pkgVersion, '5.0.0') test('transaction segments tests', (t) => { t.autoend() @@ -215,14 +214,25 @@ test('transaction segments tests', (t) => { res.send('test') }) - const path = isExpress5 ? '(.*)' : '*' + let path = '*' + let segmentPath = '/*' + let metricsPath = segmentPath + + // express 5 router must be regular expressions + // need to handle the nuance of the segment vs metric name in express 5 + if (isExpress5) { + path = /(.*)/ + segmentPath = '/(.*)/' + metricsPath = '/(.*)' + } + app.get(path, router1) const { rootSegment, transaction } = await runTest({ app, t }) t.assertSegments( rootSegment, [ - `Expressjs/Route Path: /${path}`, + `Expressjs/Route Path: ${segmentPath}`, [ 'Expressjs/Router: /', ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'testHandler']] @@ -234,8 +244,8 @@ test('transaction segments tests', (t) => { checkMetrics( t, transaction.metrics, - [`${NAMES.EXPRESS.MIDDLEWARE}testHandler//${path}/test`], - `/${path}/test` + [`${NAMES.EXPRESS.MIDDLEWARE}testHandler/${metricsPath}/test`], + `${metricsPath}/test` ) }) diff --git a/test/versioned/express-esm/transaction-naming.tap.mjs b/test/versioned/express-esm/transaction-naming.tap.mjs index 30e2bb3890..aef6bce861 100644 --- a/test/versioned/express-esm/transaction-naming.tap.mjs +++ b/test/versioned/express-esm/transaction-naming.tap.mjs @@ -20,8 +20,7 @@ const { setup, makeRequest, makeRequestAndFinishTransaction } = expressHelpers // const pkgVersion = expressPkg.version import { readFileSync } from 'node:fs' const { version: pkgVersion } = JSON.parse(readFileSync('./node_modules/express/package.json')) -// TODO: change to 5.0.0 when officially released -const isExpress5 = semver.gte(pkgVersion, '5.0.0-beta.3') +const isExpress5 = semver.gte(pkgVersion, '5.0.0') test('transaction naming tests', (t) => { t.autoend() diff --git a/test/versioned/express/async-error.tap.js b/test/versioned/express/async-error.tap.js index adce020ebb..56014e6bc5 100644 --- a/test/versioned/express/async-error.tap.js +++ b/test/versioned/express/async-error.tap.js @@ -8,6 +8,7 @@ const path = require('path') const test = require('tap').test const fork = require('child_process').fork +const { isExpress5 } = require('./utils') /* * @@ -16,7 +17,7 @@ const fork = require('child_process').fork */ const COMPLETION = 27 -test('Express async throw', function (t) { +test('Express async throw', { skip: isExpress5() }, function (t) { const erk = fork(path.join(__dirname, 'erk.js')) let timer diff --git a/test/versioned/express/async-handlers.tap.js b/test/versioned/express/async-handlers.tap.js index eb9050c5be..b90df9dc55 100644 --- a/test/versioned/express/async-handlers.tap.js +++ b/test/versioned/express/async-handlers.tap.js @@ -5,10 +5,10 @@ 'use strict' -const { makeRequest, setup } = require('./utils') +const { makeRequest, setup, isExpress5 } = require('./utils') const { test } = require('tap') -test('should properly track async handlers', (t) => { +test('should properly track async handlers', { skip: !isExpress5() }, (t) => { setup(t) const { app } = t.context const mwTimeout = 20 @@ -44,7 +44,7 @@ test('should properly track async handlers', (t) => { }) }) -test('should properly handle errors in async handlers', (t) => { +test('should properly handle errors in async handlers', { skip: !isExpress5() }, (t) => { setup(t) const { app } = t.context diff --git a/test/versioned/express/express-enrouten.tap.js b/test/versioned/express/express-enrouten.tap.js index c77056f9e0..7b2643263d 100644 --- a/test/versioned/express/express-enrouten.tap.js +++ b/test/versioned/express/express-enrouten.tap.js @@ -10,8 +10,9 @@ const test = require('tap').test const helper = require('../../lib/agent_helper') +const { isExpress5 } = require('./utils') -test('Express + express-enrouten compatibility test', function (t) { +test('Express + express-enrouten compatibility test', { skip: isExpress5() }, function (t) { t.plan(2) const agent = helper.instrumentMockedAgent() diff --git a/test/versioned/express/package.json b/test/versioned/express/package.json index cedb647048..694ed62abe 100644 --- a/test/versioned/express/package.json +++ b/test/versioned/express/package.json @@ -15,7 +15,7 @@ }, "dependencies": { "express": { - "versions": ">=4.6.0 && <5.0.0", + "versions": ">=4.6.0", "samples": 5 }, "express-enrouten": "1.1", @@ -24,39 +24,12 @@ "files": [ "app-use.tap.js", "async-error.tap.js", - "bare-router.tap.js", - "captures-params.tap.js", - "client-disconnect.tap.js", - "errors.tap.js", - "express-enrouten.tap.js", - "ignoring.tap.js", - "issue171.tap.js", - "middleware-name.tap.js", - "render.tap.js", - "require.tap.js", - "route-iteration.tap.js", - "route-param.tap.js", - "router-params.tap.js", - "segments.tap.js", - "transaction-naming.tap.js" - ] - }, - { - "engines": { - "node": ">=18" - }, - "dependencies": { - "express": "5.0.0-beta.3", - "ejs": "2.5.9" - }, - "files": [ - "app-use.tap.js", "async-handlers.tap.js", - "async-error.tap.js", "bare-router.tap.js", "captures-params.tap.js", "client-disconnect.tap.js", "errors.tap.js", + "express-enrouten.tap.js", "ignoring.tap.js", "issue171.tap.js", "middleware-name.tap.js", diff --git a/test/versioned/express/segments.tap.js b/test/versioned/express/segments.tap.js index 74db64a938..2092862c50 100644 --- a/test/versioned/express/segments.tap.js +++ b/test/versioned/express/segments.tap.js @@ -243,7 +243,17 @@ test('router mounted as a route handler', function (t) { res.send('test') }) - const path = isExpress5 ? '(.*)' : '*' + let path = '*' + let segmentPath = '/*' + let metricsPath = segmentPath + + // express 5 router must be regular expressions + // need to handle the nuance of the segment vs metric name in express 5 + if (isExpress5) { + path = /(.*)/ + segmentPath = '/(.*)/' + metricsPath = '/(.*)' + } app.get(path, router1) runTest(t, '/test', function (segments, transaction) { @@ -251,7 +261,7 @@ test('router mounted as a route handler', function (t) { t, transaction.trace.root.children[0], [ - `Expressjs/Route Path: /${path}`, + `Expressjs/Route Path: ${segmentPath}`, [ 'Expressjs/Router: /', ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'testHandler']] @@ -263,8 +273,8 @@ test('router mounted as a route handler', function (t) { checkMetrics( t, transaction.metrics, - [`${NAMES.EXPRESS.MIDDLEWARE}testHandler//${path}/test`], - `/${path}/test` + [`${NAMES.EXPRESS.MIDDLEWARE}testHandler/${metricsPath}/test`], + `${metricsPath}/test` ) t.end() diff --git a/test/versioned/express/utils.js b/test/versioned/express/utils.js index 8221494c11..18c39b3856 100644 --- a/test/versioned/express/utils.js +++ b/test/versioned/express/utils.js @@ -10,8 +10,7 @@ const semver = require('semver') function isExpress5() { const { version } = require('express/package') - // TODO: change to 5.0.0 when officially released - return semver.gte(version, '5.0.0-beta.3') + return semver.gte(version, '5.0.0') } function makeRequest(server, path, callback) {