From 5330ace042058a1dc13771d5791f853e88b3f62e Mon Sep 17 00:00:00 2001 From: Sumit Suthar Date: Fri, 20 Sep 2024 16:30:31 +0530 Subject: [PATCH] Revert "feat: Added support for `express@5` (#2555)" This reverts commit 15b7e5c24bd5244e3cb030cb812b5c0e1263557f. --- 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, 72 insertions(+), 57 deletions(-) diff --git a/lib/shim/shim.js b/lib/shim/shim.js index 9505a28872..54a7fca45a 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 != null && (obj instanceof Object || (!obj.constructor && typeof obj === 'object')) + return obj instanceof Object } /** diff --git a/test/unit/shim/shim.test.js b/test/unit/shim/shim.test.js index 226bf507f7..7e89320a46 100644 --- a/test/unit/shim/shim.test.js +++ b/test/unit/shim/shim.test.js @@ -2545,20 +2545,16 @@ tap.test('Shim', function (t) { t.beforeEach(beforeEach) t.afterEach(afterEach) t.test('should detect if an item is an object', function (t) { - 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.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.end() }) }) diff --git a/test/versioned/express-esm/package.json b/test/versioned/express-esm/package.json index 924c057779..4232eb1421 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", + "versions": ">=4.6.0 && <5.0.0", "samples": 5 } }, @@ -19,6 +19,18 @@ "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 f497e87f7c..5751a09893 100644 --- a/test/versioned/express-esm/segments.tap.mjs +++ b/test/versioned/express-esm/segments.tap.mjs @@ -20,7 +20,8 @@ const assertSegmentsOptions = { // const pkgVersion = expressPkg.version import { readFileSync } from 'node:fs' const { version: pkgVersion } = JSON.parse(readFileSync('./node_modules/express/package.json')) -const isExpress5 = semver.gte(pkgVersion, '5.0.0') +// TODO: change to 5.0.0 when officially released +const isExpress5 = semver.gte(pkgVersion, '5.0.0-beta.3') test('transaction segments tests', (t) => { t.autoend() @@ -214,25 +215,14 @@ test('transaction segments tests', (t) => { res.send('test') }) - 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 = '/(.*)' - } - + const path = isExpress5 ? '(.*)' : '*' app.get(path, router1) const { rootSegment, transaction } = await runTest({ app, t }) t.assertSegments( rootSegment, [ - `Expressjs/Route Path: ${segmentPath}`, + `Expressjs/Route Path: /${path}`, [ 'Expressjs/Router: /', ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'testHandler']] @@ -244,8 +234,8 @@ test('transaction segments tests', (t) => { checkMetrics( t, transaction.metrics, - [`${NAMES.EXPRESS.MIDDLEWARE}testHandler/${metricsPath}/test`], - `${metricsPath}/test` + [`${NAMES.EXPRESS.MIDDLEWARE}testHandler//${path}/test`], + `/${path}/test` ) }) diff --git a/test/versioned/express-esm/transaction-naming.tap.mjs b/test/versioned/express-esm/transaction-naming.tap.mjs index aef6bce861..30e2bb3890 100644 --- a/test/versioned/express-esm/transaction-naming.tap.mjs +++ b/test/versioned/express-esm/transaction-naming.tap.mjs @@ -20,7 +20,8 @@ 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')) -const isExpress5 = semver.gte(pkgVersion, '5.0.0') +// TODO: change to 5.0.0 when officially released +const isExpress5 = semver.gte(pkgVersion, '5.0.0-beta.3') 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 56014e6bc5..adce020ebb 100644 --- a/test/versioned/express/async-error.tap.js +++ b/test/versioned/express/async-error.tap.js @@ -8,7 +8,6 @@ const path = require('path') const test = require('tap').test const fork = require('child_process').fork -const { isExpress5 } = require('./utils') /* * @@ -17,7 +16,7 @@ const { isExpress5 } = require('./utils') */ const COMPLETION = 27 -test('Express async throw', { skip: isExpress5() }, function (t) { +test('Express async throw', 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 b90df9dc55..eb9050c5be 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, isExpress5 } = require('./utils') +const { makeRequest, setup } = require('./utils') const { test } = require('tap') -test('should properly track async handlers', { skip: !isExpress5() }, (t) => { +test('should properly track async handlers', (t) => { setup(t) const { app } = t.context const mwTimeout = 20 @@ -44,7 +44,7 @@ test('should properly track async handlers', { skip: !isExpress5() }, (t) => { }) }) -test('should properly handle errors in async handlers', { skip: !isExpress5() }, (t) => { +test('should properly handle errors in async handlers', (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 7b2643263d..c77056f9e0 100644 --- a/test/versioned/express/express-enrouten.tap.js +++ b/test/versioned/express/express-enrouten.tap.js @@ -10,9 +10,8 @@ const test = require('tap').test const helper = require('../../lib/agent_helper') -const { isExpress5 } = require('./utils') -test('Express + express-enrouten compatibility test', { skip: isExpress5() }, function (t) { +test('Express + express-enrouten compatibility test', function (t) { t.plan(2) const agent = helper.instrumentMockedAgent() diff --git a/test/versioned/express/package.json b/test/versioned/express/package.json index 694ed62abe..cedb647048 100644 --- a/test/versioned/express/package.json +++ b/test/versioned/express/package.json @@ -15,7 +15,7 @@ }, "dependencies": { "express": { - "versions": ">=4.6.0", + "versions": ">=4.6.0 && <5.0.0", "samples": 5 }, "express-enrouten": "1.1", @@ -24,7 +24,6 @@ "files": [ "app-use.tap.js", "async-error.tap.js", - "async-handlers.tap.js", "bare-router.tap.js", "captures-params.tap.js", "client-disconnect.tap.js", @@ -41,6 +40,34 @@ "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", + "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" + ] } ] } diff --git a/test/versioned/express/segments.tap.js b/test/versioned/express/segments.tap.js index 2092862c50..74db64a938 100644 --- a/test/versioned/express/segments.tap.js +++ b/test/versioned/express/segments.tap.js @@ -243,17 +243,7 @@ test('router mounted as a route handler', function (t) { res.send('test') }) - 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 = '/(.*)' - } + const path = isExpress5 ? '(.*)' : '*' app.get(path, router1) runTest(t, '/test', function (segments, transaction) { @@ -261,7 +251,7 @@ test('router mounted as a route handler', function (t) { t, transaction.trace.root.children[0], [ - `Expressjs/Route Path: ${segmentPath}`, + `Expressjs/Route Path: /${path}`, [ 'Expressjs/Router: /', ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'testHandler']] @@ -273,8 +263,8 @@ test('router mounted as a route handler', function (t) { checkMetrics( t, transaction.metrics, - [`${NAMES.EXPRESS.MIDDLEWARE}testHandler/${metricsPath}/test`], - `${metricsPath}/test` + [`${NAMES.EXPRESS.MIDDLEWARE}testHandler//${path}/test`], + `/${path}/test` ) t.end() diff --git a/test/versioned/express/utils.js b/test/versioned/express/utils.js index 18c39b3856..8221494c11 100644 --- a/test/versioned/express/utils.js +++ b/test/versioned/express/utils.js @@ -10,7 +10,8 @@ const semver = require('semver') function isExpress5() { const { version } = require('express/package') - return semver.gte(version, '5.0.0') + // TODO: change to 5.0.0 when officially released + return semver.gte(version, '5.0.0-beta.3') } function makeRequest(server, path, callback) {