From c97c000053100bd6a772c959c42cfa4294d91d1b Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 23 Jan 2023 15:57:27 +0000 Subject: [PATCH 01/10] test(nextjs): New server-side integration tests. --- packages/nextjs/jest.config.js | 2 +- .../nextjs/test/integration/jest.config.js | 11 ++ .../nextjs/test/integration/jest.setup.js | 8 + .../nextjs/test/integration/test/runner.js | 130 ------------- .../nextjs/test/integration/test/server.js | 18 -- .../test/server/cjsApiEndpoints.js | 55 ------ .../test/server/cjsApiEndpoints.test.ts | 69 +++++++ .../test/server/doubleEndMethodOnVercel.js | 14 -- .../server/doubleEndMethodOnVercel.test.ts | 18 ++ .../test/server/errorApiEndpoint.js | 56 ------ .../test/server/errorApiEndpoint.test.ts | 59 ++++++ .../test/server/errorServerSideProps.js | 59 ------ .../test/server/errorServerSideProps.test.ts | 60 ++++++ .../test/server/excludedApiEndpoints.js | 67 ------- .../test/server/excludedApiEndpoints.test.ts | 29 +++ .../integration/test/server/tracing200.js | 37 ---- .../test/server/tracing200.test.ts | 33 ++++ .../integration/test/server/tracing500.js | 36 ---- .../test/server/tracing500.test.ts | 33 ++++ .../integration/test/server/tracingHttp.js | 53 ----- .../test/server/tracingHttp.test.ts | 45 +++++ .../server/tracingServerGetInitialProps.js | 36 ---- .../tracingServerGetInitialProps.test.ts | 32 +++ .../server/tracingServerGetServerSideProps.js | 36 ---- .../tracingServerGetServerSideProps.test.ts | 32 +++ ...erGetServerSidePropsCustomPageExtension.js | 36 ---- ...ServerSidePropsCustomPageExtension.test.ts | 32 +++ .../test/server/tracingWithSentryAPI.js | 69 ------- .../test/server/tracingWithSentryAPI.test.ts | 66 +++++++ .../test/integration/test/utils/common.js | 91 --------- .../test/integration/test/utils/common.ts | 65 +++++++ .../test/integration/test/utils/server.js | 183 ------------------ .../test/integration/test/utils/server.ts | 119 ++++++++++++ .../nextjs/test/integration/tsconfig.json | 19 +- packages/nextjs/test/run-integration-tests.sh | 2 +- packages/nextjs/tsconfig.json | 1 + .../node-integration-tests/utils/index.ts | 49 ++++- 37 files changed, 776 insertions(+), 984 deletions(-) create mode 100644 packages/nextjs/test/integration/jest.config.js create mode 100644 packages/nextjs/test/integration/jest.setup.js delete mode 100644 packages/nextjs/test/integration/test/runner.js delete mode 100644 packages/nextjs/test/integration/test/server.js delete mode 100644 packages/nextjs/test/integration/test/server/cjsApiEndpoints.js create mode 100644 packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.js create mode 100644 packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/errorApiEndpoint.js create mode 100644 packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/errorServerSideProps.js create mode 100644 packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/excludedApiEndpoints.js create mode 100644 packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracing200.js create mode 100644 packages/nextjs/test/integration/test/server/tracing200.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracing500.js create mode 100644 packages/nextjs/test/integration/test/server/tracing500.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracingHttp.js create mode 100644 packages/nextjs/test/integration/test/server/tracingHttp.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js create mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js create mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.js create mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts delete mode 100644 packages/nextjs/test/integration/test/server/tracingWithSentryAPI.js create mode 100644 packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts delete mode 100644 packages/nextjs/test/integration/test/utils/common.js create mode 100644 packages/nextjs/test/integration/test/utils/common.ts delete mode 100644 packages/nextjs/test/integration/test/utils/server.js create mode 100644 packages/nextjs/test/integration/test/utils/server.ts diff --git a/packages/nextjs/jest.config.js b/packages/nextjs/jest.config.js index 4001570bcdb9..70485db447fa 100644 --- a/packages/nextjs/jest.config.js +++ b/packages/nextjs/jest.config.js @@ -4,5 +4,5 @@ module.exports = { ...baseConfig, // This prevents the build tests from running when unit tests run. (If they do, they fail, because the build being // tested hasn't necessarily run yet.) - testPathIgnorePatterns: ['/test/buildProcess/', '/test/integration/'], + testPathIgnorePatterns: ['/test/buildProcess/'], }; diff --git a/packages/nextjs/test/integration/jest.config.js b/packages/nextjs/test/integration/jest.config.js new file mode 100644 index 000000000000..4e888e0f14fb --- /dev/null +++ b/packages/nextjs/test/integration/jest.config.js @@ -0,0 +1,11 @@ +const baseConfig = require('../../jest.config.js'); + +module.exports = { + ...baseConfig, + testMatch: [`${__dirname}/test/server/**/*.test.ts`], + testPathIgnorePatterns: [`${__dirname}/test/client`], + detectOpenHandles: true, + forceExit: true, + testTimeout: 30000, + setupFilesAfterEnv: [`${__dirname}/jest.setup.js`], +}; diff --git a/packages/nextjs/test/integration/jest.setup.js b/packages/nextjs/test/integration/jest.setup.js new file mode 100644 index 000000000000..6360e753c4a5 --- /dev/null +++ b/packages/nextjs/test/integration/jest.setup.js @@ -0,0 +1,8 @@ +global.console = { + ...console, + log: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + // console.debug is available +}; diff --git a/packages/nextjs/test/integration/test/runner.js b/packages/nextjs/test/integration/test/runner.js deleted file mode 100644 index 0871284d32f4..000000000000 --- a/packages/nextjs/test/integration/test/runner.js +++ /dev/null @@ -1,130 +0,0 @@ -const fs = require('fs').promises; -const path = require('path'); - -const yargs = require('yargs/yargs'); - -const { colorize, verifyDir } = require('./utils/common'); -const { error, log } = console; - -const argv = yargs(process.argv.slice(2)) - .option('filter', { - type: 'string', - description: 'Filter scenarios based on filename (case-insensitive)', - }) - .option('silent', { - type: 'boolean', - description: 'Hide all stdout and console logs except test results', - conflicts: ['debug'], - }) - .option('debug', { - type: 'string', - description: 'Log intercepted requests and/or debug messages', - choices: ['', 'requests', 'logs'], // empty string will be equivalent to "logs" - conflicts: ['silent'], - }) - .option('depth', { - type: 'number', - description: 'Set the logging depth for intercepted requests (default = 4)', - }).argv; - -const runScenario = async (scenario, execute, env) => { - try { - await execute(require(scenario), { ...env }); - log(colorize(`✓ Scenario succeded: ${path.basename(scenario)}`, 'green')); - return true; - } catch (error) { - const scenarioFrames = error.stack.split('\n').filter(l => l.includes(scenario)); - - if (scenarioFrames.length === 0) { - log(error); - return false; - } - - /** - * Find first frame that matches our scenario filename and extract line number from it, eg.: - * - * at assertObjectMatches (/test/integration/test/utils.js:184:7) - * at module.exports.expectEvent (/test/integration/test/utils.js:122:10) - * at module.exports (/test/integration/test/client/errorGlobal.js:6:3) - */ - const line = scenarioFrames[0].match(/.+:(\d+):/)[1]; - log(colorize(`X Scenario failed: ${path.basename(scenario)} (line: ${line})`, 'red')); - log(error.message); - return false; - } -}; - -const runScenarios = async (scenarios, execute, env) => { - return Promise.all( - scenarios.map(scenario => { - return runScenario(scenario, execute, env); - }), - ); -}; - -module.exports.run = async ({ - setup = async () => {}, - teardown = async () => {}, - execute = async (scenario, env) => scenario(env), - scenariosDir, -}) => { - try { - await verifyDir(scenariosDir); - - let scenarios = await fs.readdir(scenariosDir); - if (argv.filter) { - scenarios = scenarios.filter(file => file.toLowerCase().includes(argv.filter.toLowerCase())); - } - scenarios = scenarios.map(s => path.resolve(scenariosDir, s)); - - if (scenarios.length === 0) { - log('No scenarios found'); - process.exit(0); - } else { - if (!argv.silent) { - scenarios.forEach(s => log(`⊙ Scenario found: ${path.basename(s)}`)); - } - } - - // Turn on the SDK's `debug` option or the logging of intercepted requests, or both - - // `yarn test:integration --debug` or - // `yarn test:integration --debug logs` or - // `yarn test:integration --debug logs --debug requests` - if (argv.debug === '' || argv.debug === 'logs' || (Array.isArray(argv.debug) && argv.debug.includes('logs'))) { - process.env.SDK_DEBUG = true; // will set `debug: true` in `Sentry.init() - } - - // `yarn test:integration --debug requests` or - // `yarn test:integration --debug logs --debug requests` - if (argv.debug === 'requests' || (Array.isArray(argv.debug) && argv.debug.includes('requests'))) { - process.env.LOG_REQUESTS = true; - } - - // Silence all the unnecessary server noise. We are capturing errors manualy anyway. - if (argv.silent) { - for (const level of ['log', 'warn', 'info', 'error']) { - console[level] = () => {}; - } - } - - const env = { - argv, - ...(await setup({ argv })), - }; - const results = await runScenarios(scenarios, execute, env); - const success = results.every(Boolean); - await teardown(env); - - if (success) { - log(colorize(`✓ All scenarios succeded`, 'green')); - process.exit(0); - } else { - log(colorize(`X Some scenarios failed`, 'red')); - process.exit(1); - } - } catch (e) { - error(e.message); - process.exit(1); - } -}; diff --git a/packages/nextjs/test/integration/test/server.js b/packages/nextjs/test/integration/test/server.js deleted file mode 100644 index 335f74808299..000000000000 --- a/packages/nextjs/test/integration/test/server.js +++ /dev/null @@ -1,18 +0,0 @@ -const path = require('path'); -const { run } = require('./runner'); -const { createNextServer, startServer } = require('./utils/common'); - -const setup = async () => { - const server = await createNextServer({ dev: false, dir: path.resolve(__dirname, '..') }); - return startServer(server); -}; - -const teardown = async ({ server }) => { - return new Promise(resolve => server.close(resolve)); -}; - -run({ - setup, - teardown, - scenariosDir: path.resolve(__dirname, './server'), -}); diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js deleted file mode 100644 index 3b25a405589a..000000000000 --- a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js +++ /dev/null @@ -1,55 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const unwrappedRoute = '/api/wrapApiHandlerWithSentry/unwrapped/cjsExport'; - const interceptedUnwrappedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - }, - transaction: `GET ${unwrappedRoute}`, - type: 'transaction', - request: { - url: `${urlBase}${unwrappedRoute}`, - }, - }, - argv, - 'unwrapped CJS route', - ); - const responseUnwrapped = await getAsync(`${urlBase}${unwrappedRoute}`); - assert.equal(responseUnwrapped, '{"success":true}'); - - const wrappedRoute = '/api/wrapApiHandlerWithSentry/wrapped/cjsExport'; - const interceptedWrappedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - }, - transaction: `GET ${wrappedRoute}`, - type: 'transaction', - request: { - url: `${urlBase}${wrappedRoute}`, - }, - }, - argv, - 'wrapped CJS route', - ); - const responseWrapped = await getAsync(`${urlBase}${wrappedRoute}`); - assert.equal(responseWrapped, '{"success":true}'); - - await sleep(250); - - assert.ok(interceptedUnwrappedRequest.isDone(), 'Did not intercept unwrapped request'); - assert.ok(interceptedWrappedRequest.isDone(), 'Did not intercept wrapped request'); -}; diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts new file mode 100644 index 000000000000..bd8769db4afa --- /dev/null +++ b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts @@ -0,0 +1,69 @@ +import { NextTestEnv } from '../utils/server'; + +describe('CommonJS API Endpoints', () => { + it('should not intercept unwrapped request', async () => { + const env = await NextTestEnv.init(); + const unwrappedRoute = '/api/wrapApiHandlerWithSentry/unwrapped/cjsExport'; + const url = `${env.url}${unwrappedRoute}`; + + const unwrappedEnvelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + endServer: false, + }); + + expect(unwrappedEnvelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + tags: { 'http.status_code': '200' }, + }, + }, + transaction: `GET ${unwrappedRoute}`, + type: 'transaction', + request: { + url, + }, + }); + + const response = await env.getAPIResponse(url); + + expect(response).toMatchObject({ + success: true, + }); + }); + + it('should intercept wrapped request', async () => { + const env = await NextTestEnv.init(); + const wrappedRoute = '/api/wrapApiHandlerWithSentry/wrapped/cjsExport'; + const url = `${env.url}${wrappedRoute}`; + + const wrappedEnvelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + endServer: false, + }); + + expect(wrappedEnvelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + tags: { 'http.status_code': '200' }, + }, + }, + transaction: `GET ${wrappedRoute}`, + type: 'transaction', + request: { + url, + }, + }); + + const response = await env.getAPIResponse(url); + + expect(response).toMatchObject({ + success: true, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.js b/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.js deleted file mode 100644 index cd3af491bb32..000000000000 --- a/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.js +++ /dev/null @@ -1,14 +0,0 @@ -const assert = require('assert'); -const { getAsync } = require('../utils/server'); - -// This test asserts that our wrapping of `res.end` doesn't break API routes on Vercel if people call `res.json` or -// `res.send` multiple times in one request handler. -// https://github.com/getsentry/sentry-javascript/issues/6670 -module.exports = async ({ url: urlBase }) => { - if (process.env.NODE_MAJOR === '10') { - console.log('not running doubleEndMethodOnVercel test on Node 10'); - return; - } - const response = await getAsync(`${urlBase}/api/doubleEndMethodOnVercel`); - assert.equal(response, '{"success":true}'); -}; diff --git a/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts b/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts new file mode 100644 index 000000000000..d17631bd4fa5 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts @@ -0,0 +1,18 @@ +import { NextTestEnv } from '../utils/server'; + +// This test asserts that our wrapping of `res.end` doesn't break API routes on Vercel if people call `res.json` or +// `res.send` multiple times in one request handler. +// https://github.com/getsentry/sentry-javascript/issues/6670 +it.skip('should not break API routes on Vercel if people call res.json or res.send multiple times in one request handler', async () => { + if (process.env.NODE_MAJOR === '10') { + console.log('not running doubleEndMethodOnVercel test on Node 10'); + return; + } + const env = await NextTestEnv.init(); + const url = `${env.url}/api/doubleEndMethodOnVercel`; + const response = await env.getAPIResponse(url); + + expect(response).toMatchObject({ + success: true, + }); +}); diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.js b/packages/nextjs/test/integration/test/server/errorApiEndpoint.js deleted file mode 100644 index 00e3ab128d81..000000000000 --- a/packages/nextjs/test/integration/test/server/errorApiEndpoint.js +++ /dev/null @@ -1,56 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/api/error`; - - const capturedErrorRequest = interceptEventRequest( - { - exception: { - values: [ - { - type: 'Error', - value: 'API Error', - }, - ], - }, - tags: { - runtime: 'node', - }, - request: { - url, - method: 'GET', - }, - transaction: 'GET /api/error', - }, - argv, - 'errorApiEndpoint', - ); - - const capturedTransactionRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'internal_error', - tags: { 'http.status_code': '500' }, - }, - }, - transaction: 'GET /api/error', - type: 'transaction', - request: { - url, - }, - }, - argv, - 'errorApiEndpoint', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedErrorRequest.isDone(), 'Did not intercept expected error request'); - assert.ok(capturedTransactionRequest.isDone(), 'Did not intercept expected transaction request'); -}; diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts new file mode 100644 index 000000000000..c2664b66f149 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts @@ -0,0 +1,59 @@ +import { NextTestEnv } from '../utils/server'; + +jest.spyOn(console, 'error').mockImplementation(); + +describe('Error API Endpoints', () => { + it('should capture an error event', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/error`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'event', + }); + + expect(envelope[2]).toMatchObject({ + exception: { + values: [ + { + type: 'Error', + value: 'API Error', + }, + ], + }, + tags: { + runtime: 'node', + }, + request: { + url, + method: 'GET', + }, + transaction: 'GET /api/error', + }); + }); + + it('should capture an erroneous transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/error`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'internal_error', + tags: { 'http.status_code': '500' }, + }, + }, + transaction: 'GET /api/error', + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.js b/packages/nextjs/test/integration/test/server/errorServerSideProps.js deleted file mode 100644 index 283e99a51363..000000000000 --- a/packages/nextjs/test/integration/test/server/errorServerSideProps.js +++ /dev/null @@ -1,59 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/withErrorServerSideProps`; - - const capturedRequest = interceptEventRequest( - { - exception: { - values: [ - { - type: 'Error', - value: 'ServerSideProps Error', - }, - ], - }, - tags: { - runtime: 'node', - }, - request: { - url, - method: 'GET', - }, - }, - argv, - 'errorServerSideProps', - ); - - const capturedTransactionRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'internal_error', - }, - }, - transaction: '/withErrorServerSideProps', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'errorServerSideProps', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); - assert.ok(capturedTransactionRequest.isDone(), 'Did not intercept expected transaction request'); -}; diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts new file mode 100644 index 000000000000..cbe11e6957e1 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts @@ -0,0 +1,60 @@ +import { NextTestEnv } from '../utils/server'; + +describe('Error Server-side Props', () => { + it('should capture an error event', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/withErrorServerSideProps`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'event', + }); + + expect(envelope[2]).toMatchObject({ + exception: { + values: [ + { + type: 'Error', + value: 'ServerSideProps Error', + }, + ], + }, + tags: { + runtime: 'node', + }, + request: { + url, + method: 'GET', + }, + }); + }); + + it('should capture an erroneous transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/withErrorServerSideProps`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'internal_error', + }, + }, + transaction: '/withErrorServerSideProps', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/excludedApiEndpoints.js b/packages/nextjs/test/integration/test/server/excludedApiEndpoints.js deleted file mode 100644 index db49bbbc57cf..000000000000 --- a/packages/nextjs/test/integration/test/server/excludedApiEndpoints.js +++ /dev/null @@ -1,67 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const regExpUrl = `${urlBase}/api/excludedEndpoints/excludedWithRegExp`; - const stringUrl = `${urlBase}/api/excludedEndpoints/excludedWithString`; - - const capturedRegExpErrorRequest = interceptEventRequest( - { - exception: { - values: [ - { - type: 'Error', - value: 'API Error', - }, - ], - }, - tags: { - runtime: 'node', - }, - request: { - url: regExpUrl, - method: 'GET', - }, - transaction: 'GET /api/excludedEndpoints/excludedWithRegExp', - }, - argv, - 'excluded API endpoint via RegExp', - ); - - const capturedStringErrorRequest = interceptEventRequest( - { - exception: { - values: [ - { - type: 'Error', - value: 'API Error', - }, - ], - }, - tags: { - runtime: 'node', - }, - request: { - url: regExpUrl, - method: 'GET', - }, - transaction: 'GET /api/excludedEndpoints/excludedWithString', - }, - argv, - 'excluded API endpoint via String', - ); - - await Promise.all([getAsync(regExpUrl), getAsync(stringUrl)]); - await sleep(250); - - assert.ok( - !capturedRegExpErrorRequest.isDone(), - 'Did intercept error request even though route should be excluded (RegExp)', - ); - assert.ok( - !capturedStringErrorRequest.isDone(), - 'Did intercept error request even though route should be excluded (String)', - ); -}; diff --git a/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts b/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts new file mode 100644 index 000000000000..d6c60c8886f8 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts @@ -0,0 +1,29 @@ +import { NextTestEnv } from '../utils/server'; + +describe('Excluded API Endpoints', () => { + it('Should exclude API endpoint via RegExp', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/excludedEndpoints/excludedWithRegExp`; + + const count = await env.countEnvelopes({ + url, + envelopeType: 'event', + timeout: 3000, + }); + + expect(count).toBe(0); + }); + + it('Should exclude API endpoint via string', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/excludedEndpoints/excludedWithString`; + + const count = await env.countEnvelopes({ + url, + envelopeType: 'event', + timeout: 3000, + }); + + expect(count).toBe(0); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracing200.js b/packages/nextjs/test/integration/test/server/tracing200.js deleted file mode 100644 index a8df030f9468..000000000000 --- a/packages/nextjs/test/integration/test/server/tracing200.js +++ /dev/null @@ -1,37 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/api/users`; - - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - }, - transaction: 'GET /api/users', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracing200', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracing200.test.ts b/packages/nextjs/test/integration/test/server/tracing200.test.ts new file mode 100644 index 000000000000..0288c7172623 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracing200.test.ts @@ -0,0 +1,33 @@ +import { NextTestEnv } from '../utils/server'; + +describe('Tracing 200', () => { + it('should capture a transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/users`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + tags: { 'http.status_code': '200' }, + }, + }, + transaction: 'GET /api/users', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracing500.js b/packages/nextjs/test/integration/test/server/tracing500.js deleted file mode 100644 index 1e5ed467a83d..000000000000 --- a/packages/nextjs/test/integration/test/server/tracing500.js +++ /dev/null @@ -1,36 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/api/broken`; - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'internal_error', - tags: { 'http.status_code': '500' }, - }, - }, - transaction: 'GET /api/broken', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracing500', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracing500.test.ts b/packages/nextjs/test/integration/test/server/tracing500.test.ts new file mode 100644 index 000000000000..38067eecb4d9 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracing500.test.ts @@ -0,0 +1,33 @@ +import { NextTestEnv } from '../utils/server'; + +describe('Tracing 500', () => { + it('should capture an erroneous transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/broken`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'internal_error', + tags: { 'http.status_code': '500' }, + }, + }, + transaction: 'GET /api/broken', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.js b/packages/nextjs/test/integration/test/server/tracingHttp.js deleted file mode 100644 index 00cc2d1e3129..000000000000 --- a/packages/nextjs/test/integration/test/server/tracingHttp.js +++ /dev/null @@ -1,53 +0,0 @@ -const assert = require('assert'); - -const nock = require('nock'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/api/http`; - - // this intercepts the outgoing request made by the route handler (which it makes in order to test span creation) - nock('http://example.com').get('/').reply(200, 'ok'); - - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - }, - spans: [ - { - description: 'GET http://example.com/', - op: 'http.client', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - ], - transaction: 'GET /api/http', - transaction_info: { - source: 'route', - changes: [], - propagations: 1, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracingHttp', - ); - - // The `true` causes `getAsync` to rewrap `http.get` in next 12, since it will have been overwritten by the import of - // `nock` above. See https://github.com/getsentry/sentry-javascript/pull/4619. - // TODO: see note in `getAsync` about removing the boolean - await getAsync(url, true); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts new file mode 100644 index 000000000000..d8eb7089a09f --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts @@ -0,0 +1,45 @@ +import { NextTestEnv } from '../utils/server'; +import nock from 'nock'; + +describe('Tracing HTTP', () => { + it('should capture a transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/api/http`; + + // this intercepts the outgoing request made by the route handler (which it makes in order to test span creation) + nock('http://example.com').get('/').reply(200, 'ok'); + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + tags: { 'http.status_code': '200' }, + }, + }, + spans: [ + { + description: 'GET http://example.com/', + op: 'http.client', + status: 'ok', + tags: { 'http.status_code': '200' }, + }, + ], + transaction: 'GET /api/http', + transaction_info: { + source: 'route', + changes: [], + propagations: 1, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js deleted file mode 100644 index d4e5f63c8f56..000000000000 --- a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js +++ /dev/null @@ -1,36 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/239/withInitialProps`; - - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - }, - }, - transaction: '/[id]/withInitialProps', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracingGetInitialProps', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts new file mode 100644 index 000000000000..7ae9b8b0404e --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts @@ -0,0 +1,32 @@ +import { NextTestEnv } from '../utils/server'; + +describe('getInitialProps', () => { + it('should capture a transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/239/withInitialProps`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + }, + }, + transaction: '/[id]/withInitialProps', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js deleted file mode 100644 index 6b8d7b1fa65a..000000000000 --- a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js +++ /dev/null @@ -1,36 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/193/withServerSideProps`; - - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - }, - }, - transaction: '/[id]/withServerSideProps', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracingServerGetServerSideProps', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts new file mode 100644 index 000000000000..e73c0124d9cf --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts @@ -0,0 +1,32 @@ +import { NextTestEnv } from '../utils/server'; + +describe('getServerSideProps', () => { + it('should capture a transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/193/withServerSideProps`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + }, + }, + transaction: '/[id]/withServerSideProps', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.js b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.js deleted file mode 100644 index a2ca6a5142d1..000000000000 --- a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.js +++ /dev/null @@ -1,36 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/customPageExtension`; - - const capturedRequest = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - }, - }, - transaction: '/customPageExtension', - transaction_info: { - source: 'route', - changes: [], - propagations: 0, - }, - type: 'transaction', - request: { - url, - }, - }, - argv, - 'tracingServerGetServerSidePropsCustomPageExtension', - ); - - await getAsync(url); - await sleep(250); - - assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); -}; diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts new file mode 100644 index 000000000000..a138844cb9be --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts @@ -0,0 +1,32 @@ +import { NextTestEnv } from '../utils/server'; + +describe('tracingServerGetServerSidePropsCustomPageExtension', () => { + it('should capture a transaction', async () => { + const env = await NextTestEnv.init(); + const url = `${env.url}/customPageExtension`; + + const envelope = await env.getEnvelopeRequest({ + url, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + }, + }, + transaction: '/customPageExtension', + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.js b/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.js deleted file mode 100644 index a630e5b97e00..000000000000 --- a/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.js +++ /dev/null @@ -1,69 +0,0 @@ -const assert = require('assert'); - -const { sleep } = require('../utils/common'); -const { getAsync, interceptTracingRequest } = require('../utils/server'); - -module.exports = async ({ url: urlBase, argv }) => { - const urls = { - // testName: [url, route] - unwrappedNoParamURL: [ - `/api/wrapApiHandlerWithSentry/unwrapped/noParams`, - '/api/wrapApiHandlerWithSentry/unwrapped/noParams', - ], - unwrappedDynamicURL: [ - `/api/wrapApiHandlerWithSentry/unwrapped/dog`, - '/api/wrapApiHandlerWithSentry/unwrapped/[animal]', - ], - unwrappedCatchAllURL: [ - `/api/wrapApiHandlerWithSentry/unwrapped/dog/facts`, - '/api/wrapApiHandlerWithSentry/unwrapped/[...pathParts]', - ], - wrappedNoParamURL: [ - `/api/wrapApiHandlerWithSentry/wrapped/noParams`, - '/api/wrapApiHandlerWithSentry/wrapped/noParams', - ], - wrappedDynamicURL: [`/api/wrapApiHandlerWithSentry/wrapped/dog`, '/api/wrapApiHandlerWithSentry/wrapped/[animal]'], - wrappedCatchAllURL: [ - `/api/wrapApiHandlerWithSentry/wrapped/dog/facts`, - '/api/wrapApiHandlerWithSentry/wrapped/[...pathParts]', - ], - }; - - const interceptedRequests = {}; - - Object.entries(urls).forEach(([testName, [url, route]]) => { - interceptedRequests[testName] = interceptTracingRequest( - { - contexts: { - trace: { - op: 'http.server', - status: 'ok', - tags: { 'http.status_code': '200' }, - }, - }, - transaction: `GET ${route}`, - type: 'transaction', - request: { - url: `${urlBase}${url}`, - }, - }, - argv, - testName, - ); - }); - - // Wait until all requests have completed - await Promise.all(Object.values(urls).map(([url]) => getAsync(`${urlBase}${url}`))); - - await sleep(250); - - const failingTests = Object.entries(interceptedRequests).reduce( - (failures, [testName, request]) => (!request.isDone() ? failures.concat(testName) : failures), - [], - ); - - assert.ok( - failingTests.length === 0, - `Did not intercept transaction request for the following tests: ${failingTests.join(', ')}.`, - ); -}; diff --git a/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts b/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts new file mode 100644 index 000000000000..b2a11c9ebf57 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts @@ -0,0 +1,66 @@ +import { NextTestEnv } from '../utils/server'; + +const cases = [ + { + name: 'unwrappedNoParamURL', + url: `/api/wrapApiHandlerWithSentry/unwrapped/noParams`, + transactionName: '/api/wrapApiHandlerWithSentry/unwrapped/noParams', + }, + { + name: 'unwrappedDynamicURL', + url: `/api/wrapApiHandlerWithSentry/unwrapped/dog`, + transactionName: '/api/wrapApiHandlerWithSentry/unwrapped/[animal]', + }, + { + name: 'unwrappedCatchAllURL', + url: `/api/wrapApiHandlerWithSentry/unwrapped/dog/facts`, + transactionName: '/api/wrapApiHandlerWithSentry/unwrapped/[...pathParts]', + }, + { + name: 'wrappedNoParamURL', + url: `/api/wrapApiHandlerWithSentry/wrapped/noParams`, + transactionName: '/api/wrapApiHandlerWithSentry/wrapped/noParams', + }, + { + name: 'wrappedDynamicURL', + url: `/api/wrapApiHandlerWithSentry/wrapped/dog`, + transactionName: '/api/wrapApiHandlerWithSentry/wrapped/[animal]', + }, + { + name: 'wrappedCatchAllURL', + url: `/api/wrapApiHandlerWithSentry/wrapped/dog/facts`, + transactionName: '/api/wrapApiHandlerWithSentry/wrapped/[...pathParts]', + }, +]; + +describe('getServerSideProps', () => { + it.each(cases)(`should capture a transaction for %s`, async ({ url, transactionName }) => { + const env = await NextTestEnv.init(); + + const fullUrl = `${env.url}${url}`; + + const envelope = await env.getEnvelopeRequest({ + url: fullUrl, + envelopeType: 'transaction', + }); + + expect(envelope[2]).toMatchObject({ + contexts: { + trace: { + op: 'http.server', + status: 'ok', + }, + }, + transaction: `GET ${transactionName}`, + transaction_info: { + source: 'route', + changes: [], + propagations: 0, + }, + type: 'transaction', + request: { + url: fullUrl, + }, + }); + }); +}); diff --git a/packages/nextjs/test/integration/test/utils/common.js b/packages/nextjs/test/integration/test/utils/common.js deleted file mode 100644 index f8453ce075a9..000000000000 --- a/packages/nextjs/test/integration/test/utils/common.js +++ /dev/null @@ -1,91 +0,0 @@ -const { createServer } = require('http'); -const { parse } = require('url'); -const { stat } = require('fs').promises; -const next = require('next'); - -const createNextServer = async config => { - const app = next(config); - const handle = app.getRequestHandler(); - await app.prepare(); - return createServer((req, res) => handle(req, res, parse(req.url, true))); -}; - -const startServer = async (server, env) => { - return new Promise((resolve, reject) => { - server.listen(0, err => { - if (err) { - reject(err); - } else { - const url = `http://localhost:${server.address().port}`; - resolve({ server, url, ...env }); - } - }); - }); -}; - -const parseEnvelope = body => { - const [envelopeHeaderString, itemHeaderString, itemString] = body.split('\n'); - - return { - envelopeHeader: JSON.parse(envelopeHeaderString), - itemHeader: JSON.parse(itemHeaderString), - item: JSON.parse(itemString), - }; -}; - -const logIf = (condition, message, input, depth = 4) => { - if (condition) { - console.log(message); - if (input) { - console.dir(input, { depth, colors: true }); - } - } -}; - -const COLOR_RESET = '\x1b[0m'; -const COLORS = { - green: '\x1b[32m', - red: '\x1b[31m', -}; - -const colorize = (str, color) => { - if (!(color in COLORS)) { - throw new Error(`Unknown color. Available colors: ${Object.keys(COLORS).join(', ')}`); - } - - return `${COLORS[color]}${str}${COLOR_RESET}`; -}; - -const verifyDir = async path => { - try { - if (!(await stat(path)).isDirectory()) { - throw new Error(`Invalid scenariosDir: ${path} is not a directory`); - } - } catch (e) { - if (e.code === 'ENOENT') { - throw new Error(`Invalid scenariosDir: ${path} does not exist`); - } - throw e; - } -}; - -const sleep = duration => { - return new Promise(resolve => setTimeout(() => resolve(), duration)); -}; - -const waitForAll = actions => { - return Promise.all(actions).catch(() => { - throw new Error('Failed to await on all requested actions'); - }); -}; - -module.exports = { - colorize, - createNextServer, - logIf, - parseEnvelope, - sleep, - startServer, - verifyDir, - waitForAll, -}; diff --git a/packages/nextjs/test/integration/test/utils/common.ts b/packages/nextjs/test/integration/test/utils/common.ts new file mode 100644 index 000000000000..e101c8aed5f6 --- /dev/null +++ b/packages/nextjs/test/integration/test/utils/common.ts @@ -0,0 +1,65 @@ +import { createServer, Server } from 'http'; +import { parse } from 'url'; +import { promises, PathLike } from 'fs'; +import next from 'next'; +import { NextServer } from 'next/dist/server/next'; + +const { stat } = promises; + +// Type not exported from NextJS +type NextServerConstructor = ConstructorParameters[0]; + +export const createNextServer = async (config: NextServerConstructor) => { + const app = next(config); + const handle = app.getRequestHandler(); + await app.prepare(); + + return createServer((req, res) => { + const { url } = req; + + if (!url) { + throw new Error('No url'); + } + + handle(req, res, parse(url, true)); + }); +}; + +export const startServer = async (server: Server, port: string | number) => { + return new Promise(resolve => { + server.listen(port || 0, () => { + const url = `http://localhost:${port}`; + resolve({ server, url }); + }); + }); +}; + +export const COLOR_RESET = '\x1b[0m'; +export const COLORS = { + green: '\x1b[32m', + red: '\x1b[31m', +}; + +export const colorize = (str: string, color: 'green' | 'red') => { + if (!(color in COLORS)) { + throw new Error(`Unknown color. Available colors: ${Object.keys(COLORS).join(', ')}`); + } + + return `${COLORS[color]}${str}${COLOR_RESET}`; +}; + +export const verifyDir = async (path: PathLike) => { + if (!(await stat(path)).isDirectory()) { + throw new Error(`Invalid scenariosDir: ${path} is not a directory`); + } +}; + +export const sleep = (duration: number) => { + return new Promise(resolve => setTimeout(() => resolve(), duration)); +}; + +export const waitForAll = (actions: any[]) => { + return Promise.all(actions).catch(() => { + throw new Error('Failed to await on all requested actions'); + }); +}; diff --git a/packages/nextjs/test/integration/test/utils/server.js b/packages/nextjs/test/integration/test/utils/server.js deleted file mode 100644 index d29a4961d2e8..000000000000 --- a/packages/nextjs/test/integration/test/utils/server.js +++ /dev/null @@ -1,183 +0,0 @@ -// @ts-check -const { get } = require('http'); -const nock = require('nock'); -const nodeSDK = require('@sentry/node'); -const { logIf, parseEnvelope } = require('./common'); - -Error.stackTraceLimit = Infinity; - -const getAsync = (url, rewrap = false) => { - // Depending on what version of Nextjs we're testing, the wrapping which happens in the `Http` integration may have - // happened too early and gotten overwritten by `nock`. This redoes the wrapping if so. - // - // TODO: This works but is pretty hacky in that it has the potential to wrap things multiple times, more even than the - // double-wrapping which is discussed at length in the comment in `ensureWrappedGet` below, which is why we need - // `rewrap`. Once we fix `fill` to not wrap things twice, we should be able to take this out and just always call - // `ensureWrappedGet`. - const wrappedGet = rewrap ? ensureWrappedGet(get, url) : get; - - return new Promise((resolve, reject) => { - wrappedGet(url, res => { - res.setEncoding('utf8'); - let rawData = ''; - res.on('data', chunk => { - rawData += chunk; - }); - res.on('end', () => { - try { - resolve(rawData); - } catch (e) { - reject(e); - } - }); - }); - }); -}; - -const interceptEventRequest = (expectedEvent, argv, testName = '') => { - return nock('https://dsn.ingest.sentry.io') - .post('/api/1337/envelope/', body => { - const { envelopeHeader, itemHeader, item } = parseEnvelope(body); - logIf( - process.env.LOG_REQUESTS, - '\nIntercepted Event' + (testName.length ? ` (from test \`${testName}\`)` : ''), - { envelopeHeader, itemHeader, item }, - argv.depth, - ); - return itemHeader.type === 'event' && objectMatches(item, expectedEvent); - }) - .query(true) // accept any query params - used for sentry_key param used by the envelope endpoint - .reply(200); -}; - -const interceptSessionRequest = (expectedItem, argv, testName = '') => { - return nock('https://dsn.ingest.sentry.io') - .post('/api/1337/envelope/', body => { - const { envelopeHeader, itemHeader, item } = parseEnvelope(body); - logIf( - process.env.LOG_REQUESTS, - '\nIntercepted Session' + (testName.length ? ` (from test \`${testName}\`)` : ''), - { envelopeHeader, itemHeader, item }, - argv.depth, - ); - return itemHeader.type === 'session' && objectMatches(item, expectedItem); - }) - .query(true) // accept any query params - used for sentry_key param used by the envelope endpoint - .reply(200); -}; - -const interceptTracingRequest = (expectedItem, argv, testName = '') => { - return nock('https://dsn.ingest.sentry.io') - .post('/api/1337/envelope/', body => { - const { envelopeHeader, itemHeader, item } = parseEnvelope(body); - logIf( - process.env.LOG_REQUESTS, - '\nIntercepted Transaction' + (testName.length ? ` (from test \`${testName}\`)` : ''), - { envelopeHeader, itemHeader, item }, - argv.depth, - ); - return itemHeader.type === 'transaction' && objectMatches(item, expectedItem); - }) - .query(true) // accept any query params - used for sentry_key param used by the envelope endpoint - .reply(200); -}; - -/** - * Recursively checks that every path/value pair in `expected` matches that in `actual` (but not vice-versa). - * - * Only works for JSONifiable data. - */ -const objectMatches = (actual, expected) => { - // each will output either '[object Object]' or '[object ]' - if (Object.prototype.toString.call(actual) !== Object.prototype.toString.call(expected)) { - return false; - } - - for (const key in expected) { - const expectedValue = expected[key]; - const actualValue = actual[key]; - - // recurse - if (Object.prototype.toString.call(expectedValue) === '[object Object]' || Array.isArray(expectedValue)) { - if (!objectMatches(actualValue, expectedValue)) { - return false; - } - } - // base case - else { - if (actualValue !== expectedValue) { - return false; - } - } - } - - return true; -}; - -/** - * Rewrap `http.get` if the wrapped version has been overridden by `nock`. - * - * This is only relevant for Nextjs >= 12.1, which changed when `_app` is initialized, which in turn changed the order - * in which our SDK and `nock` wrap `http.get`. See https://github.com/getsentry/sentry-javascript/pull/4619. - * - * TODO: We'll have to do this for `ClientRequest` also if we decide to start wrapping that. - * TODO: Can we fix the wrapping-things-twice problem discussed in the comment below? - */ -function ensureWrappedGet(importedGet, url) { - // we always test against the latest minor for any given Nextjs major version, so if we're testing Next 12, it's - // definitely at least 12.1, making this check against the major version sufficient - if (Number(process.env.NEXTJS_VERSION) < 12) { - return importedGet; - } - - // As of Next 12.1, creating a `NextServer` instance (which we do immediately upon starting this test runner) loads - // `_app`, which has the effect of initializing the SDK. So, unless something's gone wrong, we should always be able - // to find the integration - const hub = nodeSDK.getCurrentHub(); - const client = hub.getClient(); - - if (!client) { - console.warn(`Warning: Sentry SDK not set up at \`NextServer\` initialization. Request URL: ${url}`); - return importedGet; - } - - const httpIntegration = client.getIntegration(nodeSDK.Integrations.Http); - - // This rewraps `http.get` and `http.request`, which, at this point, look like `nockWrapper(sentryWrapper(get))` and - // `nockWrapper(sentryWrapper(request))`. By the time we're done with this function, they'll look like - // `sentryWrapper(nockWrapper(sentryWrapper(get)))` and `sentryWrapper(nockWrapper(sentryWrapper(request)))`, - // respectively. Though this seems less than ideal, we don't have to worry about our instrumentation being - // (meaningfully) called twice because: - // - // 1) As long as we set up a `nock` interceptor for any outgoing http request, `nock`'s wrapper will call a replacement - // function for that request rather than call the function it's wrapping (in other words, it will look more like - // `sentryWrapper(nockWrapper(getSubstitute))` than `sentryWrapper(nockWrapper(sentryWrapper(get)))`), which means - // our code is only called once. - // 2) In cases where we don't set up an interceptor (such as for the `wrappedGet` call in `getAsync` above), it's true - // that we can end up with `sentryWrapper(nockWrapper(sentryWrapper(get)))`, meaning our wrapper code will run - // twice. For now that's okay because in those cases we're not in the middle of a transactoin and therefore - // the two wrappers' respective attempts to start spans will both no-op. - // - // TL; DR - if the double-wrapping means you're seeing two spans where you really only want one, set up a nock - // interceptor for the request. - // - // TODO: add in a "don't do this twice" check (in `fill`, maybe moved from `wrap`), so that we don't wrap the outer - // wrapper with a third wrapper - if (httpIntegration) { - httpIntegration.setupOnce( - () => undefined, - () => hub, - ); - } - - // now that we've rewrapped it, grab the correct version of `get` for use in our tests - const httpModule = require('http'); - return httpModule.get; -} - -module.exports = { - getAsync, - interceptEventRequest, - interceptSessionRequest, - interceptTracingRequest, -}; diff --git a/packages/nextjs/test/integration/test/utils/server.ts b/packages/nextjs/test/integration/test/utils/server.ts new file mode 100644 index 000000000000..8c7fb4fcfb14 --- /dev/null +++ b/packages/nextjs/test/integration/test/utils/server.ts @@ -0,0 +1,119 @@ +import { getPortPromise } from 'portfinder'; +import { TestEnv } from '../../../../../node-integration-tests/utils'; +import * as http from 'http'; +import * as path from 'path'; +import * as nodeSDK from '@sentry/node'; +import { createNextServer, startServer } from '../utils/common'; +import { get } from 'http'; + +type HttpGet = ( + options: http.RequestOptions | string | URL, + callback?: (res: http.IncomingMessage) => void, +) => http.ClientRequest; + +export class NextTestEnv extends TestEnv { + private constructor(public readonly server: http.Server, public readonly url: string) { + super(server, url); + } + + public static async init(): Promise { + const port = await getPortPromise(); + const server = await createNextServer({ + dev: false, + dir: path.resolve(__dirname, '../..'), + }); + + await startServer(server, port); + + return new NextTestEnv(server, `http://localhost:${port}`); + } + + /** + * Rewrap `http.get` if the wrapped version has been overridden by `nock`. + * + * This is only relevant for Nextjs >= 12.1, which changed when `_app` is initialized, which in turn changed the order + * in which our SDK and `nock` wrap `http.get`. See https://github.com/getsentry/sentry-javascript/pull/4619. + * + * TODO: We'll have to do this for `ClientRequest` also if we decide to start wrapping that. + * TODO: Can we fix the wrapping-things-twice problem discussed in the comment below? + */ + public ensureWrappedGet(importedGet: HttpGet, url: string) { + // we always test against the latest minor for any given Nextjs major version, so if we're testing Next 12, it's + // definitely at least 12.1, making this check against the major version sufficient + if (Number(process.env.NEXTJS_VERSION) < 12) { + return importedGet; + } + + // As of Next 12.1, creating a `NextServer` instance (which we do immediately upon starting this test runner) loads + // `_app`, which has the effect of initializing the SDK. So, unless something's gone wrong, we should always be able + // to find the integration + const hub = nodeSDK.getCurrentHub(); + const client = hub.getClient(); + + if (!client) { + console.warn(`Warning: Sentry SDK not set up at \`NextServer\` initialization. Request URL: ${url}`); + return importedGet; + } + + const httpIntegration = client.getIntegration(nodeSDK.Integrations.Http); + + // This rewraps `http.get` and `http.request`, which, at this point, look like `nockWrapper(sentryWrapper(get))` and + // `nockWrapper(sentryWrapper(request))`. By the time we're done with this function, they'll look like + // `sentryWrapper(nockWrapper(sentryWrapper(get)))` and `sentryWrapper(nockWrapper(sentryWrapper(request)))`, + // respectively. Though this seems less than ideal, we don't have to worry about our instrumentation being + // (meaningfully) called twice because: + // + // 1) As long as we set up a `nock` interceptor for any outgoing http request, `nock`'s wrapper will call a replacement + // function for that request rather than call the function it's wrapping (in other words, it will look more like + // `sentryWrapper(nockWrapper(getSubstitute))` than `sentryWrapper(nockWrapper(sentryWrapper(get)))`), which means + // our code is only called once. + // 2) In cases where we don't set up an interceptor (such as for the `wrappedGet` call in `getAsync` above), it's true + // that we can end up with `sentryWrapper(nockWrapper(sentryWrapper(get)))`, meaning our wrapper code will run + // twice. For now that's okay because in those cases we're not in the middle of a transactoin and therefore + // the two wrappers' respective attempts to start spans will both no-op. + // + // TL; DR - if the double-wrapping means you're seeing two spans where you really only want one, set up a nock + // interceptor for the request. + // + // TODO: add in a "don't do this twice" check (in `fill`, maybe moved from `wrap`), so that we don't wrap the outer + // wrapper with a third wrapper + if (httpIntegration) { + httpIntegration.setupOnce( + () => undefined, + () => hub, + ); + } + + // now that we've rewrapped it, grab the correct version of `get` for use in our tests + const httpModule = require('http'); + return httpModule.get; + } + + public async getAsync(url: string, rewrap = false) { + // Depending on what version of Nextjs we're testing, the wrapping which happens in the `Http` integration may have + // happened too early and gotten overwritten by `nock`. This redoes the wrapping if so. + // + // TODO: This works but is pretty hacky in that it has the potential to wrap things multiple times, more even than the + // double-wrapping which is discussed at length in the comment in `ensureWrappedGet` below, which is why we need + // `rewrap`. Once we fix `fill` to not wrap things twice, we should be able to take this out and just always call + // `ensureWrappedGet`. + const wrappedGet = rewrap ? this.ensureWrappedGet(get, url) : get; + + return new Promise((resolve, reject) => { + wrappedGet(url, (res: http.IncomingMessage) => { + res.setEncoding('utf8'); + let rawData = ''; + res.on('data', chunk => { + rawData += chunk; + }); + res.on('end', () => { + try { + resolve(rawData); + } catch (e) { + reject(e); + } + }); + }); + }); + } +} diff --git a/packages/nextjs/test/integration/tsconfig.json b/packages/nextjs/test/integration/tsconfig.json index 5418171e8e66..b18a128f2711 100644 --- a/packages/nextjs/test/integration/tsconfig.json +++ b/packages/nextjs/test/integration/tsconfig.json @@ -6,7 +6,10 @@ "forceConsistentCasingInFileNames": true, "isolatedModules": true, "jsx": "preserve", - "lib": ["dom", "es2017"], + "lib": [ + "dom", + "es2017" + ], "module": "esnext", "moduleResolution": "node", "noEmit": true, @@ -21,8 +24,16 @@ { "name": "next" } - ] + ], + "incremental": true }, - "exclude": ["node_modules"], - "include": ["**/*.ts", "**/*.tsx", "../../playwright.config.ts", ".next/types/**/*.ts"] + "exclude": [ + "node_modules" + ], + "include": [ + "**/*.ts", + "**/*.tsx", + "../../playwright.config.ts", + ".next/types/**/*.ts" + ] } diff --git a/packages/nextjs/test/run-integration-tests.sh b/packages/nextjs/test/run-integration-tests.sh index 6ffce457c29a..f3e167bb4040 100755 --- a/packages/nextjs/test/run-integration-tests.sh +++ b/packages/nextjs/test/run-integration-tests.sh @@ -138,7 +138,7 @@ for NEXTJS_VERSION in 10 11 12 13; do EXIT_CODE=0 echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running server tests with options: $args" - node test/server.js $args || EXIT_CODE=$? + (cd .. && yarn test:integration:server) || EXIT_CODE=$? if [ $EXIT_CODE -eq 0 ]; then echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests passed" diff --git a/packages/nextjs/tsconfig.json b/packages/nextjs/tsconfig.json index bf45a09f2d71..ff9a3c1f98f6 100644 --- a/packages/nextjs/tsconfig.json +++ b/packages/nextjs/tsconfig.json @@ -4,6 +4,7 @@ "include": ["src/**/*"], "compilerOptions": { + "esModuleInterop": true // package-specific options } } diff --git a/packages/node-integration-tests/utils/index.ts b/packages/node-integration-tests/utils/index.ts index a46e76621e07..88183423db02 100644 --- a/packages/node-integration-tests/utils/index.ts +++ b/packages/node-integration-tests/utils/index.ts @@ -200,13 +200,20 @@ export class TestEnv { * @param {Record} [headers] * @return {*} {Promise} */ - public async getAPIResponse(url?: string, headers?: Record): Promise { + public async getAPIResponse( + url?: string, + headers?: Record, + endServer: boolean = true, + ): Promise { try { const { data } = await axios.get(url || this.url, { headers: headers || {} }); return data; } finally { await Sentry.flush(); - this.server.close(); + + if (endServer) { + this.server.close(); + } } } @@ -257,4 +264,42 @@ export class TestEnv { public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { this._axiosConfig = axiosConfig; } + + public async countEnvelopes(options: { + url?: string; + timeout?: number; + envelopeType: EnvelopeItemType | EnvelopeItemType[]; + endServer?: boolean; + }): Promise { + return new Promise(resolve => { + let reqCount = 0; + + const mock = nock('https://dsn.ingest.sentry.io') + .persist() + .post('/api/1337/envelope/', body => { + const envelope = parseEnvelope(body); + + if (options.envelopeType.includes(envelope[1].type as EnvelopeItemType)) { + reqCount++; + return true; + } + + return false; + }); + + setTimeout(() => { + nock.removeInterceptor(mock); + + if (options.endServer) { + nock.cleanAll(); + + this.server.close(() => { + resolve(reqCount); + }); + } + + resolve(reqCount); + }, options.timeout || 1000); + }); + } } From cc52db7f475e3ef5cd2c0a08f3d1df8a1e299256 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Jan 2023 09:39:10 +0000 Subject: [PATCH 02/10] Use the same TS version with `integration-tests`. --- packages/nextjs/test/integration/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/test/integration/package.json b/packages/nextjs/test/integration/package.json index d86872ff999a..0e862d2a52dd 100644 --- a/packages/nextjs/test/integration/package.json +++ b/packages/nextjs/test/integration/package.json @@ -20,7 +20,7 @@ "@types/react": "17.0.47", "@types/react-dom": "17.0.17", "nock": "^13.1.0", - "typescript": "^4.2.4", + "typescript": "^4.5.2", "yargs": "^16.2.0" }, "resolutions": { From ae0202c48100bd06eb0c28619fa3a7b504dbf884 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Jan 2023 09:59:45 +0000 Subject: [PATCH 03/10] Fix chalk import. --- packages/nextjs/src/config/webpack.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index a4bd5e6bba65..8894ef33b2a0 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -3,7 +3,7 @@ import { getSentryRelease } from '@sentry/node'; import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; -import * as chalk from 'chalk'; +import chalk from 'chalk'; import * as fs from 'fs'; import * as path from 'path'; From c5028dfa67080b44a411c180a1cd6464f581aed9 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Jan 2023 11:19:03 +0000 Subject: [PATCH 04/10] Clean up --- packages/nextjs/jest.config.js | 2 +- packages/nextjs/src/config/webpack.ts | 2 +- .../test/server/cjsApiEndpoints.test.ts | 2 +- .../server/doubleEndMethodOnVercel.test.ts | 2 +- .../test/server/errorApiEndpoint.test.ts | 2 +- .../test/server/errorServerSideProps.test.ts | 2 +- .../test/server/excludedApiEndpoints.test.ts | 2 +- .../test/server/tracing200.test.ts | 2 +- .../test/server/tracing500.test.ts | 2 +- .../test/server/tracingHttp.test.ts | 2 +- .../tracingServerGetInitialProps.test.ts | 2 +- .../tracingServerGetServerSideProps.test.ts | 2 +- ...ServerSidePropsCustomPageExtension.test.ts | 2 +- .../test/server/tracingWithSentryAPI.test.ts | 2 +- .../integration/test/server/utils/helpers.ts | 23 ++++ .../test/integration/test/utils/server.ts | 119 ------------------ .../test/integration/tsconfig.test.json | 3 +- packages/nextjs/test/tsconfig.json | 7 +- packages/nextjs/tsconfig.json | 1 - packages/nextjs/tsconfig.test.json | 1 + 20 files changed, 46 insertions(+), 136 deletions(-) create mode 100644 packages/nextjs/test/integration/test/server/utils/helpers.ts delete mode 100644 packages/nextjs/test/integration/test/utils/server.ts diff --git a/packages/nextjs/jest.config.js b/packages/nextjs/jest.config.js index 70485db447fa..4001570bcdb9 100644 --- a/packages/nextjs/jest.config.js +++ b/packages/nextjs/jest.config.js @@ -4,5 +4,5 @@ module.exports = { ...baseConfig, // This prevents the build tests from running when unit tests run. (If they do, they fail, because the build being // tested hasn't necessarily run yet.) - testPathIgnorePatterns: ['/test/buildProcess/'], + testPathIgnorePatterns: ['/test/buildProcess/', '/test/integration/'], }; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 8894ef33b2a0..a4bd5e6bba65 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -3,7 +3,7 @@ import { getSentryRelease } from '@sentry/node'; import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; -import chalk from 'chalk'; +import * as chalk from 'chalk'; import * as fs from 'fs'; import * as path from 'path'; diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts index bd8769db4afa..510a8c917e4e 100644 --- a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts +++ b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('CommonJS API Endpoints', () => { it('should not intercept unwrapped request', async () => { diff --git a/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts b/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts index d17631bd4fa5..07b70e61e036 100644 --- a/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts +++ b/packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; // This test asserts that our wrapping of `res.end` doesn't break API routes on Vercel if people call `res.json` or // `res.send` multiple times in one request handler. diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts index c2664b66f149..ced2d0570c04 100644 --- a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts +++ b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; jest.spyOn(console, 'error').mockImplementation(); diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts index cbe11e6957e1..fe455719d760 100644 --- a/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts +++ b/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('Error Server-side Props', () => { it('should capture an error event', async () => { diff --git a/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts b/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts index d6c60c8886f8..810fa899e926 100644 --- a/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts +++ b/packages/nextjs/test/integration/test/server/excludedApiEndpoints.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('Excluded API Endpoints', () => { it('Should exclude API endpoint via RegExp', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracing200.test.ts b/packages/nextjs/test/integration/test/server/tracing200.test.ts index 0288c7172623..f26a2feffb43 100644 --- a/packages/nextjs/test/integration/test/server/tracing200.test.ts +++ b/packages/nextjs/test/integration/test/server/tracing200.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('Tracing 200', () => { it('should capture a transaction', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracing500.test.ts b/packages/nextjs/test/integration/test/server/tracing500.test.ts index 38067eecb4d9..a1a2fb24edff 100644 --- a/packages/nextjs/test/integration/test/server/tracing500.test.ts +++ b/packages/nextjs/test/integration/test/server/tracing500.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('Tracing 500', () => { it('should capture an erroneous transaction', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts index d8eb7089a09f..c09e64a13cd8 100644 --- a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; import nock from 'nock'; describe('Tracing HTTP', () => { diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts index 7ae9b8b0404e..530a9b1e9b63 100644 --- a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('getInitialProps', () => { it('should capture a transaction', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts index e73c0124d9cf..cfcad2f59dfc 100644 --- a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('getServerSideProps', () => { it('should capture a transaction', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts index a138844cb9be..896123e9ce9e 100644 --- a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; describe('tracingServerGetServerSidePropsCustomPageExtension', () => { it('should capture a transaction', async () => { diff --git a/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts b/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts index b2a11c9ebf57..c0f91f20c98e 100644 --- a/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingWithSentryAPI.test.ts @@ -1,4 +1,4 @@ -import { NextTestEnv } from '../utils/server'; +import { NextTestEnv } from './utils/helpers'; const cases = [ { diff --git a/packages/nextjs/test/integration/test/server/utils/helpers.ts b/packages/nextjs/test/integration/test/server/utils/helpers.ts new file mode 100644 index 000000000000..8c9643fb2f4a --- /dev/null +++ b/packages/nextjs/test/integration/test/server/utils/helpers.ts @@ -0,0 +1,23 @@ +import { getPortPromise } from 'portfinder'; +import { TestEnv } from '../../../../../../node-integration-tests/utils'; +import * as http from 'http'; +import * as path from 'path'; +import { createNextServer, startServer } from '../../utils/common'; + +export class NextTestEnv extends TestEnv { + private constructor(public readonly server: http.Server, public readonly url: string) { + super(server, url); + } + + public static async init(): Promise { + const port = await getPortPromise(); + const server = await createNextServer({ + dev: false, + dir: path.resolve(__dirname, '../../..'), + }); + + await startServer(server, port); + + return new NextTestEnv(server, `http://localhost:${port}`); + } +} diff --git a/packages/nextjs/test/integration/test/utils/server.ts b/packages/nextjs/test/integration/test/utils/server.ts deleted file mode 100644 index 8c7fb4fcfb14..000000000000 --- a/packages/nextjs/test/integration/test/utils/server.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { getPortPromise } from 'portfinder'; -import { TestEnv } from '../../../../../node-integration-tests/utils'; -import * as http from 'http'; -import * as path from 'path'; -import * as nodeSDK from '@sentry/node'; -import { createNextServer, startServer } from '../utils/common'; -import { get } from 'http'; - -type HttpGet = ( - options: http.RequestOptions | string | URL, - callback?: (res: http.IncomingMessage) => void, -) => http.ClientRequest; - -export class NextTestEnv extends TestEnv { - private constructor(public readonly server: http.Server, public readonly url: string) { - super(server, url); - } - - public static async init(): Promise { - const port = await getPortPromise(); - const server = await createNextServer({ - dev: false, - dir: path.resolve(__dirname, '../..'), - }); - - await startServer(server, port); - - return new NextTestEnv(server, `http://localhost:${port}`); - } - - /** - * Rewrap `http.get` if the wrapped version has been overridden by `nock`. - * - * This is only relevant for Nextjs >= 12.1, which changed when `_app` is initialized, which in turn changed the order - * in which our SDK and `nock` wrap `http.get`. See https://github.com/getsentry/sentry-javascript/pull/4619. - * - * TODO: We'll have to do this for `ClientRequest` also if we decide to start wrapping that. - * TODO: Can we fix the wrapping-things-twice problem discussed in the comment below? - */ - public ensureWrappedGet(importedGet: HttpGet, url: string) { - // we always test against the latest minor for any given Nextjs major version, so if we're testing Next 12, it's - // definitely at least 12.1, making this check against the major version sufficient - if (Number(process.env.NEXTJS_VERSION) < 12) { - return importedGet; - } - - // As of Next 12.1, creating a `NextServer` instance (which we do immediately upon starting this test runner) loads - // `_app`, which has the effect of initializing the SDK. So, unless something's gone wrong, we should always be able - // to find the integration - const hub = nodeSDK.getCurrentHub(); - const client = hub.getClient(); - - if (!client) { - console.warn(`Warning: Sentry SDK not set up at \`NextServer\` initialization. Request URL: ${url}`); - return importedGet; - } - - const httpIntegration = client.getIntegration(nodeSDK.Integrations.Http); - - // This rewraps `http.get` and `http.request`, which, at this point, look like `nockWrapper(sentryWrapper(get))` and - // `nockWrapper(sentryWrapper(request))`. By the time we're done with this function, they'll look like - // `sentryWrapper(nockWrapper(sentryWrapper(get)))` and `sentryWrapper(nockWrapper(sentryWrapper(request)))`, - // respectively. Though this seems less than ideal, we don't have to worry about our instrumentation being - // (meaningfully) called twice because: - // - // 1) As long as we set up a `nock` interceptor for any outgoing http request, `nock`'s wrapper will call a replacement - // function for that request rather than call the function it's wrapping (in other words, it will look more like - // `sentryWrapper(nockWrapper(getSubstitute))` than `sentryWrapper(nockWrapper(sentryWrapper(get)))`), which means - // our code is only called once. - // 2) In cases where we don't set up an interceptor (such as for the `wrappedGet` call in `getAsync` above), it's true - // that we can end up with `sentryWrapper(nockWrapper(sentryWrapper(get)))`, meaning our wrapper code will run - // twice. For now that's okay because in those cases we're not in the middle of a transactoin and therefore - // the two wrappers' respective attempts to start spans will both no-op. - // - // TL; DR - if the double-wrapping means you're seeing two spans where you really only want one, set up a nock - // interceptor for the request. - // - // TODO: add in a "don't do this twice" check (in `fill`, maybe moved from `wrap`), so that we don't wrap the outer - // wrapper with a third wrapper - if (httpIntegration) { - httpIntegration.setupOnce( - () => undefined, - () => hub, - ); - } - - // now that we've rewrapped it, grab the correct version of `get` for use in our tests - const httpModule = require('http'); - return httpModule.get; - } - - public async getAsync(url: string, rewrap = false) { - // Depending on what version of Nextjs we're testing, the wrapping which happens in the `Http` integration may have - // happened too early and gotten overwritten by `nock`. This redoes the wrapping if so. - // - // TODO: This works but is pretty hacky in that it has the potential to wrap things multiple times, more even than the - // double-wrapping which is discussed at length in the comment in `ensureWrappedGet` below, which is why we need - // `rewrap`. Once we fix `fill` to not wrap things twice, we should be able to take this out and just always call - // `ensureWrappedGet`. - const wrappedGet = rewrap ? this.ensureWrappedGet(get, url) : get; - - return new Promise((resolve, reject) => { - wrappedGet(url, (res: http.IncomingMessage) => { - res.setEncoding('utf8'); - let rawData = ''; - res.on('data', chunk => { - rawData += chunk; - }); - res.on('end', () => { - try { - resolve(rawData); - } catch (e) { - reject(e); - } - }); - }); - }); - } -} diff --git a/packages/nextjs/test/integration/tsconfig.test.json b/packages/nextjs/test/integration/tsconfig.test.json index d3175b6a1b01..7aa20c05d60c 100644 --- a/packages/nextjs/test/integration/tsconfig.test.json +++ b/packages/nextjs/test/integration/tsconfig.test.json @@ -4,6 +4,7 @@ "include": ["test/**/*"], "compilerOptions": { - "types": ["node", "jest"] + "types": ["node", "jest"], + "esModuleInterop": true } } diff --git a/packages/nextjs/test/tsconfig.json b/packages/nextjs/test/tsconfig.json index fd214f0667a5..18476a970d87 100644 --- a/packages/nextjs/test/tsconfig.json +++ b/packages/nextjs/test/tsconfig.json @@ -4,5 +4,10 @@ { "extends": "../tsconfig.test.json", - "include": ["./**/*", "../playwright.config.ts"] + "include": ["./**/*", "../playwright.config.ts"], + + "compilerOptions": { + "types": ["node", "jest"], + "esModuleInterop": true + } } diff --git a/packages/nextjs/tsconfig.json b/packages/nextjs/tsconfig.json index ff9a3c1f98f6..bf45a09f2d71 100644 --- a/packages/nextjs/tsconfig.json +++ b/packages/nextjs/tsconfig.json @@ -4,7 +4,6 @@ "include": ["src/**/*"], "compilerOptions": { - "esModuleInterop": true // package-specific options } } diff --git a/packages/nextjs/tsconfig.test.json b/packages/nextjs/tsconfig.test.json index 87f6afa06b86..8326c827c32f 100644 --- a/packages/nextjs/tsconfig.test.json +++ b/packages/nextjs/tsconfig.test.json @@ -4,6 +4,7 @@ "include": ["test/**/*"], "compilerOptions": { + "esModuleInterop": true, // should include all types from `./tsconfig.json` plus types for all test frameworks used "types": ["node", "jest"] From 276a79dfe8f90cc0a5f5a2312f307d0fa72d265d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Jan 2023 16:20:40 +0000 Subject: [PATCH 05/10] Avoid Segmentation Fault --- packages/nextjs/package.json | 2 +- .../nextjs/test/integration/next12.config.template | 7 +------ .../nextjs/test/integration/next13.config.template | 7 +------ packages/nextjs/test/integration/package.json | 5 +++-- .../integration/test/server/errorApiEndpoint.test.ts | 2 -- .../test/integration/test/server/utils/helpers.ts | 5 +++++ packages/nextjs/test/integration/test/utils/common.ts | 6 ++---- packages/nextjs/test/integration/tsconfig.json | 1 + packages/nextjs/test/integration/tsconfig.test.json | 3 +-- packages/nextjs/test/tsconfig.json | 7 +------ packages/node-integration-tests/utils/index.ts | 11 ++++------- 11 files changed, 20 insertions(+), 36 deletions(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index c0a14fde1748..428db8717eec 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -70,7 +70,7 @@ "test:integration": "./test/run-integration-tests.sh && yarn test:types", "test:integration:clean": "(cd test/integration && rimraf .cache node_modules build)", "test:integration:client": "yarn playwright test test/integration/test/client/", - "test:integration:server": "export NODE_OPTIONS='--stack-trace-limit=25' && jest --config=test/integration/jest.config.js test/integration/test/server/", + "test:integration:server": "(cd test/integration && yarn test:server)", "test:types": "cd test/types && yarn test", "test:watch": "jest --watch", "vercel:branch": "source vercel/set-up-branch-for-test-app-use.sh", diff --git a/packages/nextjs/test/integration/next12.config.template b/packages/nextjs/test/integration/next12.config.template index 28aaba18639a..061cfb6708b6 100644 --- a/packages/nextjs/test/integration/next12.config.template +++ b/packages/nextjs/test/integration/next12.config.template @@ -1,8 +1,6 @@ const { withSentryConfig } = require('@sentry/nextjs'); -// NOTE: This will be used by integration tests to distinguish between Webpack 4 and Webpack 5 const moduleExports = { - webpack5: %RUN_WEBPACK_5%, eslint: { ignoreDuringBuilds: true, }, @@ -11,10 +9,7 @@ const moduleExports = { // Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts` // TODO (v8): This can come out in v8, because this option will get a default value hideSourceMaps: false, - excludeServerRoutes: [ - '/api/excludedEndpoints/excludedWithString', - /\/api\/excludedEndpoints\/excludedWithRegExp/, - ], + excludeServerRoutes: ['/api/excludedEndpoints/excludedWithString', /\/api\/excludedEndpoints\/excludedWithRegExp/], }, }; diff --git a/packages/nextjs/test/integration/next13.config.template b/packages/nextjs/test/integration/next13.config.template index 7719fae3fdf8..20118cbb47f3 100644 --- a/packages/nextjs/test/integration/next13.config.template +++ b/packages/nextjs/test/integration/next13.config.template @@ -1,8 +1,6 @@ const { withSentryConfig } = require('@sentry/nextjs'); -// NOTE: This will be used by integration tests to distinguish between Webpack 4 and Webpack 5 const moduleExports = { - webpack5: %RUN_WEBPACK_5%, eslint: { ignoreDuringBuilds: true, }, @@ -14,10 +12,7 @@ const moduleExports = { // Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts` // TODO (v8): This can come out in v8, because this option will get a default value hideSourceMaps: false, - excludeServerRoutes: [ - '/api/excludedEndpoints/excludedWithString', - /\/api\/excludedEndpoints\/excludedWithRegExp/, - ], + excludeServerRoutes: ['/api/excludedEndpoints/excludedWithString', /\/api\/excludedEndpoints\/excludedWithRegExp/], }, }; diff --git a/packages/nextjs/test/integration/package.json b/packages/nextjs/test/integration/package.json index 0e862d2a52dd..4ff461b86f04 100644 --- a/packages/nextjs/test/integration/package.json +++ b/packages/nextjs/test/integration/package.json @@ -7,7 +7,8 @@ "predebug": "source ../integration_test_utils.sh && link_monorepo_packages '../../..' && yarn build", "start": "next start", "pretest": "yarn build", - "test": "playwright test" + "test:client": "playwright test", + "test:server": "jest --detectOpenHandles --forceExit --runInBand" }, "dependencies": { "@sentry/nextjs": "file:../../", @@ -20,7 +21,7 @@ "@types/react": "17.0.47", "@types/react-dom": "17.0.17", "nock": "^13.1.0", - "typescript": "^4.5.2", + "typescript": "^4.2.4", "yargs": "^16.2.0" }, "resolutions": { diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts index ced2d0570c04..45168eeeee33 100644 --- a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts +++ b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts @@ -1,7 +1,5 @@ import { NextTestEnv } from './utils/helpers'; -jest.spyOn(console, 'error').mockImplementation(); - describe('Error API Endpoints', () => { it('should capture an error event', async () => { const env = await NextTestEnv.init(); diff --git a/packages/nextjs/test/integration/test/server/utils/helpers.ts b/packages/nextjs/test/integration/test/server/utils/helpers.ts index 8c9643fb2f4a..73a08bc73971 100644 --- a/packages/nextjs/test/integration/test/server/utils/helpers.ts +++ b/packages/nextjs/test/integration/test/server/utils/helpers.ts @@ -14,6 +14,11 @@ export class NextTestEnv extends TestEnv { const server = await createNextServer({ dev: false, dir: path.resolve(__dirname, '../../..'), + + // This needs to be explicitly passed to the server + // Otherwise it causes Segmentation Fault with NextJS >= 12 + // https://github.com/vercel/next.js/issues/33008 + conf: path.resolve(__dirname, '../../next.config.js'), }); await startServer(server, port); diff --git a/packages/nextjs/test/integration/test/utils/common.ts b/packages/nextjs/test/integration/test/utils/common.ts index e101c8aed5f6..11685f218175 100644 --- a/packages/nextjs/test/integration/test/utils/common.ts +++ b/packages/nextjs/test/integration/test/utils/common.ts @@ -2,14 +2,12 @@ import { createServer, Server } from 'http'; import { parse } from 'url'; import { promises, PathLike } from 'fs'; import next from 'next'; -import { NextServer } from 'next/dist/server/next'; const { stat } = promises; // Type not exported from NextJS -type NextServerConstructor = ConstructorParameters[0]; - -export const createNextServer = async (config: NextServerConstructor) => { +// @ts-ignore +export const createNextServer = async config => { const app = next(config); const handle = app.getRequestHandler(); await app.prepare(); diff --git a/packages/nextjs/test/integration/tsconfig.json b/packages/nextjs/test/integration/tsconfig.json index b18a128f2711..34a4dafdcf48 100644 --- a/packages/nextjs/test/integration/tsconfig.json +++ b/packages/nextjs/test/integration/tsconfig.json @@ -1,6 +1,7 @@ { "compilerOptions": { "allowJs": true, + "allowSyntheticDefaultImports": true, "alwaysStrict": true, "esModuleInterop": true, "forceConsistentCasingInFileNames": true, diff --git a/packages/nextjs/test/integration/tsconfig.test.json b/packages/nextjs/test/integration/tsconfig.test.json index 7aa20c05d60c..d3175b6a1b01 100644 --- a/packages/nextjs/test/integration/tsconfig.test.json +++ b/packages/nextjs/test/integration/tsconfig.test.json @@ -4,7 +4,6 @@ "include": ["test/**/*"], "compilerOptions": { - "types": ["node", "jest"], - "esModuleInterop": true + "types": ["node", "jest"] } } diff --git a/packages/nextjs/test/tsconfig.json b/packages/nextjs/test/tsconfig.json index 18476a970d87..fd214f0667a5 100644 --- a/packages/nextjs/test/tsconfig.json +++ b/packages/nextjs/test/tsconfig.json @@ -4,10 +4,5 @@ { "extends": "../tsconfig.test.json", - "include": ["./**/*", "../playwright.config.ts"], - - "compilerOptions": { - "types": ["node", "jest"], - "esModuleInterop": true - } + "include": ["./**/*", "../playwright.config.ts"] } diff --git a/packages/node-integration-tests/utils/index.ts b/packages/node-integration-tests/utils/index.ts index 88183423db02..cde2cb745cd9 100644 --- a/packages/node-integration-tests/utils/index.ts +++ b/packages/node-integration-tests/utils/index.ts @@ -269,7 +269,6 @@ export class TestEnv { url?: string; timeout?: number; envelopeType: EnvelopeItemType | EnvelopeItemType[]; - endServer?: boolean; }): Promise { return new Promise(resolve => { let reqCount = 0; @@ -290,13 +289,11 @@ export class TestEnv { setTimeout(() => { nock.removeInterceptor(mock); - if (options.endServer) { - nock.cleanAll(); + nock.cleanAll(); - this.server.close(() => { - resolve(reqCount); - }); - } + this.server.close(() => { + resolve(reqCount); + }); resolve(reqCount); }, options.timeout || 1000); From 94937c62096d08017179887e8155d87edd3b2436 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Jan 2023 16:24:46 +0000 Subject: [PATCH 06/10] Separate TS test configurations. --- packages/nextjs/test/integration/tsconfig.test.json | 3 ++- packages/nextjs/tsconfig.test.json | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/test/integration/tsconfig.test.json b/packages/nextjs/test/integration/tsconfig.test.json index d3175b6a1b01..7aa20c05d60c 100644 --- a/packages/nextjs/test/integration/tsconfig.test.json +++ b/packages/nextjs/test/integration/tsconfig.test.json @@ -4,6 +4,7 @@ "include": ["test/**/*"], "compilerOptions": { - "types": ["node", "jest"] + "types": ["node", "jest"], + "esModuleInterop": true } } diff --git a/packages/nextjs/tsconfig.test.json b/packages/nextjs/tsconfig.test.json index 8326c827c32f..87f6afa06b86 100644 --- a/packages/nextjs/tsconfig.test.json +++ b/packages/nextjs/tsconfig.test.json @@ -4,7 +4,6 @@ "include": ["test/**/*"], "compilerOptions": { - "esModuleInterop": true, // should include all types from `./tsconfig.json` plus types for all test frameworks used "types": ["node", "jest"] From add1f41b14c59256c8bee4117467124f095d7ec8 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 25 Jan 2023 09:48:16 +0000 Subject: [PATCH 07/10] Clean up --- .../integration/test/server/utils/helpers.ts | 31 ++++++++- .../test/integration/test/utils/common.ts | 63 ------------------- .../test/integration/tsconfig.test.json | 2 +- 3 files changed, 31 insertions(+), 65 deletions(-) delete mode 100644 packages/nextjs/test/integration/test/utils/common.ts diff --git a/packages/nextjs/test/integration/test/server/utils/helpers.ts b/packages/nextjs/test/integration/test/server/utils/helpers.ts index 73a08bc73971..e483f08a144c 100644 --- a/packages/nextjs/test/integration/test/server/utils/helpers.ts +++ b/packages/nextjs/test/integration/test/server/utils/helpers.ts @@ -2,7 +2,36 @@ import { getPortPromise } from 'portfinder'; import { TestEnv } from '../../../../../../node-integration-tests/utils'; import * as http from 'http'; import * as path from 'path'; -import { createNextServer, startServer } from '../../utils/common'; +import { createServer, Server } from 'http'; +import { parse } from 'url'; +import next from 'next'; + +// Type not exported from NextJS +// @ts-ignore +export const createNextServer = async config => { + const app = next(config); + const handle = app.getRequestHandler(); + await app.prepare(); + + return createServer((req, res) => { + const { url } = req; + + if (!url) { + throw new Error('No url'); + } + + handle(req, res, parse(url, true)); + }); +}; + +export const startServer = async (server: Server, port: string | number) => { + return new Promise(resolve => { + server.listen(port || 0, () => { + const url = `http://localhost:${port}`; + resolve({ server, url }); + }); + }); +}; export class NextTestEnv extends TestEnv { private constructor(public readonly server: http.Server, public readonly url: string) { diff --git a/packages/nextjs/test/integration/test/utils/common.ts b/packages/nextjs/test/integration/test/utils/common.ts deleted file mode 100644 index 11685f218175..000000000000 --- a/packages/nextjs/test/integration/test/utils/common.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { createServer, Server } from 'http'; -import { parse } from 'url'; -import { promises, PathLike } from 'fs'; -import next from 'next'; - -const { stat } = promises; - -// Type not exported from NextJS -// @ts-ignore -export const createNextServer = async config => { - const app = next(config); - const handle = app.getRequestHandler(); - await app.prepare(); - - return createServer((req, res) => { - const { url } = req; - - if (!url) { - throw new Error('No url'); - } - - handle(req, res, parse(url, true)); - }); -}; - -export const startServer = async (server: Server, port: string | number) => { - return new Promise(resolve => { - server.listen(port || 0, () => { - const url = `http://localhost:${port}`; - resolve({ server, url }); - }); - }); -}; - -export const COLOR_RESET = '\x1b[0m'; -export const COLORS = { - green: '\x1b[32m', - red: '\x1b[31m', -}; - -export const colorize = (str: string, color: 'green' | 'red') => { - if (!(color in COLORS)) { - throw new Error(`Unknown color. Available colors: ${Object.keys(COLORS).join(', ')}`); - } - - return `${COLORS[color]}${str}${COLOR_RESET}`; -}; - -export const verifyDir = async (path: PathLike) => { - if (!(await stat(path)).isDirectory()) { - throw new Error(`Invalid scenariosDir: ${path} is not a directory`); - } -}; - -export const sleep = (duration: number) => { - return new Promise(resolve => setTimeout(() => resolve(), duration)); -}; - -export const waitForAll = (actions: any[]) => { - return Promise.all(actions).catch(() => { - throw new Error('Failed to await on all requested actions'); - }); -}; diff --git a/packages/nextjs/test/integration/tsconfig.test.json b/packages/nextjs/test/integration/tsconfig.test.json index 7aa20c05d60c..3b04de6d5bc0 100644 --- a/packages/nextjs/test/integration/tsconfig.test.json +++ b/packages/nextjs/test/integration/tsconfig.test.json @@ -1,5 +1,5 @@ { - "extends": "./tsconfig.json", + "extends": "../../tsconfig.test.json", "include": ["test/**/*"], From be9911433f3821f6f6af0078f2a2d5fadb1e3497 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 25 Jan 2023 11:37:12 +0000 Subject: [PATCH 08/10] Separate `appDir` tests. --- .../integration/next13.appdir.config.template | 25 ++ .../test/integration/next13.config.template | 4 +- ...pDirTracingPageloadClientcomponent.test.ts | 3 +- ...pDirTracingPageloadServercomponent.test.ts | 3 +- packages/nextjs/test/run-integration-tests.sh | 240 ++++++++++-------- 5 files changed, 156 insertions(+), 119 deletions(-) create mode 100644 packages/nextjs/test/integration/next13.appdir.config.template diff --git a/packages/nextjs/test/integration/next13.appdir.config.template b/packages/nextjs/test/integration/next13.appdir.config.template new file mode 100644 index 000000000000..4b17134ee1d7 --- /dev/null +++ b/packages/nextjs/test/integration/next13.appdir.config.template @@ -0,0 +1,25 @@ +const { withSentryConfig } = require('@sentry/nextjs'); + +const moduleExports = { + eslint: { + ignoreDuringBuilds: true, + }, + experimental: { + appDir: Number(process.env.NODE_MAJOR) >= 16, // experimental.appDir requires Node v16.8.0 or later. + }, + pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'page.tsx'], + sentry: { + // Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts` + // TODO (v8): This can come out in v8, because this option will get a default value + hideSourceMaps: false, + excludeServerRoutes: ['/api/excludedEndpoints/excludedWithString', /\/api\/excludedEndpoints\/excludedWithRegExp/], + }, +}; + +const SentryWebpackPluginOptions = { + dryRun: true, + silent: true, +}; + +module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions); + diff --git a/packages/nextjs/test/integration/next13.config.template b/packages/nextjs/test/integration/next13.config.template index 20118cbb47f3..e32ecec4b0f4 100644 --- a/packages/nextjs/test/integration/next13.config.template +++ b/packages/nextjs/test/integration/next13.config.template @@ -4,9 +4,6 @@ const moduleExports = { eslint: { ignoreDuringBuilds: true, }, - experimental: { - appDir: Number(process.env.NODE_MAJOR) >= 16, // experimental.appDir requires Node v16.8.0 or later. - }, pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'page.tsx'], sentry: { // Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts` @@ -22,3 +19,4 @@ const SentryWebpackPluginOptions = { }; module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions); + diff --git a/packages/nextjs/test/integration/test/client/appDirTracingPageloadClientcomponent.test.ts b/packages/nextjs/test/integration/test/client/appDirTracingPageloadClientcomponent.test.ts index c1fca9390ac4..25b4be5fedc0 100644 --- a/packages/nextjs/test/integration/test/client/appDirTracingPageloadClientcomponent.test.ts +++ b/packages/nextjs/test/integration/test/client/appDirTracingPageloadClientcomponent.test.ts @@ -4,8 +4,7 @@ import { test, expect } from '@playwright/test'; test('should create a pageload transaction when the `app` directory is used with a client component.', async ({ page, }) => { - if (Number(process.env.NEXTJS_VERSION) < 13 || Number(process.env.NODE_MAJOR) < 16) { - // Next.js versions < 13 don't support the app directory and the app dir requires Node v16.8.0 or later. + if (process.env.USE_APPDIR !== 'true') { return; } diff --git a/packages/nextjs/test/integration/test/client/appDirTracingPageloadServercomponent.test.ts b/packages/nextjs/test/integration/test/client/appDirTracingPageloadServercomponent.test.ts index 3c6e3fcd6181..bb3c2a61d675 100644 --- a/packages/nextjs/test/integration/test/client/appDirTracingPageloadServercomponent.test.ts +++ b/packages/nextjs/test/integration/test/client/appDirTracingPageloadServercomponent.test.ts @@ -4,8 +4,7 @@ import { test, expect } from '@playwright/test'; test('should create a pageload transaction when the `app` directory is used with a server component.', async ({ page, }) => { - if (Number(process.env.NEXTJS_VERSION) < 13 || Number(process.env.NODE_MAJOR) < 16) { - // Next.js versions < 13 don't support the app directory and the app dir requires Node v16.8.0 or later. + if (process.env.USE_APPDIR !== 'true') { return; } diff --git a/packages/nextjs/test/run-integration-tests.sh b/packages/nextjs/test/run-integration-tests.sh index f3e167bb4040..225ff2139e80 100755 --- a/packages/nextjs/test/run-integration-tests.sh +++ b/packages/nextjs/test/run-integration-tests.sh @@ -31,133 +31,149 @@ echo "Running integration tests on Node $NODE_VERSION" # make a backup of our config file so we can restore it when we're done mv next.config.js next.config.js.bak -for NEXTJS_VERSION in 10 11 12 13; do - - # export this to the env so that we can behave differently depending on which version of next we're testing, without - # having to pass this value from function to function to function to the one spot, deep in some callstack, where we - # actually need it - export NEXTJS_VERSION=$NEXTJS_VERSION - export NODE_MAJOR=$NODE_MAJOR - - # Next 10 requires at least Node v10 - if [ "$NODE_MAJOR" -lt "10" ]; then - echo "[nextjs] Next.js is not compatible with versions of Node older than v10. Current version $NODE_VERSION" - exit 0 - fi - - # Next.js v11 requires at least Node v12 - if [ "$NODE_MAJOR" -lt "12" ] && [ "$NEXTJS_VERSION" -ge "11" ]; then - echo "[nextjs@$NEXTJS_VERSION] Not compatible with Node $NODE_MAJOR" - exit 0 - fi - - # Next.js v13 requires at least Node v14 - if [ "$NODE_MAJOR" -lt "14" ] && [ "$NEXTJS_VERSION" -ge "13" ]; then - echo "[nextjs@$NEXTJS_VERSION] Not compatible with Node $NODE_MAJOR" - exit 0 - fi - - echo "[nextjs@$NEXTJS_VERSION] Preparing environment..." - rm -rf node_modules .next .env.local 2>/dev/null || true - - echo "[nextjs@$NEXTJS_VERSION] Installing dependencies..." - # set the desired version of next long enough to run yarn, and then restore the old version (doing the restoration now - # rather than during overall cleanup lets us look for "latest" in every loop) - cp package.json package.json.bak - if [[ $(uname) == "Darwin" ]]; then - sed -i "" /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json - else - sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json - fi - - # Next.js v13 requires React 18.2.0 - if [ "$NEXTJS_VERSION" -eq "13" ]; then - npm i --save react@18.2.0 react-dom@18.2.0 - fi - # We have to use `--ignore-engines` because sucrase claims to need Node 12, even though tests pass just fine on Node - # 10 - yarn --no-lockfile --ignore-engines --silent >/dev/null 2>&1 - # if applicable, use local versions of `@sentry/cli` and/or `@sentry/webpack-plugin` (these commands no-op unless - # LINKED_CLI_REPO and/or LINKED_PLUGIN_REPO are set) - linkcli && linkplugin - mv -f package.json.bak package.json 2>/dev/null || true - - SHOULD_RUN_WEBPACK_5=(true) - # Only run Webpack 4 tests for Next 10 and Next 11 - if [ "$NEXTJS_VERSION" -eq "10" ] || [ "$NEXTJS_VERSION" -eq "11" ]; then - SHOULD_RUN_WEBPACK_5+=(false) - fi - - for RUN_WEBPACK_5 in ${SHOULD_RUN_WEBPACK_5[*]}; do - [ "$RUN_WEBPACK_5" == true ] && - WEBPACK_VERSION=5 || - WEBPACK_VERSION=4 - - if [ "$NODE_MAJOR" -gt "17" ]; then - # Node v17+ does not work with NextJS 10 and 11 because of their legacy openssl use - # Ref: https://github.com/vercel/next.js/issues/30078 - if [ "$NEXTJS_VERSION" -lt "12" ]; then - echo "[nextjs@$NEXTJS_VERSION Node $NODE_MAJOR not compatible with NextJS $NEXTJS_VERSION" - # Continues the 2nd enclosing loop, which is the outer loop that iterates over the NextJS version - continue 2 - fi +# for NEXTJS_VERSION in 10 11 12 13; do +for NEXTJS_VERSION in 13; do + for USE_APPDIR in true false; do + if ([ "$NEXTJS_VERSION" -lt "13" ] || [ "$NODE_MAJOR" -lt "16" ]) && [ "$USE_APPDIR" == true ]; then + # Next.js < 13 do not support appDir + continue + fi - # Node v18 only with Webpack 5 and above - # https://github.com/webpack/webpack/issues/14532#issuecomment-947513562 - # Context: https://github.com/vercel/next.js/issues/30078#issuecomment-947338268 - if [ "$WEBPACK_VERSION" -eq "4" ]; then - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Node $NODE_MAJOR not compatible with Webpack $WEBPACK_VERSION" - # Continues the 1st enclosing loop, which is the inner loop that iterates over the Webpack version - continue - fi + # export this to the env so that we can behave differently depending on which version of next we're testing, without + # having to pass this value from function to function to function to the one spot, deep in some callstack, where we + # actually need it + export NEXTJS_VERSION=$NEXTJS_VERSION + export NODE_MAJOR=$NODE_MAJOR + export USE_APPDIR=$USE_APPDIR + + # Next 10 requires at least Node v10 + if [ "$NODE_MAJOR" -lt "10" ]; then + echo "[nextjs] Next.js is not compatible with versions of Node older than v10. Current version $NODE_VERSION" + exit 0 + fi + # Next.js v11 requires at least Node v12 + if [ "$NODE_MAJOR" -lt "12" ] && [ "$NEXTJS_VERSION" -ge "11" ]; then + echo "[nextjs@$NEXTJS_VERSION] Not compatible with Node $NODE_MAJOR" + exit 0 fi - # next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings - if [ "$NEXTJS_VERSION" -eq "10" ]; then - sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js - elif [ "$NEXTJS_VERSION" -eq "11" ]; then - sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js - elif [ "$NEXTJS_VERSION" -eq "12" ]; then - sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js - elif [ "$NEXTJS_VERSION" -eq "13" ]; then - sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + # Next.js v13 requires at least Node v14 + if [ "$NODE_MAJOR" -lt "14" ] && [ "$NEXTJS_VERSION" -ge "13" ]; then + echo "[nextjs@$NEXTJS_VERSION] Not compatible with Node $NODE_MAJOR" + exit 0 fi - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Building..." - yarn build + echo "[nextjs@$NEXTJS_VERSION] Preparing environment..." + rm -rf node_modules .next .env.local 2>/dev/null || true - # if the user hasn't passed any args, use the default one, which restricts each test to only outputting success and - # failure messages - args=$* - if [[ ! $args ]]; then - args="--silent" + echo "[nextjs@$NEXTJS_VERSION] Installing dependencies..." + # set the desired version of next long enough to run yarn, and then restore the old version (doing the restoration now + # rather than during overall cleanup lets us look for "latest" in every loop) + cp package.json package.json.bak + if [[ $(uname) == "Darwin" ]]; then + sed -i "" /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json + else + sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json fi - # we keep this updated as we run the tests, so that if it's ever non-zero, we can bail - EXIT_CODE=0 + # Next.js v13 requires React 18.2.0 + if [ "$NEXTJS_VERSION" -eq "13" ]; then + npm i --save react@18.2.0 react-dom@18.2.0 + fi + # We have to use `--ignore-engines` because sucrase claims to need Node 12, even though tests pass just fine on Node + # 10 + yarn --no-lockfile --ignore-engines --silent >/dev/null 2>&1 + # if applicable, use local versions of `@sentry/cli` and/or `@sentry/webpack-plugin` (these commands no-op unless + # LINKED_CLI_REPO and/or LINKED_PLUGIN_REPO are set) + linkcli && linkplugin + mv -f package.json.bak package.json 2>/dev/null || true + + SHOULD_RUN_WEBPACK_5=(true) + # Only run Webpack 4 tests for Next 10 and Next 11 + if [ "$NEXTJS_VERSION" -eq "10" ] || [ "$NEXTJS_VERSION" -eq "11" ]; then + SHOULD_RUN_WEBPACK_5+=(false) + fi - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running server tests with options: $args" - (cd .. && yarn test:integration:server) || EXIT_CODE=$? + for RUN_WEBPACK_5 in ${SHOULD_RUN_WEBPACK_5[*]}; do + [ "$RUN_WEBPACK_5" == true ] && + WEBPACK_VERSION=5 || + WEBPACK_VERSION=4 + + if [ "$NODE_MAJOR" -gt "17" ]; then + # Node v17+ does not work with NextJS 10 and 11 because of their legacy openssl use + # Ref: https://github.com/vercel/next.js/issues/30078 + if [ "$NEXTJS_VERSION" -lt "12" ]; then + echo "[nextjs@$NEXTJS_VERSION Node $NODE_MAJOR not compatible with NextJS $NEXTJS_VERSION" + # Continues the 2nd enclosing loop, which is the outer loop that iterates over the NextJS version + continue 2 + fi + + # Node v18 only with Webpack 5 and above + # https://github.com/webpack/webpack/issues/14532#issuecomment-947513562 + # Context: https://github.com/vercel/next.js/issues/30078#issuecomment-947338268 + if [ "$WEBPACK_VERSION" -eq "4" ]; then + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Node $NODE_MAJOR not compatible with Webpack $WEBPACK_VERSION" + # Continues the 1st enclosing loop, which is the inner loop that iterates over the Webpack version + continue + fi - if [ $EXIT_CODE -eq 0 ]; then - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests passed" - else - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests failed" - exit 1 - fi + fi + + # next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings + if [ "$NEXTJS_VERSION" -eq "10" ]; then + sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + elif [ "$NEXTJS_VERSION" -eq "11" ]; then + sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + elif [ "$NEXTJS_VERSION" -eq "12" ]; then + sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + elif [ "$NEXTJS_VERSION" -eq "13" ]; then + if [ "$USE_APPDIR" == true ]; then + sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + else + sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js + fi + fi + + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Building..." + yarn build + + # if the user hasn't passed any args, use the default one, which restricts each test to only outputting success and + # failure messages + args=$* + if [[ ! $args ]]; then + args="--silent" + fi + + # we keep this updated as we run the tests, so that if it's ever non-zero, we can bail + EXIT_CODE=0 + + if [ "$USE_APPDIR" == true ]; then + echo "Skipping server tests for appdir" + else + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running server tests with options: $args" + (cd .. && yarn test:integration:server) || EXIT_CODE=$? + fi - if [ "$NODE_MAJOR" -lt "14" ]; then - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Skipping client tests on Node $NODE_MAJOR" - else - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running client tests with options: $args" - (cd .. && yarn test:integration:client) || EXIT_CODE=$? if [ $EXIT_CODE -eq 0 ]; then - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests passed" + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests passed" else - echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests failed" + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests failed" exit 1 fi - fi + + if [ "$NODE_MAJOR" -lt "14" ]; then + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Skipping client tests on Node $NODE_MAJOR" + else + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running client tests with options: $args" + (cd .. && yarn test:integration:client) || EXIT_CODE=$? + if [ $EXIT_CODE -eq 0 ]; then + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests passed" + else + echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests failed" + exit 1 + fi + fi + done done done From c050b57d831955f653eadebc23d3de5a3391a30b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 25 Jan 2023 14:18:56 +0000 Subject: [PATCH 09/10] Don't collect coverage --- packages/nextjs/test/integration/jest.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nextjs/test/integration/jest.config.js b/packages/nextjs/test/integration/jest.config.js index 4e888e0f14fb..e5b9abdfdc31 100644 --- a/packages/nextjs/test/integration/jest.config.js +++ b/packages/nextjs/test/integration/jest.config.js @@ -8,4 +8,5 @@ module.exports = { forceExit: true, testTimeout: 30000, setupFilesAfterEnv: [`${__dirname}/jest.setup.js`], + collectCoverage: false, }; From ba4061dcdaee11b96bacdadf29f28e0d5867883e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 25 Jan 2023 14:27:45 +0000 Subject: [PATCH 10/10] Test all versions --- packages/nextjs/test/run-integration-tests.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/test/run-integration-tests.sh b/packages/nextjs/test/run-integration-tests.sh index 225ff2139e80..30ecce9db907 100755 --- a/packages/nextjs/test/run-integration-tests.sh +++ b/packages/nextjs/test/run-integration-tests.sh @@ -31,11 +31,10 @@ echo "Running integration tests on Node $NODE_VERSION" # make a backup of our config file so we can restore it when we're done mv next.config.js next.config.js.bak -# for NEXTJS_VERSION in 10 11 12 13; do -for NEXTJS_VERSION in 13; do +for NEXTJS_VERSION in 10 11 12 13; do for USE_APPDIR in true false; do if ([ "$NEXTJS_VERSION" -lt "13" ] || [ "$NODE_MAJOR" -lt "16" ]) && [ "$USE_APPDIR" == true ]; then - # Next.js < 13 do not support appDir + # App dir doesn not work on Next.js < 13 or Node.js < 16 continue fi