Skip to content

Commit

Permalink
feat: Added support for express@5 (#2555)
Browse files Browse the repository at this point in the history
  • Loading branch information
bizob2828 authored Sep 10, 2024
1 parent 44edd0c commit 252f3b2
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 72 deletions.
2 changes: 1 addition & 1 deletion lib/shim/shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
}

/**
Expand Down
24 changes: 14 additions & 10 deletions test/unit/shim/shim.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
Expand Down
14 changes: 1 addition & 13 deletions test/versioned/express-esm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,14 @@
},
"dependencies": {
"express": {
"versions": ">=4.6.0 && <5.0.0",
"versions": ">=4.6.0",
"samples": 5
}
},
"files": [
"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": {}
Expand Down
22 changes: 16 additions & 6 deletions test/versioned/express-esm/segments.tap.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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']]
Expand All @@ -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`
)
})

Expand Down
3 changes: 1 addition & 2 deletions test/versioned/express-esm/transaction-naming.tap.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion test/versioned/express/async-error.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const path = require('path')
const test = require('tap').test
const fork = require('child_process').fork
const { isExpress5 } = require('./utils')

/*
*
Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions test/versioned/express/async-handlers.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion test/versioned/express/express-enrouten.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
31 changes: 2 additions & 29 deletions test/versioned/express/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"dependencies": {
"express": {
"versions": ">=4.6.0 && <5.0.0",
"versions": ">=4.6.0",
"samples": 5
},
"express-enrouten": "1.1",
Expand All @@ -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",
Expand Down
18 changes: 14 additions & 4 deletions test/versioned/express/segments.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,25 @@ 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) {
checkSegments(
t,
transaction.trace.root.children[0],
[
`Expressjs/Route Path: /${path}`,
`Expressjs/Route Path: ${segmentPath}`,
[
'Expressjs/Router: /',
['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'testHandler']]
Expand All @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions test/versioned/express/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 252f3b2

Please sign in to comment.