diff --git a/test/versioned/express/app-use.tap.js b/test/versioned/express/app-use.test.js similarity index 74% rename from test/versioned/express/app-use.tap.js rename to test/versioned/express/app-use.test.js index 5a6e597691..1c3d127167 100644 --- a/test/versioned/express/app-use.tap.js +++ b/test/versioned/express/app-use.test.js @@ -4,35 +4,38 @@ */ 'use strict' - -const test = require('tap').test +const test = require('node:test') const helper = require('../../lib/agent_helper') const http = require('http') const { isExpress5 } = require('./utils') +const tsplan = require('@matteo.collina/tspl') +const promiseResolvers = require('../../lib/promise-resolvers') // This test is no longer applicable in express 5 as mounting a child router does not emit the same // mount event -test('app should be at top of stack when mounted', { skip: isExpress5 }, function (t) { +test('app should be at top of stack when mounted', { skip: isExpress5() }, function (t, end) { const agent = helper.instrumentMockedAgent() const express = require('express') - t.teardown(() => { + t.after(() => { helper.unloadAgent(agent) }) - t.plan(1) + const plan = tsplan(t, { plan: 1 }) const main = express() const child = express() child.on('mount', function () { - t.equal(main._router.stack.length, 3, '3 middleware functions: query parser, Express, child') + plan.equal(main._router.stack.length, 3, '3 middleware functions: query parser, Express, child') + end() }) main.use(child) }) -test('app should be at top of stack when mounted', function (t) { +test('app should be at top of stack when mounted', async function (t) { + const { promise, resolve } = promiseResolvers() const agent = helper.instrumentMockedAgent() const express = require('express') @@ -42,10 +45,19 @@ test('app should be at top of stack when mounted', function (t) { const router = new express.Router() const router2 = new express.Router() const server = http.createServer(main) + // track how many requests and wait for all to be done + let tests = 0 + + const int = setInterval(() => { + if (tests === 5) { + resolve() + } + }, 10) - t.teardown(function () { + t.after(function () { helper.unloadAgent(agent) server.close() + clearInterval(int) }) main.use('/:app', app) @@ -58,7 +70,7 @@ test('app should be at top of stack when mounted', function (t) { router2.get('/', respond) main.get('/:foo/:bar', respond) - t.plan(10) + const plan = tsplan(t, { plan: 10 }) // store finished transactions const finishedTransactions = {} @@ -70,59 +82,67 @@ test('app should be at top of stack when mounted', function (t) { server.listen(port, function () { const host = 'http://localhost:' + port helper.makeGetRequest(host + '/myApp/myChild/app', function (err, res, body) { - t.notOk(err) - t.equal( + plan.ok(!err) + plan.equal( finishedTransactions[body].nameState.getName(), 'Expressjs/GET//:app/:child/app', 'should set partialName correctly for nested apps' ) + ++tests }) helper.makeGetRequest(host + '/myApp/nestedApp ', function (err, res, body) { - t.notOk(err) - t.equal( + plan.ok(!err) + plan.equal( finishedTransactions[body].nameState.getName(), 'Expressjs/GET//:app/nestedApp', 'should set partialName correctly for deeply nested apps' ) + ++tests }) helper.makeGetRequest(host + '/myApp/myChild/router', function (err, res, body) { - t.notOk(err) - t.equal( + plan.ok(!err) + plan.equal( finishedTransactions[body].nameState.getName(), 'Expressjs/GET//:router/:child/router', 'should set partialName correctly for nested routers' ) + ++tests }) helper.makeGetRequest(host + '/myApp/nestedRouter', function (err, res, body) { - t.notOk(err) - t.equal( + plan.ok(!err) + plan.equal( finishedTransactions[body].nameState.getName(), 'Expressjs/GET//:router/nestedRouter', 'should set partialName correctly for deeply nested routers' ) + ++tests }) helper.makeGetRequest(host + '/foo/bar', function (err, res, body) { - t.notOk(err) - t.equal( + plan.ok(!err) + plan.equal( finishedTransactions[body].nameState.getName(), 'Expressjs/GET//:foo/:bar', 'should reset partialName after a router without a matching route' ) + ++tests }) }) }) function respond(req, res) { - res.send(agent.getTransaction().id) + const tx = agent.getTransaction() + res.send(tx.id) } + + await promise }) -test('should not pass wrong args when transaction is not present', function (t) { - t.plan(5) +test('should not pass wrong args when transaction is not present', function (t, end) { + const plan = tsplan(t, { plan: 5 }) const agent = helper.instrumentMockedAgent() @@ -136,7 +156,7 @@ test('should not pass wrong args when transaction is not present', function (t) main.use('/', router) main.use('/', router2) - t.teardown(function () { + t.after(function () { helper.unloadAgent(agent) server.close() }) @@ -148,18 +168,18 @@ test('should not pass wrong args when transaction is not present', function (t) }) router2.get('/', function (req, res, next) { - t.equal(req, args[0]) - t.equal(res, args[1]) - t.equal(typeof next, 'function') + plan.equal(req, args[0]) + plan.equal(res, args[1]) + plan.equal(typeof next, 'function') res.send('ok') }) helper.randomPort(function (port) { server.listen(port, function () { helper.makeGetRequest('http://localhost:' + port + '/', function (err, res, body) { - t.notOk(err) - t.equal(body, 'ok') - t.end() + plan.ok(!err) + plan.equal(body, 'ok') + end() }) }) }) diff --git a/test/versioned/express/async-error.tap.js b/test/versioned/express/async-error.test.js similarity index 70% rename from test/versioned/express/async-error.tap.js rename to test/versioned/express/async-error.test.js index 56014e6bc5..6b48af509c 100644 --- a/test/versioned/express/async-error.tap.js +++ b/test/versioned/express/async-error.test.js @@ -4,9 +4,9 @@ */ 'use strict' - +const assert = require('node:assert') const path = require('path') -const test = require('tap').test +const test = require('node:test') const fork = require('child_process').fork const { isExpress5 } = require('./utils') @@ -17,26 +17,26 @@ const { isExpress5 } = require('./utils') */ const COMPLETION = 27 -test('Express async throw', { skip: isExpress5() }, function (t) { +test('Express async throw', { skip: isExpress5() }, function (t, end) { const erk = fork(path.join(__dirname, 'erk.js')) let timer erk.on('error', function (error) { - t.fail(error) - t.end() + assert.ok(!error) + end() }) erk.on('exit', function (code) { clearTimeout(timer) - t.notEqual(code, COMPLETION, "request didn't complete") - t.end() + assert.notEqual(code, COMPLETION, "request didn't complete") + end() }) // wait for the child vm to boot erk.on('message', function (message) { if (message === 'ready') { timer = setTimeout(function () { - t.fail('hung waiting for exit') + end(new Error('hung waiting for exit')) erk.kill() }, 1000) timer.unref() diff --git a/test/versioned/express/async-handlers.tap.js b/test/versioned/express/async-handlers.tap.js deleted file mode 100644 index b90df9dc55..0000000000 --- a/test/versioned/express/async-handlers.tap.js +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright 2024 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { makeRequest, setup, isExpress5 } = require('./utils') -const { test } = require('tap') - -test('should properly track async handlers', { skip: !isExpress5() }, (t) => { - setup(t) - const { app } = t.context - const mwTimeout = 20 - const handlerTimeout = 25 - - app.use(async function (req, res, next) { - await new Promise((resolve) => { - setTimeout(() => { - resolve() - }, mwTimeout) - }) - next() - }) - app.use('/test', async function handler(req, res) { - await new Promise((resolve) => { - setTimeout(resolve, handlerTimeout) - }) - res.send('ok') - }) - - runTest(t, '/test', (tx) => { - const [children] = tx.trace.root.children - const [mw, handler] = children.children - t.ok( - Math.ceil(mw.getDurationInMillis()) >= mwTimeout, - `should be at least ${mwTimeout} for middleware segment` - ) - t.ok( - Math.ceil(handler.getDurationInMillis()) >= handlerTimeout, - `should be at least ${handlerTimeout} for handler segment` - ) - t.end() - }) -}) - -test('should properly handle errors in async handlers', { skip: !isExpress5() }, (t) => { - setup(t) - const { app } = t.context - - app.use(() => { - return Promise.reject(new Error('whoops i failed')) - }) - app.use('/test', function handler(req, res) { - t.fail('should not call handler on error') - res.send('ok') - }) - // eslint-disable-next-line no-unused-vars - app.use(function (error, req, res, next) { - res.status(400).end() - }) - - runTest(t, '/test', (tx) => { - const errors = tx.agent.errors.traceAggregator.errors - t.equal(errors.length, 1) - const [error] = errors - t.equal(error[2], 'HttpError 400', 'should return 400 from custom error handler') - t.end() - }) -}) - -function runTest(t, endpoint, callback) { - const { agent, app } = t.context - - agent.on('transactionFinished', callback) - - const server = app.listen(function () { - makeRequest(this, endpoint, function (response) { - response.resume() - }) - }) - - t.teardown(() => { - server.close() - }) -} diff --git a/test/versioned/express/async-handlers.test.js b/test/versioned/express/async-handlers.test.js new file mode 100644 index 0000000000..449f2ed261 --- /dev/null +++ b/test/versioned/express/async-handlers.test.js @@ -0,0 +1,86 @@ +/* + * Copyright 2024 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const assert = require('node:assert') +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { makeRequest, setup, isExpress5 } = require('./utils') + +test('async handlers', { skip: !isExpress5() }, async (t) => { + t.beforeEach(async (ctx) => { + await setup(ctx) + }) + + t.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() + }) + + await t.test('should properly track async handlers', async (t) => { + const { app } = t.nr + const mwTimeout = 20 + const handlerTimeout = 25 + + app.use(async function (req, res, next) { + await new Promise((resolve) => { + setTimeout(() => { + resolve() + }, mwTimeout) + }) + next() + }) + app.use('/test', async function handler(req, res) { + await new Promise((resolve) => { + setTimeout(resolve, handlerTimeout) + }) + res.send('ok') + }) + + const tx = await runTest(t, '/test') + const [children] = tx.trace.root.children + const [mw, handler] = children.children + assert.ok( + Math.ceil(mw.getDurationInMillis()) >= mwTimeout, + `should be at least ${mwTimeout} for middleware segment` + ) + assert.ok( + Math.ceil(handler.getDurationInMillis()) >= handlerTimeout, + `should be at least ${handlerTimeout} for handler segment` + ) + }) + + await test('should properly handle errors in async handlers', async (t) => { + const { app } = t.nr + + app.use(() => { + return Promise.reject(new Error('whoops i failed')) + }) + app.use('/test', function handler() { + throw new Error('should not call handler on error') + }) + // eslint-disable-next-line no-unused-vars + app.use(function (error, req, res, next) { + res.status(400).end() + }) + + const tx = await runTest(t, '/test') + const errors = tx.agent.errors.traceAggregator.errors + assert.equal(errors.length, 1) + const [error] = errors + assert.equal(error[2], 'HttpError 400', 'should return 400 from custom error handler') + }) +}) + +async function runTest(t, endpoint) { + const { agent, server } = t.nr + return new Promise((resolve) => { + agent.on('transactionFinished', resolve) + + makeRequest(server, endpoint, function (response) { + response.resume() + }) + }) +} diff --git a/test/versioned/express/bare-router.tap.js b/test/versioned/express/bare-router.tap.js deleted file mode 100644 index ed244e0ebf..0000000000 --- a/test/versioned/express/bare-router.tap.js +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const test = require('tap').test -const helper = require('../../lib/agent_helper') - -test('Express router introspection', function (t) { - t.plan(11) - - const agent = helper.instrumentMockedAgent() - - const express = require('express') - const app = express() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close(() => { - helper.unloadAgent(agent) - }) - }) - - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal(transaction.name, 'WebTransaction/Expressjs/GET//test', 'transaction has expected name') - - t.equal(transaction.url, '/test', 'URL is left alone') - t.equal(transaction.statusCode, 200, 'status code is OK') - t.equal(transaction.verb, 'GET', 'HTTP method is GET') - t.ok(transaction.trace, 'transaction has trace') - - const web = transaction.trace.root.children[0] - t.ok(web, 'trace has web segment') - t.equal(web.name, transaction.name, 'segment name and transaction name match') - - t.equal(web.partialName, 'Expressjs/GET//test', 'should have partial name for apdex') - }) - - app.get('/test', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ status: 'ok' }) - res.end() - }) - - helper.randomPort(function (port) { - server.listen(port, function () { - const url = 'http://localhost:' + port + '/test' - helper.makeGetRequest(url, { json: true }, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, { status: 'ok' }, 'got expected response') - }) - }) - }) -}) diff --git a/test/versioned/express/bare-router.test.js b/test/versioned/express/bare-router.test.js new file mode 100644 index 0000000000..616834a37b --- /dev/null +++ b/test/versioned/express/bare-router.test.js @@ -0,0 +1,61 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const tsplan = require('@matteo.collina/tspl') +const { setup } = require('./utils') + +test.beforeEach(async (ctx) => { + await setup(ctx) +}) + +test.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() +}) + +test('Express router introspection', function (t, end) { + const { agent, app, port } = t.nr + const plan = tsplan(t, { plan: 11 }) + + // need to capture parameters + agent.config.attributes.enabled = true + + agent.on('transactionFinished', function (transaction) { + plan.equal( + transaction.name, + 'WebTransaction/Expressjs/GET//test', + 'transaction has expected name' + ) + + plan.equal(transaction.url, '/test', 'URL is left alone') + plan.equal(transaction.statusCode, 200, 'status code is OK') + plan.equal(transaction.verb, 'GET', 'HTTP method is GET') + plan.ok(transaction.trace, 'transaction has trace') + + const web = transaction.trace.root.children[0] + plan.ok(web, 'trace has web segment') + plan.equal(web.name, transaction.name, 'segment name and transaction name match') + + plan.equal(web.partialName, 'Expressjs/GET//test', 'should have partial name for apdex') + }) + + app.get('/test', function (req, res) { + plan.ok(agent.getTransaction(), 'transaction is available') + + res.send({ status: 'ok' }) + res.end() + }) + + const url = 'http://localhost:' + port + '/test' + helper.makeGetRequest(url, { json: true }, function (error, res, body) { + plan.equal(res.statusCode, 200, 'nothing exploded') + plan.deepEqual(body, { status: 'ok' }, 'got expected response') + end() + }) +}) diff --git a/test/versioned/express/captures-params.tap.js b/test/versioned/express/captures-params.tap.js deleted file mode 100644 index aaa2aa5cd7..0000000000 --- a/test/versioned/express/captures-params.tap.js +++ /dev/null @@ -1,264 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -// shut up, Express -process.env.NODE_ENV = 'test' - -const DESTINATIONS = require('../../../lib/config/attribute-filter').DESTINATIONS -const tap = require('tap') -const helper = require('../../lib/agent_helper') -const HTTP_ATTS = require('../../lib/fixtures').httpAttributes - -// CONSTANTS -const TEST_HOST = 'localhost' -const TEST_URL = 'http://' + TEST_HOST + ':' - -tap.test('test attributes.enabled for express', function (t) { - t.autoend() - - let agent = null - t.beforeEach(function () { - agent = helper.instrumentMockedAgent({ - apdex_t: 1, - allow_all_headers: false, - attributes: { - enabled: true, - include: ['request.parameters.*'] - } - }) - }) - - t.afterEach(function () { - helper.unloadAgent(agent) - }) - - t.test('no variables', function (t) { - const app = require('express')() - const server = require('http').createServer(app) - let port = null - - t.teardown(function () { - server.close() - }) - - app.get('/user/', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ yep: true }) - res.end() - }) - - agent.on('transactionFinished', function (transaction) { - t.ok(transaction.trace, 'transaction has a trace.') - const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) - HTTP_ATTS.forEach(function (key) { - t.ok(attributes[key], 'Trace contains expected HTTP attribute: ' + key) - }) - if (attributes.httpResponseMessage) { - t.equal(attributes.httpResponseMessage, 'OK', 'Trace contains httpResponseMessage') - } - }) - - helper.randomPort(function (_port) { - port = _port - server.listen(port, TEST_HOST, function () { - const url = TEST_URL + port + '/user/' - helper.makeGetRequest(url, function (error, response, body) { - if (error) { - t.fail(error) - } - - t.ok( - /application\/json/.test(response.headers['content-type']), - 'got correct content type' - ) - - t.same(body, { yep: true }, 'Express correctly serves.') - t.end() - }) - }) - }) - }) - - t.test('route variables', function (t) { - const app = require('express')() - const server = require('http').createServer(app) - let port = null - - t.teardown(function () { - server.close() - }) - - app.get('/user/:id', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ yep: true }) - res.end() - }) - - agent.on('transactionFinished', function (transaction) { - t.ok(transaction.trace, 'transaction has a trace.') - const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) - t.equal( - attributes['request.parameters.route.id'], - '5', - 'Trace attributes include `id` route param' - ) - }) - - helper.randomPort(function (_port) { - port = _port - server.listen(port, TEST_HOST, function () { - const url = TEST_URL + port + '/user/5' - helper.makeGetRequest(url, function (error, response, body) { - if (error) { - t.fail(error) - } - - t.ok( - /application\/json/.test(response.headers['content-type']), - 'got correct content type' - ) - - t.same(body, { yep: true }, 'Express correctly serves.') - t.end() - }) - }) - }) - }) - - t.test('query variables', { timeout: 1000 }, function (t) { - const app = require('express')() - const server = require('http').createServer(app) - let port = null - - t.teardown(function () { - server.close() - }) - - app.get('/user/', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ yep: true }) - res.end() - }) - - agent.on('transactionFinished', function (transaction) { - t.ok(transaction.trace, 'transaction has a trace.') - const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) - t.equal( - attributes['request.parameters.name'], - 'bob', - 'Trace attributes include `name` query param' - ) - }) - - helper.randomPort(function (_port) { - port = _port - server.listen(port, TEST_HOST, function () { - const url = TEST_URL + port + '/user/?name=bob' - helper.makeGetRequest(url, function (error, response, body) { - if (error) { - t.fail(error) - } - - t.ok( - /application\/json/.test(response.headers['content-type']), - 'got correct content type' - ) - - t.same(body, { yep: true }, 'Express correctly serves.') - t.end() - }) - }) - }) - }) - - t.test('route and query variables', function (t) { - const app = require('express')() - const server = require('http').createServer(app) - let port = null - - t.teardown(function () { - server.close() - }) - - app.get('/user/:id', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ yep: true }) - res.end() - }) - - agent.on('transactionFinished', function (transaction) { - t.ok(transaction.trace, 'transaction has a trace.') - const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) - t.equal( - attributes['request.parameters.route.id'], - '5', - 'Trace attributes include `id` route param' - ) - t.equal( - attributes['request.parameters.name'], - 'bob', - 'Trace attributes include `name` query param' - ) - }) - - helper.randomPort(function (_port) { - port = _port - server.listen(port, TEST_HOST, function () { - const url = TEST_URL + port + '/user/5?name=bob' - helper.makeGetRequest(url, function (error, response, body) { - if (error) { - t.fail(error) - } - - t.ok( - /application\/json/.test(response.headers['content-type']), - 'got correct content type' - ) - - t.same(body, { yep: true }, 'Express correctly serves.') - t.end() - }) - }) - }) - }) - - t.test('query params should not mask route attributes', function (t) { - const app = require('express')() - const server = require('http').createServer(app) - let port = null - - t.teardown(function () { - server.close() - }) - - app.get('/user/:id', function (req, res) { - res.end() - }) - - agent.on('transactionFinished', function (transaction) { - const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) - t.equal( - attributes['request.parameters.route.id'], - '5', - 'attributes should include route params' - ) - t.equal(attributes['request.parameters.id'], '6', 'attributes should include query params') - t.end() - }) - - helper.randomPort(function (_port) { - port = _port - server.listen(port, TEST_HOST, function () { - helper.makeGetRequest(TEST_URL + port + '/user/5?id=6') - }) - }) - }) -}) diff --git a/test/versioned/express/captures-params.test.js b/test/versioned/express/captures-params.test.js new file mode 100644 index 0000000000..73db905cc9 --- /dev/null +++ b/test/versioned/express/captures-params.test.js @@ -0,0 +1,199 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +// shut up, Express +process.env.NODE_ENV = 'test' +const DESTINATIONS = require('../../../lib/config/attribute-filter').DESTINATIONS +const assert = require('node:assert') +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const HTTP_ATTS = require('../../lib/fixtures').httpAttributes +const { setup } = require('./utils') + +// CONSTANTS +const TEST_HOST = 'localhost' +const TEST_URL = 'http://' + TEST_HOST + ':' + +test('test attributes.enabled for express', async function (t) { + t.beforeEach(async function (ctx) { + await setup(ctx, { + apdex_t: 1, + allow_all_headers: false, + attributes: { + enabled: true, + include: ['request.parameters.*'] + } + }) + }) + + t.afterEach(function (ctx) { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() + }) + + await t.test('no variables', function (t, end) { + const { agent, app, port } = t.nr + app.get('/user/', function (req, res) { + assert.ok(agent.getTransaction(), 'transaction is available') + + res.send({ yep: true }) + res.end() + }) + + agent.on('transactionFinished', function (transaction) { + assert.ok(transaction.trace, 'transaction has a trace.') + const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) + HTTP_ATTS.forEach(function (key) { + assert.ok(attributes[key], 'Trace contains expected HTTP attribute: ' + key) + }) + if (attributes.httpResponseMessage) { + assert.equal(attributes.httpResponseMessage, 'OK', 'Trace contains httpResponseMessage') + } + }) + + const url = TEST_URL + port + '/user/' + helper.makeGetRequest(url, function (error, response, body) { + assert.ok(!error) + assert.ok( + /application\/json/.test(response.headers['content-type']), + 'got correct content type' + ) + + assert.deepEqual(body, { yep: true }, 'Express correctly serves.') + end() + }) + }) + + await t.test('route variables', function (t, end) { + const { agent, app, port } = t.nr + + app.get('/user/:id', function (req, res) { + assert.ok(agent.getTransaction(), 'transaction is available') + + res.send({ yep: true }) + res.end() + }) + + agent.on('transactionFinished', function (transaction) { + assert.ok(transaction.trace, 'transaction has a trace.') + const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) + assert.equal( + attributes['request.parameters.route.id'], + '5', + 'Trace attributes include `id` route param' + ) + }) + + const url = TEST_URL + port + '/user/5' + helper.makeGetRequest(url, function (error, response, body) { + assert.ok(!error) + assert.ok( + /application\/json/.test(response.headers['content-type']), + 'got correct content type' + ) + + assert.deepEqual(body, { yep: true }, 'Express correctly serves.') + end() + }) + }) + + await t.test('query variables', { timeout: 1000 }, function (t, end) { + const { agent, app, port } = t.nr + + app.get('/user/', function (req, res) { + assert.ok(agent.getTransaction(), 'transaction is available') + + res.send({ yep: true }) + res.end() + }) + + agent.on('transactionFinished', function (transaction) { + assert.ok(transaction.trace, 'transaction has a trace.') + const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) + assert.equal( + attributes['request.parameters.name'], + 'bob', + 'Trace attributes include `name` query param' + ) + }) + + const url = TEST_URL + port + '/user/?name=bob' + helper.makeGetRequest(url, function (error, response, body) { + assert.ok(!error) + assert.ok( + /application\/json/.test(response.headers['content-type']), + 'got correct content type' + ) + + assert.deepEqual(body, { yep: true }, 'Express correctly serves.') + end() + }) + }) + + await t.test('route and query variables', function (t, end) { + const { agent, app, port } = t.nr + + app.get('/user/:id', function (req, res) { + assert.ok(agent.getTransaction(), 'transaction is available') + + res.send({ yep: true }) + res.end() + }) + + agent.on('transactionFinished', function (transaction) { + assert.ok(transaction.trace, 'transaction has a trace.') + const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) + assert.equal( + attributes['request.parameters.route.id'], + '5', + 'Trace attributes include `id` route param' + ) + assert.equal( + attributes['request.parameters.name'], + 'bob', + 'Trace attributes include `name` query param' + ) + }) + + const url = TEST_URL + port + '/user/5?name=bob' + helper.makeGetRequest(url, function (error, response, body) { + assert.ok(!error) + assert.ok( + /application\/json/.test(response.headers['content-type']), + 'got correct content type' + ) + + assert.deepEqual(body, { yep: true }, 'Express correctly serves.') + end() + }) + }) + + await t.test('query params should not mask route attributes', function (t, end) { + const { agent, app, port } = t.nr + + app.get('/user/:id', function (req, res) { + res.end() + }) + + agent.on('transactionFinished', function (transaction) { + const attributes = transaction.trace.attributes.get(DESTINATIONS.TRANS_TRACE) + assert.equal( + attributes['request.parameters.route.id'], + '5', + 'attributes should include route params' + ) + assert.equal( + attributes['request.parameters.id'], + '6', + 'attributes should include query params' + ) + end() + }) + + helper.makeGetRequest(TEST_URL + port + '/user/5?id=6') + }) +}) diff --git a/test/versioned/express/client-disconnect.tap.js b/test/versioned/express/client-disconnect.test.js similarity index 75% rename from test/versioned/express/client-disconnect.tap.js rename to test/versioned/express/client-disconnect.test.js index 28ec65c153..8505abe891 100644 --- a/test/versioned/express/client-disconnect.tap.js +++ b/test/versioned/express/client-disconnect.test.js @@ -4,13 +4,13 @@ */ 'use strict' - -const tap = require('tap') +const assert = require('node:assert') +const test = require('node:test') const helper = require('../../lib/agent_helper') -require('../../lib/metrics_helper') const http = require('http') +const { assertSegments } = require('../../lib/custom-assertions') -function generateApp(t) { +function generateApp() { const express = require('express') const bodyParser = require('body-parser') @@ -20,33 +20,30 @@ function generateApp(t) { app.post('/test', function controller(req, res) { const timeout = setTimeout(() => { const err = new Error('should not hit this as request was aborted') - t.error(err) - + assert.ok(!err) res.status(200).send('OK') }, req.body.timeout) res.on('close', () => { - t.comment('cancelling setTimeout') clearTimeout(timeout) }) }) - return app + return app.listen(0) } -tap.test('Client Premature Disconnection', (t) => { - t.setTimeout(3000) +test('Client Premature Disconnection', { timeout: 3000 }, (t, end) => { const agent = helper.instrumentMockedAgent() - const server = generateApp(t).listen(0) + const server = generateApp() const { port } = server.address() - t.teardown(() => { + t.after(() => { server.close() helper.unloadAgent(agent) }) agent.on('transactionFinished', (transaction) => { - t.assertSegments( + assertSegments( transaction.trace.root, [ 'WebTransaction/Expressjs/POST//test', @@ -59,8 +56,8 @@ tap.test('Client Premature Disconnection', (t) => { { exact: false } ) - t.equal(agent.getTransaction(), null, 'should have ended the transaction') - t.end() + assert.equal(agent.getTransaction(), null, 'should have ended the transaction') + end() }) const postData = JSON.stringify({ timeout: 1500 }) @@ -77,12 +74,13 @@ tap.test('Client Premature Disconnection', (t) => { }, function () {} ) - request.on('error', () => t.comment('swallowing request error')) + request.on('error', (err) => { + assert.equal(err.code, 'ECONNRESET') + }) request.write(postData) request.end() setTimeout(() => { - t.comment('aborting request') request.destroy() }, 100) }) diff --git a/test/versioned/express/errors.tap.js b/test/versioned/express/errors.tap.js deleted file mode 100644 index 4d771473b0..0000000000 --- a/test/versioned/express/errors.tap.js +++ /dev/null @@ -1,268 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const http = require('http') -const tap = require('tap') -const { setup, makeRequest } = require('./utils') - -tap.test('reports error when thrown from a route', function (t) { - setup(t) - const { app } = t.context - - app.get('/test', function () { - throw new Error('some error') - }) - - runTest(t, function (errors, statusCode) { - t.equal(errors.length, 1) - t.equal(statusCode, 500) - t.end() - }) -}) - -tap.test('reports error when thrown from a middleware', function (t) { - setup(t) - const { app } = t.context - - app.use(function () { - throw new Error('some error') - }) - - runTest(t, function (errors, statusCode) { - t.equal(errors.length, 1) - t.equal(statusCode, 500) - t.end() - }) -}) - -tap.test('reports error when called in next from a middleware', function (t) { - setup(t) - const { app } = t.context - - app.use(function (req, res, next) { - next(new Error('some error')) - }) - - runTest(t, function (errors, statusCode) { - t.equal(errors.length, 1) - t.equal(statusCode, 500) - t.end() - }) -}) - -tap.test('should not report error when error handler responds', function (t) { - setup(t) - const { app } = t.context - - app.get('/test', function () { - throw new Error('some error') - }) - - // eslint-disable-next-line no-unused-vars - app.use(function (error, req, res, next) { - res.end() - }) - - runTest(t, function (errors, statusCode) { - t.equal(errors.length, 0) - t.equal(statusCode, 200) - t.end() - }) -}) - -tap.test( - 'should report error when error handler responds, but sets error status code', - function (t) { - setup(t) - const { app } = t.context - - app.get('/test', function () { - throw new Error('some error') - }) - - // eslint-disable-next-line no-unused-vars - app.use(function (error, req, res, next) { - res.status(400).end() - }) - - runTest(t, function (errors, statusCode) { - t.equal(errors.length, 1) - t.equal(errors[0][2], 'some error') - t.equal(statusCode, 400) - t.end() - }) - } -) - -tap.test('should report errors passed out of errorware', function (t) { - setup(t) - const { app } = t.context - - app.get('/test', function () { - throw new Error('some error') - }) - - app.use(function (error, req, res, next) { - next(error) - }) - - runTest(t, function (errors, statuscode) { - t.equal(errors.length, 1) - t.equal(statuscode, 500) - t.end() - }) -}) - -tap.test('should report errors from errorware followed by routes', function (t) { - setup(t) - const { app } = t.context - - app.use(function () { - throw new Error('some error') - }) - - app.use(function (error, req, res, next) { - next(error) - }) - - app.get('/test', function (req, res) { - res.end() - }) - - runTest(t, function (errors, statuscode) { - t.equal(errors.length, 1) - t.equal(statuscode, 500) - t.end() - }) -}) - -tap.test('should not report errors swallowed by errorware', function (t) { - setup(t) - const { app } = t.context - - app.get('/test', function () { - throw new Error('some error') - }) - - app.use(function (err, req, res, next) { - next() - }) - - app.get('/test', function (req, res) { - res.end() - }) - - runTest(t, function (errors, statuscode) { - t.equal(errors.length, 0) - t.equal(statuscode, 200) - t.end() - }) -}) - -tap.test('should not report errors handled by errorware outside router', function (t) { - setup(t) - const { app, express } = t.context - - const router1 = express.Router() // eslint-disable-line new-cap - router1.get('/test', function () { - throw new Error('some error') - }) - - app.use(router1) - - // eslint-disable-next-line no-unused-vars - app.use(function (error, req, res, next) { - res.end() - }) - - runTest(t, function (errors, statuscode) { - t.equal(errors.length, 0) - t.equal(statuscode, 200) - t.end() - }) -}) - -tap.test('does not error when request is aborted', function (t) { - t.plan(3) - setup(t) - const { app, agent } = t.context - - let request = null - - app.get('/test', function (req, res, next) { - t.comment('middleware') - t.ok(agent.getTransaction(), 'transaction exists') - - // generate error after client has aborted - request.abort() - setTimeout(function () { - t.comment('timed out') - t.ok(agent.getTransaction() == null, 'transaction has already ended') - next(new Error('some error')) - }, 100) - }) - - // eslint-disable-next-line no-unused-vars - app.use(function (error, req, res, next) { - t.comment('errorware') - t.ok(agent.getTransaction() == null, 'no active transaction when responding') - res.end() - }) - - const server = app.listen(function () { - t.comment('making request') - const port = this.address().port - request = http.request( - { - hostname: 'localhost', - port: port, - path: '/test' - }, - function () {} - ) - request.end() - - // add error handler, otherwise aborting will cause an exception - request.on('error', function (err) { - t.comment('request errored: ' + err) - }) - request.on('abort', function () { - t.comment('request aborted') - }) - }) - - t.teardown(function () { - server.close() - }) -}) - -function runTest(t, callback) { - let statusCode - let errors - const { agent, app } = t.context - - agent.on('transactionFinished', function () { - errors = agent.errors.traceAggregator.errors - if (statusCode) { - callback(errors, statusCode) - } - }) - - const endpoint = '/test' - const server = app.listen(function () { - makeRequest(this, endpoint, function (response) { - statusCode = response.statusCode - if (errors) { - callback(errors, statusCode) - } - response.resume() - }) - }) - t.teardown(function () { - server.close() - }) -} diff --git a/test/versioned/express/errors.test.js b/test/versioned/express/errors.test.js new file mode 100644 index 0000000000..86e9b73f0c --- /dev/null +++ b/test/versioned/express/errors.test.js @@ -0,0 +1,252 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const assert = require('node:assert') +const http = require('http') +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { setup, makeRequest } = require('./utils') +const tsplan = require('@matteo.collina/tspl') + +test('Error handling tests', async (t) => { + t.beforeEach(async (ctx) => { + await setup(ctx) + }) + + t.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() + }) + + await t.test('reports error when thrown from a route', function (t, end) { + const { app } = t.nr + + app.get('/test', function () { + throw new Error('some error') + }) + + runTest(t, function (errors, statusCode) { + assert.equal(errors.length, 1) + assert.equal(statusCode, 500) + end() + }) + }) + + await t.test('reports error when thrown from a middleware', function (t, end) { + const { app } = t.nr + + app.use(function () { + throw new Error('some error') + }) + + runTest(t, function (errors, statusCode) { + assert.equal(errors.length, 1) + assert.equal(statusCode, 500) + end() + }) + }) + + await t.test('reports error when called in next from a middleware', function (t, end) { + const { app } = t.nr + + app.use(function (req, res, next) { + next(new Error('some error')) + }) + + runTest(t, function (errors, statusCode) { + assert.equal(errors.length, 1) + assert.equal(statusCode, 500) + end() + }) + }) + + await t.test('should not report error when error handler responds', function (t, end) { + const { app } = t.nr + + app.get('/test', function () { + throw new Error('some error') + }) + + // eslint-disable-next-line no-unused-vars + app.use(function (error, req, res, next) { + res.end() + }) + + runTest(t, function (errors, statusCode) { + assert.equal(errors.length, 0) + assert.equal(statusCode, 200) + end() + }) + }) + + await t.test( + 'should report error when error handler responds, but sets error status code', + function (t, end) { + const { app } = t.nr + + app.get('/test', function () { + throw new Error('some error') + }) + + // eslint-disable-next-line no-unused-vars + app.use(function (error, req, res, next) { + res.status(400).end() + }) + + runTest(t, function (errors, statusCode) { + assert.equal(errors.length, 1) + assert.equal(errors[0][2], 'some error') + assert.equal(statusCode, 400) + end() + }) + } + ) + + await t.test('should report errors passed out of errorware', function (t, end) { + const { app } = t.nr + + app.get('/test', function () { + throw new Error('some error') + }) + + app.use(function (error, req, res, next) { + next(error) + }) + + runTest(t, function (errors, statuscode) { + assert.equal(errors.length, 1) + assert.equal(statuscode, 500) + end() + }) + }) + + await t.test('should report errors from errorware followed by routes', function (t, end) { + const { app } = t.nr + + app.use(function () { + throw new Error('some error') + }) + + app.use(function (error, req, res, next) { + next(error) + }) + + app.get('/test', function (req, res) { + res.end() + }) + + runTest(t, function (errors, statuscode) { + assert.equal(errors.length, 1) + assert.equal(statuscode, 500) + end() + }) + }) + + await t.test('should not report errors swallowed by errorware', function (t, end) { + const { app } = t.nr + + app.get('/test', function () { + throw new Error('some error') + }) + + app.use(function (err, req, res, next) { + next() + }) + + app.get('/test', function (req, res) { + res.end() + }) + + runTest(t, function (errors, statuscode) { + assert.equal(errors.length, 0) + assert.equal(statuscode, 200) + end() + }) + }) + + await t.test('should not report errors handled by errorware outside router', function (t, end) { + const { app, express } = t.nr + + const router1 = express.Router() // eslint-disable-line new-cap + router1.get('/test', function () { + throw new Error('some error') + }) + + app.use(router1) + + // eslint-disable-next-line no-unused-vars + app.use(function (error, req, res, next) { + res.end() + }) + + runTest(t, function (errors, statuscode) { + assert.equal(errors.length, 0) + assert.equal(statuscode, 200) + end() + }) + }) + + await t.test('does not error when request is aborted', function (t, end) { + const plan = tsplan(t, { plan: 4 }) + const { app, agent, port } = t.nr + let request = null + + app.get('/test', function (req, res, next) { + plan.ok(agent.getTransaction(), 'transaction exists') + + // generate error after client has aborted + request.abort() + setTimeout(function () { + plan.equal(agent.getTransaction(), null, 'transaction has already ended') + next(new Error('some error')) + }, 100) + }) + + // eslint-disable-next-line no-unused-vars + app.use(function (error, req, res, next) { + plan.equal(agent.getTransaction(), null, 'no active transaction when responding') + res.end() + end() + }) + + request = http.request( + { + hostname: 'localhost', + port, + path: '/test' + }, + function () {} + ) + request.end() + + // add error handler, otherwise aborting will cause an exception + request.on('error', function (err) { + plan.equal(err.code, 'ECONNRESET') + }) + }) +}) + +function runTest(t, callback) { + let statusCode + let errors + const { agent, server } = t.nr + + agent.on('transactionFinished', function () { + errors = agent.errors.traceAggregator.errors + if (statusCode) { + callback(errors, statusCode) + } + }) + + const endpoint = '/test' + makeRequest(server, endpoint, function (response) { + statusCode = response.statusCode + if (errors) { + callback(errors, statusCode) + } + response.resume() + }) +} diff --git a/test/versioned/express/express-enrouten.tap.js b/test/versioned/express/express-enrouten.tap.js deleted file mode 100644 index 7b2643263d..0000000000 --- a/test/versioned/express/express-enrouten.tap.js +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * This test checks for regressions on the route stack manipulation for Express apps. - */ -'use strict' - -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) { - t.plan(2) - - const agent = helper.instrumentMockedAgent() - const express = require('express') - const enrouten = require('express-enrouten') - const app = express() - const server = require('http').createServer(app) - - app.use(enrouten({ directory: './fixtures' })) - - t.teardown(() => { - server.close(() => { - helper.unloadAgent(agent) - }) - }) - - // New Relic + express-enrouten used to have a bug, where any routes after the - // first one would be lost. - server.listen(0, function () { - const port = server.address().port - helper.makeGetRequest('http://localhost:' + port + '/', function (error, res) { - t.equal(res.statusCode, 200, 'First Route loaded') - }) - - helper.makeGetRequest('http://localhost:' + port + '/foo', function (error, res) { - t.equal(res.statusCode, 200, 'Second Route loaded') - }) - }) -}) diff --git a/test/versioned/express/express-enrouten.test.js b/test/versioned/express/express-enrouten.test.js new file mode 100644 index 0000000000..ee0f52f124 --- /dev/null +++ b/test/versioned/express/express-enrouten.test.js @@ -0,0 +1,43 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * This test checks for regressions on the route stack manipulation for Express apps. + */ +'use strict' + +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { isExpress5 } = require('./utils') +const tsplan = require('@matteo.collina/tspl') +const { setup } = require('./utils') + +test.beforeEach(async (ctx) => { + await setup(ctx) +}) + +test.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() +}) + +test('Express + express-enrouten compatibility test', { skip: isExpress5() }, function (t, end) { + const { app, port } = t.nr + const plan = tsplan(t, { plan: 2 }) + + const enrouten = require('express-enrouten') + app.use(enrouten({ directory: './fixtures' })) + + // New Relic + express-enrouten used to have a bug, where any routes after the + // first one would be lost. + helper.makeGetRequest('http://localhost:' + port + '/', function (error, res) { + plan.equal(res.statusCode, 200, 'First Route loaded') + }) + + helper.makeGetRequest('http://localhost:' + port + '/foo', function (error, res) { + plan.equal(res.statusCode, 200, 'Second Route loaded') + end() + }) +}) diff --git a/test/versioned/express/ignoring.tap.js b/test/versioned/express/ignoring.test.js similarity index 52% rename from test/versioned/express/ignoring.tap.js rename to test/versioned/express/ignoring.test.js index bb7fdc620e..64b77c044e 100644 --- a/test/versioned/express/ignoring.tap.js +++ b/test/versioned/express/ignoring.test.js @@ -5,48 +5,49 @@ 'use strict' -const test = require('tap').test +const test = require('node:test') const helper = require('../../lib/agent_helper') const API = require('../../../api') +const tsplan = require('@matteo.collina/tspl') +const { setup } = require('./utils') -test('ignoring an Express route', function (t) { - t.plan(7) +test.beforeEach(async (ctx) => { + await setup(ctx) +}) - const agent = helper.instrumentMockedAgent() +test.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() +}) - const api = new API(agent) - const express = require('express') - const app = express() - const server = require('http').createServer(app) +test('ignoring an Express route', function (t, end) { + const { agent, app, port } = t.nr + const plan = tsplan(t, { plan: 7 }) - t.teardown(() => { - server.close(() => { - helper.unloadAgent(agent) - }) - }) + const api = new API(agent) agent.on('transactionFinished', function (transaction) { - t.equal( + plan.equal( transaction.name, 'WebTransaction/Expressjs/GET//polling/:id', 'transaction has expected name even on error' ) - t.ok(transaction.ignore, 'transaction is ignored') + plan.ok(transaction.ignore, 'transaction is ignored') - t.notOk(agent.traces.trace, 'should have no transaction trace') + plan.ok(!agent.traces.trace, 'should have no transaction trace') const metrics = agent.metrics._metrics.unscoped // loading k2 adds instrumentation metrics for things it loads const expectedMetrics = helper.isSecurityAgentEnabled(agent) ? 11 : 3 - t.equal( + plan.equal( Object.keys(metrics).length, expectedMetrics, 'only supportability metrics added to agent collection' ) const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 0, 'no errors noticed') + plan.equal(errors.length, 0, 'no errors noticed') }) app.get('/polling/:id', function (req, res) { @@ -55,12 +56,10 @@ test('ignoring an Express route', function (t) { res.end() }) - server.listen(0, function () { - const port = server.address().port - const url = 'http://localhost:' + port + '/polling/31337' - helper.makeGetRequest(url, function (error, res, body) { - t.equal(res.statusCode, 400, 'got expected error') - t.same(body, { status: 'pollpollpoll' }, 'got expected response') - }) + const url = 'http://localhost:' + port + '/polling/31337' + helper.makeGetRequest(url, function (error, res, body) { + plan.equal(res.statusCode, 400, 'got expected error') + plan.deepEqual(body, { status: 'pollpollpoll' }, 'got expected response') + end() }) }) diff --git a/test/versioned/express/issue171.tap.js b/test/versioned/express/issue171.tap.js deleted file mode 100644 index b4683fc818..0000000000 --- a/test/versioned/express/issue171.tap.js +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const helper = require('../../lib/agent_helper') - -helper.instrumentMockedAgent() - -const test = require('tap').test -const http = require('http') -const app = require('express')() - -test("adding 'handle' middleware", function (t) { - t.plan(2) - - // eslint-disable-next-line no-unused-vars - function handle(err, req, res, next) { - t.ok(err, 'error should exist') - - res.statusCode = 500 - res.end() - } - - app.use('/', function () { - throw new Error() - }) - - app.use(handle) - - app.listen(function () { - const server = this - const port = server.address().port - - http - .request({ port: port }, function (res) { - // drain response to let process exit - res.pipe(process.stderr) - - t.equal(res.statusCode, 500) - server.close() - }) - .end() - }) -}) diff --git a/test/versioned/express/issue171.test.js b/test/versioned/express/issue171.test.js new file mode 100644 index 0000000000..ce09fcebcf --- /dev/null +++ b/test/versioned/express/issue171.test.js @@ -0,0 +1,50 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const helper = require('../../lib/agent_helper') +const test = require('node:test') +const tsplan = require('@matteo.collina/tspl') +const { setup } = require('./utils') +const http = require('http') + +test.beforeEach(async (ctx) => { + await setup(ctx) +}) + +test.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() +}) + +test("adding 'handle' middleware", function (t, end) { + const { app, port } = t.nr + const plan = tsplan(t, { plan: 2 }) + + // eslint-disable-next-line no-unused-vars + function handle(err, req, res, next) { + plan.ok(err, 'error should exist') + + res.statusCode = 500 + res.end() + } + + app.use('/', function () { + throw new Error() + }) + + app.use(handle) + + http + .request({ port: port }, function (res) { + // drain response to let process exit + res.pipe(process.stderr) + + plan.equal(res.statusCode, 500) + end() + }) + .end() +}) diff --git a/test/versioned/express/middleware-name.tap.js b/test/versioned/express/middleware-name.tap.js deleted file mode 100644 index f53adf5973..0000000000 --- a/test/versioned/express/middleware-name.tap.js +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const test = require('tap').test -const helper = require('../../lib/agent_helper') - -test('should name middleware correctly', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - - app.use('/', testMiddleware) - - const server = app.listen(0, function () { - const router = app._router || app.router - const mwLayer = router.stack.filter((layer) => layer.name === 'testMiddleware') - t.equal(mwLayer.length, 1, 'should only find one testMiddleware function') - t.end() - }) - - t.teardown(function () { - server.close() - helper.unloadAgent(agent) - }) - - function testMiddleware(req, res, next) { - next() - } -}) diff --git a/test/versioned/express/middleware-name.test.js b/test/versioned/express/middleware-name.test.js new file mode 100644 index 0000000000..a0d848a311 --- /dev/null +++ b/test/versioned/express/middleware-name.test.js @@ -0,0 +1,31 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const assert = require('node:assert') +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { setup } = require('./utils') + +test.beforeEach(async (ctx) => { + await setup(ctx) +}) + +test.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() +}) + +test('should name middleware correctly', function (t) { + const { app } = t.nr + app.use('/', testMiddleware) + + const router = app._router || app.router + const mwLayer = router.stack.filter((layer) => layer.name === 'testMiddleware') + assert.equal(mwLayer.length, 1, 'should only find one testMiddleware function') + function testMiddleware(req, res, next) { + next() + } +}) diff --git a/test/versioned/express/package.json b/test/versioned/express/package.json index 694ed62abe..82d63966fc 100644 --- a/test/versioned/express/package.json +++ b/test/versioned/express/package.json @@ -22,24 +22,24 @@ "ejs": "2.5.9" }, "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", - "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" + "app-use.test.js", + "async-error.test.js", + "async-handlers.test.js", + "bare-router.test.js", + "captures-params.test.js", + "client-disconnect.test.js", + "errors.test.js", + "express-enrouten.test.js", + "ignoring.test.js", + "issue171.test.js", + "middleware-name.test.js", + "render.test.js", + "require.test.js", + "route-iteration.test.js", + "route-param.test.js", + "router-params.test.js", + "segments.test.js", + "transaction-naming.test.js" ] } ] diff --git a/test/versioned/express/render.tap.js b/test/versioned/express/render.tap.js deleted file mode 100644 index 610804c785..0000000000 --- a/test/versioned/express/render.tap.js +++ /dev/null @@ -1,661 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -// shut up, Express -process.env.NODE_ENV = 'test' - -const test = require('tap').test -const helper = require('../../lib/agent_helper') -const API = require('../../../api') -const symbols = require('../../../lib/symbols') - -const TEST_PATH = '/test' -const TEST_HOST = 'localhost' -const TEST_URL = 'http://' + TEST_HOST + ':' -const DELAY = 600 -const BODY = - '\n' + - '\n' + - '\n' + - ' yo dawg\n' + - '\n' + - '\n' + - '

I heard u like HTML.

\n' + - '\n' + - '\n' - -// Regression test for issue 154 -// https://github.com/newrelic/node-newrelic/pull/154 -test('using only the express router', function (t) { - const agent = helper.instrumentMockedAgent() - const router = require('express').Router() // eslint-disable-line new-cap - t.teardown(() => { - helper.unloadAgent(agent) - }) - - router.get('/test', function () { - // - }) - - router.get('/test2', function () { - // - }) - - // just try not to blow up - t.end() -}) - -test('the express router should go through a whole request lifecycle', function (t) { - const agent = helper.instrumentMockedAgent() - const router = require('express').Router() // eslint-disable-line new-cap - const finalhandler = require('finalhandler') - - t.plan(2) - - t.teardown(() => { - helper.unloadAgent(agent) - }) - - router.get('/test', function (_, res) { - t.ok(true) - res.end() - }) - - const server = require('http').createServer(function onRequest(req, res) { - router(req, res, finalhandler(req, res)) - }) - server.listen(0, function () { - const port = server.address().port - helper.makeRequest('http://localhost:' + port + '/test', function (error) { - server.close() - - t.error(error) - t.end() - }) - }) -}) - -test('agent instrumentation of Express', function (t) { - t.plan(7) - - let agent = null - let app = null - let server = null - - t.beforeEach(function () { - agent = helper.instrumentMockedAgent() - - app = require('express')() - server = require('http').createServer(app) - }) - - t.afterEach(function () { - server.close() - helper.unloadAgent(agent) - - agent = null - app = null - server = null - }) - - t.test('for a normal request', { timeout: 1000 }, function (t) { - // set apdexT so apdex stats will be recorded - agent.config.apdex_t = 1 - - app.get(TEST_PATH, function (req, res) { - res.send({ yep: true }) - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (error, response, body) { - t.error(error, 'should not fail making request') - - t.ok(/application\/json/.test(response.headers['content-type']), 'got correct content type') - - t.same(body, { yep: true }, 'Express correctly serves.') - - let stats - - stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') - t.ok(stats, 'found unscoped stats for request path') - t.equal(stats.callCount, 1, '/test was only requested once') - - stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') - t.ok(stats, 'found apdex stats for request path') - t.equal(stats.satisfying, 1, 'got satisfactory response time') - t.equal(stats.tolerating, 0, 'got no tolerable requests') - t.equal(stats.frustrating, 0, 'got no frustrating requests') - - stats = agent.metrics.getMetric('WebTransaction') - t.ok(stats, 'found roll-up statistics for web requests') - t.equal(stats.callCount, 1, 'only one web request was made') - - stats = agent.metrics.getMetric('HttpDispatcher') - t.ok(stats, 'found HTTP dispatcher statistics') - t.equal(stats.callCount, 1, 'only one HTTP-dispatched request was made') - - const serialized = JSON.stringify(agent.metrics._toPayloadSync()) - t.ok( - serialized.match(/WebTransaction\/Expressjs\/GET\/\/test/), - 'serialized metrics as expected' - ) - - t.end() - }) - }) - }) - - t.test('ignore apdex when ignoreApdex is true on transaction', { timeout: 1000 }, function (t) { - // set apdexT so apdex stats will be recorded - agent.config.apdex_t = 1 - - app.get(TEST_PATH, function (req, res) { - const tx = agent.getTransaction() - tx.ignoreApdex = true - res.send({ yep: true }) - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - let stats - - stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') - t.ok(stats, 'found unscoped stats for request path') - t.equal(stats.callCount, 1, '/test was only requested once') - - stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') - t.notOk(stats, 'should not have apdex metrics') - - stats = agent.metrics.getMetric('WebTransaction') - t.ok(stats, 'found roll-up statistics for web requests') - t.equal(stats.callCount, 1, 'only one web request was made') - - stats = agent.metrics.getMetric('HttpDispatcher') - t.ok(stats, 'found HTTP dispatcher statistics') - t.equal(stats.callCount, 1, 'only one HTTP-dispatched request was made') - t.end() - }) - }) - }) - - t.test('using EJS templates', { timeout: 1000 }, function (t) { - app.set('views', __dirname + '/views') - app.set('view engine', 'ejs') - - app.get(TEST_PATH, function (req, res) { - res.render('index', { title: 'yo dawg' }) - }) - - agent.once('transactionFinished', function () { - const stats = agent.metrics.getMetric('View/index/Rendering') - t.equal(stats.callCount, 1, 'should note the view rendering') - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (error, response, body) { - t.error(error, 'should not error making request') - - t.equal(response.statusCode, 200, 'response code should be 200') - t.equal(body, BODY, 'template should still render fine') - - t.end() - }) - }) - }) - - t.test('should generate rum headers', { timeout: 1000 }, function (t) { - const api = new API(agent) - - agent.config.application_id = '12345' - agent.config.browser_monitoring.browser_key = '12345' - agent.config.browser_monitoring.js_agent_loader = 'function() {}' - - app.set('views', __dirname + '/views') - app.set('view engine', 'ejs') - - app.get(TEST_PATH, function (req, res) { - const rum = api.getBrowserTimingHeader() - t.equal(rum.substring(0, 7), ' -1 - t.ok(isFramework, 'should indicate that express is a framework') - - t.notOk(agent.getTransaction(), "transaction shouldn't be visible from request") - t.equal(body, BODY, 'response and original page text match') - - const stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') - t.ok(stats, 'Statistics should have been found for request.') - - const timing = stats.total * 1000 - t.ok(timing > DELAY - 50, 'should have expected timing (within reason)') - - t.end() - }) - }) - }) - - t.test('should capture URL correctly with a prefix', { timeout: 2000 }, function (t) { - app.use(TEST_PATH, function (req, res) { - t.ok(agent.getTransaction(), 'should maintain transaction state in middleware') - t.equal(req.url, '/ham', 'should have correct test url') - res.send(BODY) - }) - - server.listen(0, TEST_HOST, function ready() { - const port = server.address().port - const url = TEST_URL + port + TEST_PATH + '/ham' - helper.makeGetRequest(url, function (error, response, body) { - t.error(error, 'should not fail making request') - - t.notOk(agent.getTransaction(), "transaction shouldn't be visible from request") - t.equal(body, BODY, 'response and original page text match') - - const stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') - t.ok(stats, 'Statistics should have been found for request.') - - t.end() - }) - }) - }) -}) - -test('trapping errors', function (t) { - t.autoend() - - t.test('collects the actual error object that is thrown', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - app.get(TEST_PATH, function () { - throw new Error('some error') - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 1, 'there should be one error') - t.equal(errors[0][2], 'some error', 'got the expected error') - t.ok(errors[0][4].stack_trace, 'has stack trace') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 1, 'apdex should be frustrating') - - t.end() - }) - }) - }) - - t.test('does not occur with custom defined error handlers', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - const error = new Error('some error') - - app.get(TEST_PATH, function () { - throw error - }) - - app.use(function (err, req, res, next) { - t.equal(err, error, 'should see the same error in the error handler') - next() - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 0, 'there should be no errors') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 0, 'apdex should not be frustrating') - - t.end() - }) - }) - }) - - t.test('does not occur with custom defined error handlers', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - const error = new Error('some error') - - app.get(TEST_PATH, function (req, res, next) { - next(error) - }) - - app.use(function (err, req, res, next) { - t.equal(err, error, 'should see the same error in the error handler') - next() - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 0, 'there should be no errors') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 0, 'apdex should not be frustrating') - - t.end() - }) - }) - }) - - t.test('collects the error message when string is thrown', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - app.get(TEST_PATH, function () { - throw 'some error' - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 1, 'there should be one error') - t.equal(errors[0][2], 'some error', 'got the expected error') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 1, 'apdex should be frustrating') - - t.end() - }) - }) - }) - - t.test('collects the actual error object when error handler is used', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - app.get(TEST_PATH, function () { - throw new Error('some error') - }) - - // eslint-disable-next-line no-unused-vars - app.use(function (err, rer, res, next) { - res.status(400).end() - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 1, 'there should be one error') - t.equal(errors[0][2], 'some error', 'got the expected error') - t.ok(errors[0][4].stack_trace, 'has stack trace') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 1, 'apdex should be frustrating') - - t.end() - }) - }) - }) - - // Some error handlers might sanitize the error object, removing stack and/or message - // properties, so that it can be serialized and sent back in the response body. - // We use message and stack properties to identify an Error object, so in this case - // we want to at least collect the HTTP error based on the status code. - t.test('should report errors without message or stack sent to res.send', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - const error = new Error('some error') - app.get(TEST_PATH, function () { - throw error - }) - - // eslint-disable-next-line no-unused-vars - app.use(function (err, rer, res, next) { - delete err.message - delete err.stack - res.status(400).send(err) - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 1, 'there should be one error') - t.equal(errors[0][2], 'HttpError 400', 'got the expected error') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 1, 'apdex should be frustrating') - - t.end() - }) - }) - }) - - t.test('should report errors without message or stack sent to next', function (t) { - const agent = helper.instrumentMockedAgent() - - const app = require('express')() - const server = require('http').createServer(app) - - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - const error = new Error('some error') - app.get(TEST_PATH, function () { - throw error - }) - - app.use(function errorHandler(err, rer, res, next) { - delete err.message - delete err.stack - next(err) - }) - - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { - const errors = agent.errors.traceAggregator.errors - t.equal(errors.length, 1, 'there should be one error') - t.equal(errors[0][2], 'HttpError 500', 'got the expected error') - - const metric = agent.metrics.getMetric('Apdex') - t.ok(metric.frustrating === 1, 'apdex should be frustrating') - - t.end() - }) - }) - }) -}) - -test('layer wrapping', function (t) { - t.plan(1) - - // Set up the test. - const agent = helper.instrumentMockedAgent() - const app = require('express')() - const server = require('http').createServer(app) - t.teardown(() => { - server.close() - helper.unloadAgent(agent) - }) - - // Add our route. - app.get(TEST_PATH, function (req, res) { - res.send('bar') - }) - - // Proxy the last layer on the stack. - const router = app._router || app.router - const stack = router.stack - stack[stack.length - 1] = makeProxyLayer(stack[stack.length - 1]) - - // Make our request. - server.listen(0, TEST_HOST, function () { - const port = server.address().port - helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (err, response, body) { - t.equal(body, 'bar', 'should not fail with a proxy layer') - t.end() - }) - }) -}) - -/** - * Wraps a layer in a proxy with all of the layer's prototype's methods directly - * on itself. - * - * @param {express.Layer} layer - The layer to proxy. - * - * @return {object} A POD object with all the fields of the layer copied over. - */ -function makeProxyLayer(layer) { - const fakeLayer = { - handle_request: function () { - layer.handle_request.apply(layer, arguments) - }, - handle_error: function () { - layer.handle_error.apply(layer, arguments) - } - } - Object.keys(layer).forEach(function (k) { - if (!fakeLayer[k]) { - fakeLayer[k] = layer[k] - } - }) - Object.keys(layer.constructor.prototype).forEach(function (k) { - if (!fakeLayer[k]) { - fakeLayer[k] = layer[k] - } - }) - return fakeLayer -} diff --git a/test/versioned/express/render.test.js b/test/versioned/express/render.test.js new file mode 100644 index 0000000000..3d9001973b --- /dev/null +++ b/test/versioned/express/render.test.js @@ -0,0 +1,548 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +// shut up, Express +process.env.NODE_ENV = 'test' +const assert = require('node:assert') +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const API = require('../../../api') +const symbols = require('../../../lib/symbols') +const { setup } = require('./utils') +const tsplan = require('@matteo.collina/tspl') + +const TEST_PATH = '/test' +const TEST_HOST = 'localhost' +const TEST_URL = 'http://' + TEST_HOST + ':' +const DELAY = 600 +const BODY = + '\n' + + '\n' + + '\n' + + ' yo dawg\n' + + '\n' + + '\n' + + '

I heard u like HTML.

\n' + + '\n' + + '\n' + +// Regression test for issue 154 +// https://github.com/newrelic/node-newrelic/pull/154 +test('using only the express router', function (t, end) { + const agent = helper.instrumentMockedAgent() + const router = require('express').Router() // eslint-disable-line new-cap + t.after(() => { + helper.unloadAgent(agent) + }) + + assert.doesNotThrow(() => { + router.get('/test', function () {}) + router.get('/test2', function () {}) + }) + + end() +}) + +test('the express router should go through a whole request lifecycle', function (t, end) { + const agent = helper.instrumentMockedAgent() + const router = require('express').Router() // eslint-disable-line new-cap + const finalhandler = require('finalhandler') + + const plan = tsplan(t, { plan: 2 }) + + t.after(() => { + helper.unloadAgent(agent) + }) + + router.get('/test', function (_, res) { + plan.ok(true) + res.end() + }) + + const server = require('http').createServer(function onRequest(req, res) { + router(req, res, finalhandler(req, res)) + }) + server.listen(0, function () { + const port = server.address().port + helper.makeRequest('http://localhost:' + port + '/test', function (error) { + server.close() + + plan.ok(!error) + end() + }) + }) +}) + +test('agent instrumentation of Express', async function (t) { + t.beforeEach(async function (ctx) { + await setup(ctx) + }) + + t.afterEach(function (ctx) { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() + }) + + await t.test('for a normal request', { timeout: 1000 }, function (t, end) { + const { app, agent, port } = t.nr + // set apdexT so apdex stats will be recorded + agent.config.apdex_t = 1 + + app.get(TEST_PATH, function (req, res) { + res.send({ yep: true }) + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (error, response, body) { + assert.ok(!error, 'should not fail making request') + + assert.ok( + /application\/json/.test(response.headers['content-type']), + 'got correct content type' + ) + + assert.deepEqual(body, { yep: true }, 'Express correctly serves.') + + let stats + + stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') + assert.ok(stats, 'found unscoped stats for request path') + assert.equal(stats.callCount, 1, '/test was only requested once') + + stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') + assert.ok(stats, 'found apdex stats for request path') + assert.equal(stats.satisfying, 1, 'got satisfactory response time') + assert.equal(stats.tolerating, 0, 'got no tolerable requests') + assert.equal(stats.frustrating, 0, 'got no frustrating requests') + + stats = agent.metrics.getMetric('WebTransaction') + assert.ok(stats, 'found roll-up statistics for web requests') + assert.equal(stats.callCount, 1, 'only one web request was made') + + stats = agent.metrics.getMetric('HttpDispatcher') + assert.ok(stats, 'found HTTP dispatcher statistics') + assert.equal(stats.callCount, 1, 'only one HTTP-dispatched request was made') + + const serialized = JSON.stringify(agent.metrics._toPayloadSync()) + assert.ok( + serialized.match(/WebTransaction\/Expressjs\/GET\/\/test/), + 'serialized metrics as expected' + ) + + end() + }) + }) + + await t.test( + 'ignore apdex when ignoreApdex is true on transaction', + { timeout: 1000 }, + function (t, end) { + const { app, agent, port } = t.nr + // set apdexT so apdex stats will be recorded + agent.config.apdex_t = 1 + + app.get(TEST_PATH, function (req, res) { + const tx = agent.getTransaction() + tx.ignoreApdex = true + res.send({ yep: true }) + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + let stats + + stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') + assert.ok(stats, 'found unscoped stats for request path') + assert.equal(stats.callCount, 1, '/test was only requested once') + + stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') + assert.ok(!stats, 'should not have apdex metrics') + + stats = agent.metrics.getMetric('WebTransaction') + assert.ok(stats, 'found roll-up statistics for web requests') + assert.equal(stats.callCount, 1, 'only one web request was made') + + stats = agent.metrics.getMetric('HttpDispatcher') + assert.ok(stats, 'found HTTP dispatcher statistics') + assert.equal(stats.callCount, 1, 'only one HTTP-dispatched request was made') + end() + }) + } + ) + + await t.test('using EJS templates', { timeout: 1000 }, function (t, end) { + const { app, agent, port } = t.nr + app.set('views', __dirname + '/views') + app.set('view engine', 'ejs') + + app.get(TEST_PATH, function (req, res) { + res.render('index', { title: 'yo dawg' }) + }) + + agent.once('transactionFinished', function () { + const stats = agent.metrics.getMetric('View/index/Rendering') + assert.equal(stats.callCount, 1, 'should note the view rendering') + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (error, response, body) { + assert.ok(!error, 'should not error making request') + + assert.equal(response.statusCode, 200, 'response code should be 200') + assert.equal(body, BODY, 'template should still render fine') + + end() + }) + }) + + await t.test('should generate rum headers', { timeout: 1000 }, function (t, end) { + const { app, agent, port } = t.nr + const api = new API(agent) + + agent.config.license_key = 'license_key' + agent.config.application_id = '12345' + agent.config.browser_monitoring.browser_key = '12345' + agent.config.browser_monitoring.js_agent_loader = 'function() {}' + + app.set('views', __dirname + '/views') + app.set('view engine', 'ejs') + + app.get(TEST_PATH, function (req, res) { + const rum = api.getBrowserTimingHeader() + assert.equal(rum.substring(0, 7), ' -1 + assert.ok(isFramework, 'should indicate that express is a framework') + + assert.ok(!agent.getTransaction(), "transaction shouldn't be visible from request") + assert.equal(body, BODY, 'response and original page text match') + + const stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') + assert.ok(stats, 'Statistics should have been found for request.') + + const timing = stats.total * 1000 + assert.ok(timing > DELAY - 50, 'should have expected timing (within reason)') + + end() + }) + }) + + await t.test('should capture URL correctly with a prefix', { timeout: 2000 }, function (t, end) { + const { app, agent, port } = t.nr + app.use(TEST_PATH, function (req, res) { + assert.ok(agent.getTransaction(), 'should maintain transaction state in middleware') + assert.equal(req.url, '/ham', 'should have correct test url') + res.send(BODY) + }) + + const url = TEST_URL + port + TEST_PATH + '/ham' + helper.makeGetRequest(url, function (error, response, body) { + assert.ok(!error, 'should not fail making request') + + assert.ok(!agent.getTransaction(), "transaction shouldn't be visible from request") + assert.equal(body, BODY, 'response and original page text match') + + const stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') + assert.ok(stats, 'Statistics should have been found for request.') + + end() + }) + }) + + await t.test('collects the actual error object that is thrown', function (t, end) { + const { agent, app, port } = t.nr + app.get(TEST_PATH, function () { + throw new Error('some error') + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 1, 'there should be one error') + assert.equal(errors[0][2], 'some error', 'got the expected error') + assert.ok(errors[0][4].stack_trace, 'has stack trace') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 1, 'apdex should be frustrating') + + end() + }) + }) + + await t.test('does not occur with custom defined error handlers', function (t, end) { + const { agent, app, port } = t.nr + const error = new Error('some error') + + app.get(TEST_PATH, function () { + throw error + }) + + app.use(function (err, req, res, next) { + assert.equal(err, error, 'should see the same error in the error handler') + next() + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 0, 'there should be no errors') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 0, 'apdex should not be frustrating') + + end() + }) + }) + + await t.test('does not occur with custom defined error handlers', function (t, end) { + const { agent, app, port } = t.nr + const error = new Error('some error') + + app.get(TEST_PATH, function (req, res, next) { + next(error) + }) + + app.use(function (err, req, res, next) { + assert.equal(err, error, 'should see the same error in the error handler') + next() + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 0, 'there should be no errors') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 0, 'apdex should not be frustrating') + + end() + }) + }) + + await t.test('collects the error message when string is thrown', function (t, end) { + const { agent, app, port } = t.nr + + app.get(TEST_PATH, function () { + throw 'some error' + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 1, 'there should be one error') + assert.equal(errors[0][2], 'some error', 'got the expected error') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 1, 'apdex should be frustrating') + + end() + }) + }) + + await t.test('collects the actual error object when error handler is used', function (t, end) { + const { agent, app, port } = t.nr + app.get(TEST_PATH, function () { + throw new Error('some error') + }) + + // eslint-disable-next-line no-unused-vars + app.use(function (err, rer, res, next) { + res.status(400).end() + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 1, 'there should be one error') + assert.equal(errors[0][2], 'some error', 'got the expected error') + assert.ok(errors[0][4].stack_trace, 'has stack trace') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 1, 'apdex should be frustrating') + + end() + }) + }) + + // Some error handlers might sanitize the error object, removing stack and/or message + // properties, so that it can be serialized and sent back in the response body. + // We use message and stack properties to identify an Error object, so in this case + // we want to at least collect the HTTP error based on the status code. + await t.test('should report errors without message or stack sent to res.send', function (t, end) { + const { agent, app, port } = t.nr + const error = new Error('some error') + app.get(TEST_PATH, function () { + throw error + }) + + // eslint-disable-next-line no-unused-vars + app.use(function (err, rer, res, next) { + delete err.message + delete err.stack + res.status(400).send(err) + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 1, 'there should be one error') + assert.equal(errors[0][2], 'HttpError 400', 'got the expected error') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 1, 'apdex should be frustrating') + + end() + }) + }) + + await t.test('should report errors without message or stack sent to next', function (t, end) { + const { agent, app, port } = t.nr + + const error = new Error('some error') + app.get(TEST_PATH, function () { + throw error + }) + + app.use(function errorHandler(err, rer, res, next) { + delete err.message + delete err.stack + next(err) + }) + + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + const errors = agent.errors.traceAggregator.errors + assert.equal(errors.length, 1, 'there should be one error') + assert.equal(errors[0][2], 'HttpError 500', 'got the expected error') + + const metric = agent.metrics.getMetric('Apdex') + assert.ok(metric.frustrating === 1, 'apdex should be frustrating') + + end() + }) + }) + + await t.test('layer wrapping', function (t, end) { + const { app, port } = t.nr + const plan = tsplan(t, { plan: 1 }) + // Add our route. + app.get(TEST_PATH, function (req, res) { + res.send('bar') + }) + + // Proxy the last layer on the stack. + const router = app._router || app.router + const stack = router.stack + stack[stack.length - 1] = makeProxyLayer(stack[stack.length - 1]) + + // Make our request. + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function (err, response, body) { + plan.equal(body, 'bar', 'should not fail with a proxy layer') + end() + }) + }) +}) + +/** + * Wraps a layer in a proxy with all of the layer's prototype's methods directly + * on itself. + * + * @param {express.Layer} layer - The layer to proxy. + * + * @return {object} A POD object with all the fields of the layer copied over. + */ +function makeProxyLayer(layer) { + const fakeLayer = { + handle_request: function () { + layer.handle_request.apply(layer, arguments) + }, + handle_error: function () { + layer.handle_error.apply(layer, arguments) + } + } + Object.keys(layer).forEach(function (k) { + if (!fakeLayer[k]) { + fakeLayer[k] = layer[k] + } + }) + Object.keys(layer.constructor.prototype).forEach(function (k) { + if (!fakeLayer[k]) { + fakeLayer[k] = layer[k] + } + }) + return fakeLayer +} diff --git a/test/versioned/express/require.tap.js b/test/versioned/express/require.test.js similarity index 70% rename from test/versioned/express/require.tap.js rename to test/versioned/express/require.test.js index 38ff9ef11b..4e5a9b71f6 100644 --- a/test/versioned/express/require.tap.js +++ b/test/versioned/express/require.test.js @@ -4,15 +4,14 @@ */ 'use strict' - -const test = require('tap').test +const assert = require('node:assert') +const test = require('node:test') const helper = require('../../lib/agent_helper') -test("requiring express a bunch of times shouldn't leak listeners", function (t) { +test("requiring express a bunch of times shouldn't leak listeners", function () { const agent = helper.instrumentMockedAgent() require('express') const numListeners = agent.listeners('transactionFinished').length require('express') - t.equal(agent.listeners('transactionFinished').length, numListeners) - t.end() + assert.equal(agent.listeners('transactionFinished').length, numListeners) }) diff --git a/test/versioned/express/route-iteration.tap.js b/test/versioned/express/route-iteration.test.js similarity index 81% rename from test/versioned/express/route-iteration.tap.js rename to test/versioned/express/route-iteration.test.js index dea50d2e91..e621d099b9 100644 --- a/test/versioned/express/route-iteration.tap.js +++ b/test/versioned/express/route-iteration.test.js @@ -4,19 +4,19 @@ */ 'use strict' - -const test = require('tap').test +const tsplan = require('@matteo.collina/tspl') +const test = require('node:test') const helper = require('../../lib/agent_helper') test('new relic should not break route iteration', function (t) { - t.plan(1) + const plan = tsplan(t, { plan: 1 }) const agent = helper.instrumentMockedAgent() const express = require('express') const router = new express.Router() const childA = new express.Router() const childB = new express.Router() - t.teardown(() => { + t.after(() => { helper.unloadAgent(agent) }) @@ -35,7 +35,7 @@ test('new relic should not break route iteration', function (t) { router.use(childA) router.use(childB) - t.same(findAllRoutes(router, ''), ['/get', ['/test'], ['/hello']]) + plan.deepEqual(findAllRoutes(router, ''), ['/get', ['/test'], ['/hello']]) }) function findAllRoutes(router, path) { diff --git a/test/versioned/express/route-param.tap.js b/test/versioned/express/route-param.tap.js deleted file mode 100644 index a9327a5cae..0000000000 --- a/test/versioned/express/route-param.tap.js +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const test = require('tap').test -const helper = require('../../lib/agent_helper') - -test('Express route param', function (t) { - const agent = helper.instrumentMockedAgent() - const express = require('express') - const server = createServer(express) - - t.teardown(function () { - server.close(function () { - helper.unloadAgent(agent) - }) - }) - - server.listen(0, function () { - t.autoend() - const port = server.address().port - - t.test('pass-through param', function (t) { - t.plan(4) - - agent.once('transactionFinished', function (tx) { - t.equal( - tx.name, - 'WebTransaction/Expressjs/GET//a/b/:action/c', - 'should have correct transaction name' - ) - }) - - testRequest(port, 'foo', function (err, body) { - t.notOk(err, 'should not have errored') - t.equal(body.action, 'foo', 'should pass through correct parameter value') - t.equal(body.name, 'action', 'should pass through correct parameter name') - }) - }) - - t.test('respond from param', function (t) { - t.plan(3) - - agent.once('transactionFinished', function (tx) { - t.equal( - tx.name, - 'WebTransaction/Expressjs/GET//a/[param handler :action]', - 'should have correct transaction name' - ) - }) - - testRequest(port, 'deny', function (err, body) { - t.notOk(err, 'should not have errored') - t.equal(body, 'denied', 'should have responded from within paramware') - }) - }) - - t.test('in-active transaction in param handler', function (t) { - t.plan(4) - - agent.once('transactionFinished', function (tx) { - t.equal( - tx.name, - 'WebTransaction/Expressjs/GET//a/b/preempt/c', - 'should have correct transaction name' - ) - }) - - testRequest(port, 'preempt', function (err, body) { - t.notOk(err, 'should not have errored') - t.equal(body.action, 'preempt', 'should pass through correct parameter value') - t.equal(body.name, 'action', 'should pass through correct parameter name') - }) - }) - }) -}) - -function testRequest(port, param, cb) { - const url = 'http://localhost:' + port + '/a/b/' + param + '/c' - helper.makeGetRequest(url, function (err, _response, body) { - cb(err, body) - }) -} - -function createServer(express) { - const app = express() - - const aRouter = new express.Router() - const bRouter = new express.Router() - const cRouter = new express.Router() - - cRouter.get('', function (req, res) { - if (req.action !== 'preempt') { - res.json({ action: req.action, name: req.name }) - } - }) - - bRouter.use('/c', cRouter) - - aRouter.param('action', function (req, res, next, action, name) { - req.action = action - req.name = name - if (action === 'deny') { - res.status(200).json('denied') - } else { - next() - } - }) - - aRouter.use('/b/:action', bRouter) - app.use('/a/b/preempt/c', function (req, res, next) { - res.send({ action: 'preempt', name: 'action' }) - process.nextTick(next) - }) - app.use('/a', aRouter) - - return require('http').createServer(app) -} diff --git a/test/versioned/express/route-param.test.js b/test/versioned/express/route-param.test.js new file mode 100644 index 0000000000..836c34cec2 --- /dev/null +++ b/test/versioned/express/route-param.test.js @@ -0,0 +1,119 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { setup } = require('./utils') +const tsplan = require('@matteo.collina/tspl') + +test('Express route param', async function (t) { + t.beforeEach(async (ctx) => { + await setup(ctx) + createServer(ctx.nr) + }) + + t.afterEach(function (ctx) { + helper.unloadAgent(ctx.nr.agent) + ctx.nr.server.close() + }) + + await t.test('pass-through param', function (t, end) { + const { agent, port } = t.nr + const plan = tsplan(t, { plan: 4 }) + + agent.once('transactionFinished', function (tx) { + plan.equal( + tx.name, + 'WebTransaction/Expressjs/GET//a/b/:action/c', + 'should have correct transaction name' + ) + }) + + testRequest(port, 'foo', function (err, body) { + plan.ok(!err, 'should not have errored') + plan.equal(body.action, 'foo', 'should pass through correct parameter value') + plan.equal(body.name, 'action', 'should pass through correct parameter name') + end() + }) + }) + + await t.test('respond from param', function (t, end) { + const { agent, port } = t.nr + const plan = tsplan(t, { plan: 3 }) + + agent.once('transactionFinished', function (tx) { + plan.equal( + tx.name, + 'WebTransaction/Expressjs/GET//a/[param handler :action]', + 'should have correct transaction name' + ) + }) + + testRequest(port, 'deny', function (err, body) { + plan.ok(!err, 'should not have errored') + plan.equal(body, 'denied', 'should have responded from within paramware') + end() + }) + }) + + await t.test('in-active transaction in param handler', function (t, end) { + const { agent, port } = t.nr + const plan = tsplan(t, { plan: 4 }) + + agent.once('transactionFinished', function (tx) { + plan.equal( + tx.name, + 'WebTransaction/Expressjs/GET//a/b/preempt/c', + 'should have correct transaction name' + ) + }) + + testRequest(port, 'preempt', function (err, body) { + plan.ok(!err, 'should not have errored') + plan.equal(body.action, 'preempt', 'should pass through correct parameter value') + plan.equal(body.name, 'action', 'should pass through correct parameter name') + end() + }) + }) +}) + +function testRequest(port, param, cb) { + const url = 'http://localhost:' + port + '/a/b/' + param + '/c' + helper.makeGetRequest(url, function (err, _response, body) { + cb(err, body) + }) +} + +function createServer({ express, app }) { + const aRouter = new express.Router() + const bRouter = new express.Router() + const cRouter = new express.Router() + + cRouter.get('', function (req, res) { + if (req.action !== 'preempt') { + res.json({ action: req.action, name: req.name }) + } + }) + + bRouter.use('/c', cRouter) + + aRouter.param('action', function (req, res, next, action, name) { + req.action = action + req.name = name + if (action === 'deny') { + res.status(200).json('denied') + } else { + next() + } + }) + + aRouter.use('/b/:action', bRouter) + app.use('/a/b/preempt/c', function (req, res, next) { + res.send({ action: 'preempt', name: 'action' }) + process.nextTick(next) + }) + app.use('/a', aRouter) +} diff --git a/test/versioned/express/router-params.tap.js b/test/versioned/express/router-params.tap.js deleted file mode 100644 index 0c229c7f61..0000000000 --- a/test/versioned/express/router-params.tap.js +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const test = require('tap').test -const helper = require('../../lib/agent_helper') - -test('Express router introspection', function (t) { - t.plan(14) - - const agent = helper.instrumentMockedAgent({ - attributes: { - enabled: true, - include: ['request.parameters.*'] - } - }) - - const express = require('express') - const app = express() - const server = require('http').createServer(app) - - const router = express.Router() // eslint-disable-line new-cap - router.get('/b/:param2', function (req, res) { - t.ok(agent.getTransaction(), 'transaction is available') - - res.send({ status: 'ok' }) - res.end() - }) - app.use('/a/:param1', router) - - t.teardown(() => { - server.close(() => { - helper.unloadAgent(agent) - }) - }) - - agent.on('transactionFinished', function (transaction) { - t.equal( - transaction.name, - 'WebTransaction/Expressjs/GET//a/:param1/b/:param2', - 'transaction has expected name' - ) - - t.equal(transaction.url, '/a/foo/b/bar', 'URL is left alone') - t.equal(transaction.statusCode, 200, 'status code is OK') - t.equal(transaction.verb, 'GET', 'HTTP method is GET') - t.ok(transaction.trace, 'transaction has trace') - - const web = transaction.trace.root.children[0] - t.ok(web, 'trace has web segment') - t.equal(web.name, transaction.name, 'segment name and transaction name match') - t.equal( - web.partialName, - 'Expressjs/GET//a/:param1/b/:param2', - 'should have partial name for apdex' - ) - const attributes = web.getAttributes() - t.equal(attributes['request.parameters.route.param1'], 'foo', 'should have param1') - t.equal(attributes['request.parameters.route.param2'], 'bar', 'should have param2') - }) - - server.listen(0, function () { - const port = server.address().port - const url = 'http://localhost:' + port + '/a/foo/b/bar' - helper.makeGetRequest(url, function (error, res, body) { - t.error(error, 'should not have errored') - t.equal(res.statusCode, 200, 'should have ok status') - t.same(body, { status: 'ok' }, 'should have expected response') - }) - }) -}) diff --git a/test/versioned/express/router-params.test.js b/test/versioned/express/router-params.test.js new file mode 100644 index 0000000000..63892fdb71 --- /dev/null +++ b/test/versioned/express/router-params.test.js @@ -0,0 +1,72 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const test = require('node:test') +const helper = require('../../lib/agent_helper') +const { setup } = require('./utils') +const tsplan = require('@matteo.collina/tspl') + +test.beforeEach(async (ctx) => { + await setup(ctx, { + attributes: { + enabled: true, + include: ['request.parameters.*'] + } + }) +}) + +test.afterEach((ctx) => { + ctx.nr.server.close() + helper.unloadAgent(ctx.nr.agent) +}) + +test('Express router introspection', function (t, end) { + const { agent, app, express, port } = t.nr + const plan = tsplan(t, { plan: 14 }) + + const router = express.Router() // eslint-disable-line new-cap + router.get('/b/:param2', function (req, res) { + plan.ok(agent.getTransaction(), 'transaction is available') + + res.send({ status: 'ok' }) + res.end() + }) + app.use('/a/:param1', router) + + agent.on('transactionFinished', function (transaction) { + plan.equal( + transaction.name, + 'WebTransaction/Expressjs/GET//a/:param1/b/:param2', + 'transaction has expected name' + ) + + plan.equal(transaction.url, '/a/foo/b/bar', 'URL is left alone') + plan.equal(transaction.statusCode, 200, 'status code is OK') + plan.equal(transaction.verb, 'GET', 'HTTP method is GET') + plan.ok(transaction.trace, 'transaction has trace') + + const web = transaction.trace.root.children[0] + plan.ok(web, 'trace has web segment') + plan.equal(web.name, transaction.name, 'segment name and transaction name match') + plan.equal( + web.partialName, + 'Expressjs/GET//a/:param1/b/:param2', + 'should have partial name for apdex' + ) + const attributes = web.getAttributes() + plan.equal(attributes['request.parameters.route.param1'], 'foo', 'should have param1') + plan.equal(attributes['request.parameters.route.param2'], 'bar', 'should have param2') + }) + + const url = 'http://localhost:' + port + '/a/foo/b/bar' + helper.makeGetRequest(url, function (error, res, body) { + plan.ok(!error, 'should not have errored') + plan.equal(res.statusCode, 200, 'should have ok status') + plan.deepEqual(body, { status: 'ok' }, 'should have expected response') + end() + }) +}) diff --git a/test/versioned/express/segments.tap.js b/test/versioned/express/segments.test.js similarity index 77% rename from test/versioned/express/segments.tap.js rename to test/versioned/express/segments.test.js index 2092862c50..2d43aaab01 100644 --- a/test/versioned/express/segments.tap.js +++ b/test/versioned/express/segments.test.js @@ -4,12 +4,13 @@ */ 'use strict' - +const assert = require('node:assert') +const test = require('node:test') const { makeRequest, setup } = require('./utils') const NAMES = require('../../../lib/metrics/names') const { findSegment } = require('../../lib/metrics_helper') -const tap = require('tap') -const { test } = tap +const { assertMetrics, assertSegments, assertCLMAttrs } = require('../../lib/custom-assertions') +const helper = require('../../lib/agent_helper') const assertSegmentsOptions = { exact: true, @@ -23,9 +24,17 @@ const assertSegmentsOptions = { ] } -test('first two segments are built-in Express middlewares', function (t) { - setup(t) - const { app } = t.context +test.beforeEach(async (ctx) => { + await setup(ctx) +}) + +test.afterEach((ctx) => { + ctx.nr.server.close() + helper.unloadAgent(ctx.nr.agent) +}) + +test('first two segments are built-in Express middlewares', function (t, end) { + const { app } = t.nr app.all('/test', function (req, res) { res.end() @@ -33,22 +42,20 @@ test('first two segments are built-in Express middlewares', function (t) { runTest(t, function (segments, transaction) { // TODO: check for different HTTP methods - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + '']], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) - t.end() + end() }) }) -test('middleware with child segment gets named correctly', function (t) { - setup(t) - const { app } = t.context +test('middleware with child segment gets named correctly', function (t, end) { + const { app } = t.nr app.all('/test', function (req, res) { setTimeout(function () { @@ -57,59 +64,54 @@ test('middleware with child segment gets named correctly', function (t) { }) runTest(t, function (segments, transaction) { - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) - t.end() + end() }) }) -test('segments for route handler', function (t) { - setup(t) - const { app } = t.context +test('segments for route handler', function (t, end) { + const { app } = t.nr app.all('/test', function (req, res) { res.end() }) runTest(t, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + '']], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test']) - t.end() + end() }) }) -test('route function names are in segment names', function (t) { - setup(t) - const { app } = t.context +test('route function names are in segment names', function (t, end) { + const { app } = t.nr app.all('/test', function myHandler(req, res) { res.end() }) runTest(t, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + 'myHandler']], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//test']) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//test']) - t.end() + end() }) }) -test('middleware mounted on a path should produce correct names', function (t) { - setup(t) - const { app } = t.context +test('middleware mounted on a path should produce correct names', function (t, end) { + const { app } = t.nr app.use('/test/:id', function handler(req, res) { res.send() @@ -120,22 +122,16 @@ test('middleware mounted on a path should produce correct names', function (t) { transaction.trace.root, NAMES.EXPRESS.MIDDLEWARE + 'handler//test/:id' ) - t.ok(segment) + assert.ok(segment) - checkMetrics( - t, - transaction.metrics, - [NAMES.EXPRESS.MIDDLEWARE + 'handler//test/:id'], - '/test/:id' - ) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'handler//test/:id'], '/test/:id') - t.end() + end() }) }) -test('each handler in route has its own segment', function (t) { - setup(t) - const { app } = t.context +test('each handler in route has its own segment', function (t, end) { + const { app } = t.nr app.all( '/test', @@ -148,8 +144,7 @@ test('each handler in route has its own segment', function (t) { ) runTest(t, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Route Path: /test', @@ -158,18 +153,17 @@ test('each handler in route has its own segment', function (t) { assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [ + checkMetrics(transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + 'handler1//test', NAMES.EXPRESS.MIDDLEWARE + 'handler2//test' ]) - t.end() + end() }) }) -test('segments for routers', function (t) { - setup(t) - const { app, express } = t.context +test('segments for routers', function (t, end) { + const { app, express } = t.nr const router = express.Router() // eslint-disable-line new-cap router.all('/test', function (req, res) { @@ -179,8 +173,7 @@ test('segments for routers', function (t) { app.use('/router1', router) runTest(t, '/router1/test', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router1', @@ -190,19 +183,17 @@ test('segments for routers', function (t) { ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//router1/test'], '/router1/test' ) - t.end() + end() }) }) -test('two root routers', function (t) { - setup(t) - const { app, express } = t.context +test('two root routers', function (t, end) { + const { app, express } = t.nr const router1 = express.Router() // eslint-disable-line new-cap router1.all('/', function (req, res) { @@ -217,8 +208,7 @@ test('two root routers', function (t) { app.use('/', router2) runTest(t, '/test', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /', @@ -228,15 +218,14 @@ test('two root routers', function (t) { assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test'], '/test') + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//test'], '/test') - t.end() + end() }) }) -test('router mounted as a route handler', function (t) { - setup(t) - const { app, express, isExpress5 } = t.context +test('router mounted as a route handler', function (t, end) { + const { app, express, isExpress5 } = t.nr const router1 = express.Router() // eslint-disable-line new-cap router1.all('/test', function testHandler(req, res) { @@ -257,8 +246,7 @@ test('router mounted as a route handler', function (t) { app.get(path, router1) runTest(t, '/test', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ `Expressjs/Route Path: ${segmentPath}`, @@ -271,19 +259,17 @@ test('router mounted as a route handler', function (t) { ) checkMetrics( - t, transaction.metrics, [`${NAMES.EXPRESS.MIDDLEWARE}testHandler/${metricsPath}/test`], `${metricsPath}/test` ) - t.end() + end() }) }) -test('segments for routers', function (t) { - setup(t) - const { app, express } = t.context +test('segments for routers', function (t, end) { + const { app, express } = t.nr const router = express.Router() // eslint-disable-line new-cap router.all('/test', function (req, res) { @@ -293,8 +279,7 @@ test('segments for routers', function (t) { app.use('/router1', router) runTest(t, '/router1/test', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router1', @@ -304,19 +289,17 @@ test('segments for routers', function (t) { ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//router1/test'], '/router1/test' ) - t.end() + end() }) }) -test('segments for sub-app', function (t) { - setup(t) - const { app, express, isExpress5 } = t.context +test('segments for sub-app', function (t, end) { + const { app, express, isExpress5 } = t.nr const subapp = express() subapp.all('/test', function (req, res) { @@ -331,27 +314,24 @@ test('segments for sub-app', function (t) { ? NAMES.EXPRESS.MIDDLEWARE + 'app//subapp1' : 'Expressjs/Mounted App: /subapp1' - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [firstSegment, ['Expressjs/Route Path: /test', [NAMES.EXPRESS.MIDDLEWARE + '']]], assertSegmentsOptions ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//subapp1/test'], '/subapp1/test' ) - t.end() + end() }) }) -test('segments for sub-app router', function (t) { - setup(t) - const { app, express, isExpress5 } = t.context +test('segments for sub-app router', function (t, end) { + const { app, express, isExpress5 } = t.nr const subapp = express() subapp.get( @@ -374,8 +354,7 @@ test('segments for sub-app router', function (t) { const firstSegment = isExpress5 ? NAMES.EXPRESS.MIDDLEWARE + 'app//subapp1' : 'Expressjs/Mounted App: /subapp1' - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ firstSegment, @@ -390,19 +369,17 @@ test('segments for sub-app router', function (t) { ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//subapp1/test'], '/subapp1/test' ) - t.end() + end() }) }) -test('segments for wildcard', function (t) { - setup(t) - const { app, express, isExpress5 } = t.context +test('segments for wildcard', function (t, end) { + const { app, express, isExpress5 } = t.nr const subapp = express() subapp.all('/:app', function (req, res) { @@ -416,27 +393,24 @@ test('segments for wildcard', function (t) { const firstSegment = isExpress5 ? NAMES.EXPRESS.MIDDLEWARE + 'app//subapp1' : 'Expressjs/Mounted App: /subapp1' - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [firstSegment, ['Expressjs/Route Path: /:app', [NAMES.EXPRESS.MIDDLEWARE + '']]], assertSegmentsOptions ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//subapp1/:app'], '/subapp1/:app' ) - t.end() + end() }) }) -test('router with subapp', function (t) { - setup(t) - const { app, express, isExpress5 } = t.context +test('router with subapp', function (t, end) { + const { app, express, isExpress5 } = t.nr const router = express.Router() // eslint-disable-line new-cap const subapp = express() @@ -451,8 +425,7 @@ test('router with subapp', function (t) { const subAppSegment = isExpress5 ? NAMES.EXPRESS.MIDDLEWARE + 'app//subapp1' : 'Expressjs/Mounted App: /subapp1' - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router1', @@ -462,41 +435,37 @@ test('router with subapp', function (t) { ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + '//router1/subapp1/test'], '/router1/subapp1/test' ) - t.end() + end() }) }) -test('mounted middleware', function (t) { - setup(t) - const { app } = t.context +test('mounted middleware', function (t, end) { + const { app } = t.nr app.use('/test', function myHandler(req, res) { res.end() }) runTest(t, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//test'], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//test']) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//test']) - t.end() + end() }) }) -test('error middleware', function (t) { - setup(t) - const { app } = t.context +test('error middleware', function (t, end) { + const { app } = t.nr app.get('/test', function () { throw new Error('some error') @@ -507,8 +476,7 @@ test('error middleware', function (t) { }) runTest(t, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Route Path: /test', @@ -519,7 +487,6 @@ test('error middleware', function (t) { ) checkMetrics( - t, transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + '//test', @@ -528,13 +495,12 @@ test('error middleware', function (t) { '/test' ) - t.end() + end() }) }) -test('error handler in router', function (t) { - setup(t) - const { app, express } = t.context +test('error handler in router', function (t, end) { + const { app, express } = t.nr const router = express.Router() // eslint-disable-line new-cap @@ -557,8 +523,7 @@ test('error handler in router', function (t) { errors: 0 }, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router', @@ -572,7 +537,6 @@ test('error handler in router', function (t) { ) checkMetrics( - t, transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + '//router/test', @@ -581,14 +545,13 @@ test('error handler in router', function (t) { endpoint ) - t.end() + end() } ) }) -test('error handler in second router', function (t) { - setup(t) - const { app, express } = t.context +test('error handler in second router', function (t, end) { + const { app, express } = t.nr const router1 = express.Router() // eslint-disable-line new-cap const router2 = express.Router() // eslint-disable-line new-cap @@ -613,8 +576,7 @@ test('error handler in second router', function (t) { errors: 0 }, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router1', @@ -631,7 +593,6 @@ test('error handler in second router', function (t) { ) checkMetrics( - t, transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + '//router1/router2/test', @@ -640,15 +601,13 @@ test('error handler in second router', function (t) { endpoint ) - t.end() + end() } ) }) -test('error handler outside of router', function (t) { - setup(t) - - const { app, express } = t.context +test('error handler outside of router', function (t, end) { + const { app, express } = t.nr const router = express.Router() // eslint-disable-line new-cap @@ -670,8 +629,7 @@ test('error handler outside of router', function (t) { errors: 0 }, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router', @@ -682,7 +640,6 @@ test('error handler outside of router', function (t) { ) checkMetrics( - t, transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + '//router/test', @@ -691,14 +648,13 @@ test('error handler outside of router', function (t) { endpoint ) - t.end() + end() } ) }) -test('error handler outside of two routers', function (t) { - setup(t) - const { app, express } = t.context +test('error handler outside of two routers', function (t, end) { + const { app, express } = t.nr const router1 = express.Router() // eslint-disable-line new-cap const router2 = express.Router() // eslint-disable-line new-cap @@ -723,8 +679,7 @@ test('error handler outside of two routers', function (t) { errors: 0 }, function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], [ 'Expressjs/Router: /router1', @@ -738,7 +693,6 @@ test('error handler outside of two routers', function (t) { ) checkMetrics( - t, transaction.metrics, [ NAMES.EXPRESS.MIDDLEWARE + '//router1/router2/test', @@ -747,88 +701,81 @@ test('error handler outside of two routers', function (t) { endpoint ) - t.end() + end() } ) }) -test('when using a route variable', function (t) { - setup(t) - const { app } = t.context +test('when using a route variable', function (t, end) { + const { app } = t.nr app.get('/:foo/:bar', function myHandler(req, res) { res.end() }) runTest(t, '/a/b', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: /:foo/:bar', [NAMES.EXPRESS.MIDDLEWARE + 'myHandler']], assertSegmentsOptions ) checkMetrics( - t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//:foo/:bar'], '/:foo/:bar' ) - t.end() + end() }) }) -test('when using a string pattern in path', function (t) { - setup(t) - const { app } = t.context +test('when using a string pattern in path', function (t, end) { + const { app } = t.nr - const path = t.context.isExpress5 ? /ab?cd/ : '/ab?cd' + const path = t.nr.isExpress5 ? /ab?cd/ : '/ab?cd' app.get(path, function myHandler(req, res) { res.end() }) runTest(t, '/abcd', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: ' + path, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler']], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler/' + path], path) + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler/' + path], path) - t.end() + end() }) }) -test('when using a regular expression in path', function (t) { - setup(t) - const { app } = t.context +test('when using a regular expression in path', function (t, end) { + const { app } = t.nr app.get(/a/, function myHandler(req, res) { res.end() }) runTest(t, '/a', function (segments, transaction) { - checkSegments( - t, + assertSegments( transaction.trace.root.children[0], ['Expressjs/Route Path: /a/', [NAMES.EXPRESS.MIDDLEWARE + 'myHandler']], assertSegmentsOptions ) - checkMetrics(t, transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//a/'], '/a/') + checkMetrics(transaction.metrics, [NAMES.EXPRESS.MIDDLEWARE + 'myHandler//a/'], '/a/') - t.end() + end() }) }) const codeLevelMetrics = [true, false] -codeLevelMetrics.forEach((enabled) => { - test(`Code Level Metrics ${enabled}`, function (t) { - setup(t, { code_level_metrics: { enabled } }) - const { app } = t.context +for (const enabled of codeLevelMetrics) { + test(`Code Level Metrics ${enabled}`, function (t, end) { + const { app, agent } = t.nr + agent.config.code_level_metrics.enabled = enabled function mw1(req, res, next) { next() @@ -845,8 +792,8 @@ codeLevelMetrics.forEach((enabled) => { runTest(t, '/chained', function (segments, transaction) { const routeSegment = findSegment(transaction.trace.root, 'Expressjs/Route Path: /chained') const [mw1Segment, mw2Segment, handlerSegment] = routeSegment.children - const defaultPath = 'test/versioned/express/segments.tap.js' - t.clmAttrs({ + const defaultPath = 'test/versioned/express/segments.test.js' + assertCLMAttrs({ segments: [ { segment: mw1Segment, @@ -866,13 +813,13 @@ codeLevelMetrics.forEach((enabled) => { ], enabled }) - t.end() + end() }) }) -}) +} function runTest(t, options, callback) { - const { agent, app } = t.context + const { agent, server } = t.nr let errors let endpoint @@ -891,27 +838,17 @@ function runTest(t, options, callback) { agent.on('transactionFinished', function (tx) { const baseSegment = tx.trace.root.children[0] - t.equal(agent.errors.traceAggregator.errors.length, errors, 'should have errors') + assert.equal(agent.errors.traceAggregator.errors.length, errors, 'should have errors') callback(baseSegment.children, tx) }) - const server = app.listen(function () { - makeRequest(this, endpoint, function (response) { - response.resume() - }) - }) - - t.teardown(() => { - server.close() + makeRequest(server, endpoint, function (response) { + response.resume() }) } -function checkSegments(t, segments, expected, opts) { - t.assertSegments(segments, expected, opts) -} - -function checkMetrics(t, metrics, expected, path) { +function checkMetrics(metrics, expected, path) { if (path === undefined) { path = '/test' } @@ -933,5 +870,5 @@ function checkMetrics(t, metrics, expected, path) { expectedAll.push([{ name: metric, scope: 'WebTransaction/Expressjs/GET/' + path }]) } - t.assertMetrics(metrics, expectedAll, false, false) + assertMetrics(metrics, expectedAll, false, false) } diff --git a/test/versioned/express/transaction-naming.tap.js b/test/versioned/express/transaction-naming.test.js similarity index 53% rename from test/versioned/express/transaction-naming.tap.js rename to test/versioned/express/transaction-naming.test.js index 2bbbd26cbd..2688f56d8f 100644 --- a/test/versioned/express/transaction-naming.tap.js +++ b/test/versioned/express/transaction-naming.test.js @@ -4,27 +4,32 @@ */ 'use strict' - +const assert = require('node:assert') const http = require('http') -const test = require('tap').test +const test = require('node:test') const semver = require('semver') const { version: pkgVersion } = require('express/package') -const { makeRequest, setup } = require('./utils') +const { makeRequest, setup, teardown } = require('./utils') +const tsplan = require('@matteo.collina/tspl') + +test.beforeEach(async (ctx) => { + await setup(ctx) +}) -test('transaction name with single route', function (t) { - setup(t) - const { app } = t.context +test.afterEach(teardown) + +test('transaction name with single route', function (t, end) { + const { app } = t.nr app.get('/path1', function (req, res) { res.end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('transaction name with no matched routes', function (t) { - setup(t) - const { agent, app, isExpress5 } = t.context +test('transaction name with no matched routes', function (t, end) { + const { agent, app, isExpress5, server } = t.nr app.get('/path1', function (req, res) { res.end() @@ -34,20 +39,15 @@ test('transaction name with no matched routes', function (t) { const txPrefix = isExpress5 ? 'WebTransaction/Nodejs' : 'WebTransaction/Expressjs' agent.on('transactionFinished', function (transaction) { - t.equal(transaction.name, `${txPrefix}/GET/(not found)`, 'transaction has expected name') - t.end() - }) - const server = app.listen(function () { - makeRequest(this, endpoint) - }) - t.teardown(() => { - server.close() + assert.equal(transaction.name, `${txPrefix}/GET/(not found)`, 'transaction has expected name') + end() }) + + makeRequest(server, endpoint) }) -test('transaction name with route that has multiple handlers', function (t) { - setup(t) - const { app } = t.context +test('transaction name with route that has multiple handlers', function (t, end) { + const { app } = t.nr app.get( '/path1', @@ -59,12 +59,11 @@ test('transaction name with route that has multiple handlers', function (t) { } ) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1', expectedName: '/path1' }) }) -test('transaction name with router middleware', function (t) { - setup(t) - const { app, express } = t.context +test('transaction name with router middleware', function (t, end) { + const { app, express } = t.nr const router = new express.Router() router.get('/path1', function (req, res) { @@ -73,12 +72,11 @@ test('transaction name with router middleware', function (t) { app.use(router) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('transaction name with middleware function', function (t) { - setup(t) - const { app } = t.context +test('transaction name with middleware function', function (t, end) { + const { app } = t.nr app.use('/path1', function (req, res, next) { next() @@ -88,12 +86,11 @@ test('transaction name with middleware function', function (t) { res.end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('transaction name with shared middleware function', function (t) { - setup(t) - const { app } = t.context +test('transaction name with shared middleware function', function (t, end) { + const { app } = t.nr app.use(['/path1', '/path2'], function (req, res, next) { next() @@ -103,23 +100,21 @@ test('transaction name with shared middleware function', function (t) { res.end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('transaction name when ending in shared middleware', function (t) { - setup(t) - const { app } = t.context +test('transaction name when ending in shared middleware', function (t, end) { + const { app } = t.nr app.use(['/path1', '/path2'], function (req, res) { res.end() }) - runTest(t, '/path1', '/path1,/path2') + runTest({ t, end, endpoint: '/path1', expectedName: '/path1,/path2' }) }) -test('transaction name with subapp middleware', function (t) { - setup(t) - const { app, express } = t.context +test('transaction name with subapp middleware', function (t, end) { + const { app, express } = t.nr const subapp = express() @@ -129,12 +124,11 @@ test('transaction name with subapp middleware', function (t) { app.use(subapp) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('transaction name with subrouter', function (t) { - setup(t) - const { app, express } = t.context +test('transaction name with subrouter', function (t, end) { + const { app, express } = t.nr const router = new express.Router() @@ -144,12 +138,11 @@ test('transaction name with subrouter', function (t) { app.use('/api', router) - runTest(t, '/api/path1', '/api/path1') + runTest({ t, end, endpoint: '/api/path1' }) }) -test('multiple route handlers with the same name do not duplicate transaction name', function (t) { - setup(t) - const { app } = t.context +test('multiple route handlers with the same name do not duplicate transaction name', function (t, end) { + const { app } = t.nr app.get('/path1', function (req, res, next) { next() @@ -159,36 +152,33 @@ test('multiple route handlers with the same name do not duplicate transaction na res.end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('responding from middleware', function (t) { - setup(t) - const { app } = t.context +test('responding from middleware', function (t, end) { + const { app } = t.nr app.use('/test', function (req, res, next) { res.send('ok') next() }) - runTest(t, '/test') + runTest({ t, end, endpoint: '/test' }) }) -test('responding from middleware with parameter', function (t) { - setup(t) - const { app } = t.context +test('responding from middleware with parameter', function (t, end) { + const { app } = t.nr app.use('/test', function (req, res, next) { res.send('ok') next() }) - runTest(t, '/test/param', '/test') + runTest({ t, end, endpoint: '/test/param', expectedName: '/test' }) }) -test('with error', function (t) { - setup(t) - const { app } = t.context +test('with error', function (t, end) { + const { app } = t.nr app.get('/path1', function (req, res, next) { next(new Error('some error')) @@ -198,12 +188,11 @@ test('with error', function (t) { return res.status(500).end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('with error and path-specific error handler', function (t) { - setup(t) - const { app } = t.context +test('with error and path-specific error handler', function (t, end) { + const { app } = t.nr app.get('/path1', function () { throw new Error('some error') @@ -213,12 +202,11 @@ test('with error and path-specific error handler', function (t) { res.status(500).end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) -test('when router error is handled outside of the router', function (t) { - setup(t) - const { app, express } = t.context +test('when router error is handled outside of the router', function (t, end) { + const { app, express } = t.nr const router = new express.Router() @@ -233,23 +221,21 @@ test('when router error is handled outside of the router', function (t) { return res.status(500).end() }) - runTest(t, '/router1/path1', '/router1/path1') + runTest({ t, end, endpoint: '/router1/path1' }) }) -test('when using a route variable', function (t) { - setup(t) - const { app } = t.context +test('when using a route variable', function (t, end) { + const { app } = t.nr app.get('/:foo/:bar', function (req, res) { res.end() }) - runTest(t, '/foo/bar', '/:foo/:bar') + runTest({ t, end, endpoint: '/foo/bar', expectedName: '/:foo/:bar' }) }) -test('when using a string pattern in path', function (t) { - setup(t) - const { app, isExpress5 } = t.context +test('when using a string pattern in path', function (t, end) { + const { app, isExpress5 } = t.nr const path = isExpress5 ? /ab?cd/ : '/ab?cd' @@ -257,23 +243,21 @@ test('when using a string pattern in path', function (t) { res.end() }) - runTest(t, '/abcd', path) + runTest({ t, end, endpoint: '/abcd', expectedName: path }) }) -test('when using a regular expression in path', function (t) { - setup(t) - const { app } = t.context +test('when using a regular expression in path', function (t, end) { + const { app } = t.nr app.get(/a/, function (req, res) { res.end() }) - runTest(t, '/abcd', '/a/') + runTest({ t, end, endpoint: '/abcd', expectedName: '/a/' }) }) -test('when using router with a route variable', function (t) { - setup(t) - const { app, express } = t.context +test('when using router with a route variable', function (t, end) { + const { app, express } = t.nr const router = express.Router() // eslint-disable-line new-cap @@ -283,12 +267,11 @@ test('when using router with a route variable', function (t) { app.use('/:var1', router) - runTest(t, '/foo/bar/path1', '/:var1/:var2/path1') + runTest({ t, end, endpoint: '/foo/bar/path1', expectedName: '/:var1/:var2/path1' }) }) -test('when mounting a subapp using a variable', function (t) { - setup(t) - const { app, express } = t.context +test('when mounting a subapp using a variable', function (t, end) { + const { app, express } = t.nr const subapp = express() subapp.get('/:var2/path1', function (req, res) { @@ -297,12 +280,11 @@ test('when mounting a subapp using a variable', function (t) { app.use('/:var1', subapp) - runTest(t, '/foo/bar/path1', '/:var1/:var2/path1') + runTest({ t, end, endpoint: '/foo/bar/path1', expectedName: '/:var1/:var2/path1' }) }) -test('using two routers', function (t) { - setup(t) - const { app, express } = t.context +test('using two routers', function (t, end) { + const { app, express } = t.nr const router1 = express.Router() // eslint-disable-line new-cap const router2 = express.Router() // eslint-disable-line new-cap @@ -314,12 +296,11 @@ test('using two routers', function (t) { res.end() }) - runTest(t, '/router1/router2/path1', '/:router1/:router2/path1') + runTest({ t, end, endpoint: '/router1/router2/path1', expectedName: '/:router1/:router2/path1' }) }) -test('transactions running in parallel should be recorded correctly', function (t) { - setup(t) - const { app, express } = t.context +test('transactions running in parallel should be recorded correctly', function (t, end) { + const { app, express, server } = t.nr const router1 = express.Router() // eslint-disable-line new-cap const router2 = express.Router() // eslint-disable-line new-cap @@ -333,83 +314,73 @@ test('transactions running in parallel should be recorded correctly', function ( }) const numTests = 4 - const runner = makeMultiRunner(t, '/router1/router2/path1', '/:router1/:router2/path1', numTests) - app.listen(function () { - t.teardown(() => { - this.close() - }) - for (let i = 0; i < numTests; i++) { - runner(this) - } + const runner = makeMultiRunner({ + t, + end, + endpoint: '/router1/router2/path1', + expectedName: '/:router1/:router2/path1', + numTests }) + + for (let i = 0; i < numTests; i++) { + runner(server) + } }) -test('names transaction when request is aborted', function (t) { - t.plan(4) - setup(t) - const { agent, app } = t.context +test('names transaction when request is aborted', function (t, end) { + const plan = tsplan(t, { plan: 5 }) + + const { agent, app, port } = t.nr let request = null app.get('/test', function (req, res, next) { - t.comment('middleware') - t.ok(agent.getTransaction(), 'transaction exists') + plan.ok(agent.getTransaction(), 'transaction exists') // generate error after client has aborted request.abort() setTimeout(function () { - t.comment('timed out') - t.ok(agent.getTransaction() == null, 'transaction has already ended') + plan.ok(agent.getTransaction() == null, 'transaction has already ended') next(new Error('some error')) }, 100) }) // eslint-disable-next-line no-unused-vars app.use(function (error, req, res, next) { - t.comment('errorware') - t.ok(agent.getTransaction() == null, 'no active transaction when responding') + plan.ok(agent.getTransaction() == null, 'no active transaction when responding') res.end() + end() }) - const server = app.listen(function () { - t.comment('making request') - const port = this.address().port - request = http.request( - { - hostname: 'localhost', - port: port, - path: '/test' - }, - function () {} - ) - request.end() + request = http.request( + { + hostname: 'localhost', + port, + path: '/test' + }, + function () {} + ) + request.end() - // add error handler, otherwise aborting will cause an exception - request.on('error', function (err) { - t.comment('request errored: ' + err) - }) - request.on('abort', function () { - t.comment('request aborted') - }) + // add error handler, otherwise aborting will cause an exception + request.on('error', function (err) { + plan.equal(err.code, 'ECONNRESET') }) agent.on('transactionFinished', function (tx) { - t.equal(tx.name, 'WebTransaction/Expressjs/GET//test') - }) - - t.teardown(() => { - server.close() + plan.equal(tx.name, 'WebTransaction/Expressjs/GET//test') }) }) -test('Express transaction names are unaffected by errorware', function (t) { - t.plan(1) - setup(t) - const { agent, app } = t.context +test('Express transaction names are unaffected by errorware', function (t, end) { + const plan = tsplan(t, { plan: 1 }) + + const { agent, app, port } = t.nr agent.on('transactionFinished', function (tx) { const expected = 'WebTransaction/Expressjs/GET//test' - t.equal(tx.trace.root.children[0].name, expected) + plan.equal(tx.trace.root.children[0].name, expected) + end() }) app.use('/test', function () { @@ -421,16 +392,10 @@ test('Express transaction names are unaffected by errorware', function (t) { res.send(err.message) }) - const server = app.listen(function () { - http.request({ port: this.address().port, path: '/test' }).end() - }) - - t.teardown(function () { - server.close() - }) + http.request({ port, path: '/test' }).end() }) -test('when next is called after transaction state loss', function (t) { +test('when next is called after transaction state loss', function (t, end) { // Uninstrumented work queue. This must be set up before the agent is loaded // so that no transaction state is maintained. const tasks = [] @@ -440,9 +405,12 @@ test('when next is called after transaction state loss', function (t) { } }, 10) - setup(t) - const { agent, app } = t.context - t.plan(3) + t.after(function () { + clearInterval(interval) + }) + + const { agent, app, port } = t.nr + const plan = tsplan(t, { plan: 3 }) let transactionsFinished = 0 const transactionNames = [ @@ -450,7 +418,7 @@ test('when next is called after transaction state loss', function (t) { 'WebTransaction/Expressjs/GET//foo' ] agent.on('transactionFinished', function (tx) { - t.equal( + plan.equal( tx.name, transactionNames[transactionsFinished++], 'should have expected name ' + transactionsFinished @@ -473,48 +441,39 @@ test('when next is called after transaction state loss', function (t) { res.send('bar done\n') }) - const server = app.listen(function () { - const port = this.address().port + // Send first request to `/foo` which is slow and uses the work queue. + http.get({ port: port, path: '/foo' }, function (res) { + res.resume() + res.on('end', function () { + plan.equal(transactionsFinished, 2, 'should have two transactions done') + end() + }) + }) - // Send first request to `/foo` which is slow and uses the work queue. - http.get({ port: port, path: '/foo' }, function (res) { + // Send the second request after a short wait `/bar` which is fast and + // does not use the work queue. + setTimeout(function () { + http.get({ port: port, path: '/bar' }, function (res) { res.resume() - res.on('end', function () { - t.equal(transactionsFinished, 2, 'should have two transactions done') - t.end() - }) }) - - // Send the second request after a short wait `/bar` which is fast and - // does not use the work queue. - setTimeout(function () { - http.get({ port: port, path: '/bar' }, function (res) { - res.resume() - }) - }, 100) - }) - t.teardown(function () { - server.close() - clearInterval(interval) - }) + }, 100) }) // express did not add array based middleware registration // without path until 4.9.2 // https://github.com/expressjs/express/blob/master/History.md#492--2014-09-17 -// TODO: run on express beta 5 if (semver.satisfies(pkgVersion, '>=4.9.2')) { - test('transaction name with array of middleware with unspecified mount path', (t) => { - setup(t) - const { app } = t.context + test('transaction name with array of middleware with unspecified mount path', (t, end) => { + const plan = tsplan(t, { plan: 2 }) + const { app } = t.nr function mid1(req, res, next) { - t.pass('mid1 is executed') + plan.ok(1, 'mid1 is executed') next() } function mid2(req, res, next) { - t.pass('mid2 is executed') + plan.ok(1, 'mid2 is executed') next() } @@ -524,20 +483,20 @@ if (semver.satisfies(pkgVersion, '>=4.9.2')) { res.end() }) - runTest(t, '/path1', '/path1') + runTest({ t, end, endpoint: '/path1' }) }) - test('transaction name when ending in array of unmounted middleware', (t) => { - setup(t) - const { app } = t.context + test('transaction name when ending in array of unmounted middleware', (t, end) => { + const plan = tsplan(t, { plan: 2 }) + const { app } = t.nr function mid1(req, res, next) { - t.pass('mid1 is executed') + plan.ok(1, 'mid1 is executed') next() } function mid2(req, res) { - t.pass('mid2 is executed') + plan.ok(1, 'mid2 is executed') res.end() } @@ -545,21 +504,21 @@ if (semver.satisfies(pkgVersion, '>=4.9.2')) { app.use(mid1) - runTest(t, '/path1', '/') + runTest({ t, end, endpoint: '/path1', expectedName: '/' }) }) } -function makeMultiRunner(t, endpoint, expectedName, numTests) { - const { agent } = t.context +function makeMultiRunner({ t, endpoint, expectedName, numTests, end }) { + const { agent } = t.nr let done = 0 const seen = new Set() if (!expectedName) { expectedName = endpoint } agent.on('transactionFinished', function (transaction) { - t.notOk(seen.has(transaction), 'should never see the finishing transaction twice') + assert.ok(!seen.has(transaction), 'should never see the finishing transaction twice') seen.add(transaction) - t.equal( + assert.equal( transaction.name, 'WebTransaction/Expressjs/GET/' + expectedName, 'transaction has expected name' @@ -567,7 +526,7 @@ function makeMultiRunner(t, endpoint, expectedName, numTests) { transaction.end() if (++done === numTests) { done = 0 - t.end() + end() } }) return function runMany(server) { @@ -575,23 +534,18 @@ function makeMultiRunner(t, endpoint, expectedName, numTests) { } } -function runTest(t, endpoint, expectedName) { - const { app, agent } = t.context +function runTest({ t, endpoint, expectedName, end }) { + const { agent, server } = t.nr if (!expectedName) { expectedName = endpoint } agent.on('transactionFinished', function (transaction) { - t.equal( + assert.equal( transaction.name, 'WebTransaction/Expressjs/GET/' + expectedName, 'transaction has expected name' ) - t.end() - }) - const server = app.listen(function () { - makeRequest(this, endpoint) - }) - t.teardown(() => { - server.close() + end() }) + makeRequest(server, endpoint) } diff --git a/test/versioned/express/utils.js b/test/versioned/express/utils.js index 18c39b3856..b8939fbc08 100644 --- a/test/versioned/express/utils.js +++ b/test/versioned/express/utils.js @@ -7,6 +7,7 @@ const http = require('http') const helper = require('../../lib/agent_helper') const semver = require('semver') +const promiseResolvers = require('../../lib/promise-resolvers') function isExpress5() { const { version } = require('express/package') @@ -15,22 +16,33 @@ function isExpress5() { function makeRequest(server, path, callback) { const port = server.address().port - http.request({ port: port, path: path }, callback).end() + http.request({ port, path }, callback).end() } -function setup(t, config = {}) { - t.context.agent = helper.instrumentMockedAgent(config) - t.context.isExpress5 = isExpress5() +async function setup(ctx, config = {}) { + ctx.nr = {} + ctx.nr.agent = helper.instrumentMockedAgent(config) + ctx.nr.isExpress5 = isExpress5() - t.context.express = require('express') - t.context.app = t.context.express() - t.teardown(() => { - helper.unloadAgent(t.context.agent) - }) + ctx.nr.express = require('express') + ctx.nr.app = ctx.nr.express() + const { promise, resolve } = promiseResolvers() + const server = require('http').createServer(ctx.nr.app) + server.listen(0, 'localhost', resolve) + await promise + ctx.nr.server = server + ctx.nr.port = server.address().port +} + +function teardown(ctx) { + const { server, agent } = ctx.nr + server.close() + helper.unloadAgent(agent) } module.exports = { isExpress5, makeRequest, - setup + setup, + teardown }