From 05624c71a359ba631045fe0e071a20259f519d20 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sat, 1 Oct 2022 16:04:47 +0000 Subject: [PATCH 1/3] Add tests --- .../pages/api/withSentryAPI/unwrapped/cjsExport.ts | 7 +++++++ .../pages/api/withSentryAPI/wrapped/cjsExport.ts | 7 +++++++ .../test/integration/test/server/cjsApiEndpoints.js | 12 ++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 packages/nextjs/test/integration/pages/api/withSentryAPI/unwrapped/cjsExport.ts create mode 100644 packages/nextjs/test/integration/pages/api/withSentryAPI/wrapped/cjsExport.ts create mode 100644 packages/nextjs/test/integration/test/server/cjsApiEndpoints.js diff --git a/packages/nextjs/test/integration/pages/api/withSentryAPI/unwrapped/cjsExport.ts b/packages/nextjs/test/integration/pages/api/withSentryAPI/unwrapped/cjsExport.ts new file mode 100644 index 000000000000..6ae521fa5cb4 --- /dev/null +++ b/packages/nextjs/test/integration/pages/api/withSentryAPI/unwrapped/cjsExport.ts @@ -0,0 +1,7 @@ +import { NextApiRequest, NextApiResponse } from 'next'; + +const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise => { + res.status(200).json({ success: true }); +}; + +module.exports = handler; diff --git a/packages/nextjs/test/integration/pages/api/withSentryAPI/wrapped/cjsExport.ts b/packages/nextjs/test/integration/pages/api/withSentryAPI/wrapped/cjsExport.ts new file mode 100644 index 000000000000..6ae521fa5cb4 --- /dev/null +++ b/packages/nextjs/test/integration/pages/api/withSentryAPI/wrapped/cjsExport.ts @@ -0,0 +1,7 @@ +import { NextApiRequest, NextApiResponse } from 'next'; + +const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise => { + res.status(200).json({ success: true }); +}; + +module.exports = handler; diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js new file mode 100644 index 000000000000..19c623685386 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js @@ -0,0 +1,12 @@ +const assert = require('assert'); + +const { sleep } = require('../utils/common'); +const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server'); + +module.exports = async ({ url: urlBase, argv }) => { + const responseUnwrapped = await getAsync(`${urlBase}/api/withSentryAPI/unwrapped/cjsExport`); + assert.equal(responseUnwrapped, '{"success":true}'); + + const responseWrapped = await getAsync(`${urlBase}/api/withSentryAPI/wrapped/cjsExport`); + assert.equal(responseWrapped, '{"success":true}'); +}; From 7b63a75cdfabd7787807286aa5d381313933567c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sat, 1 Oct 2022 16:05:28 +0000 Subject: [PATCH 2/3] Fix --- .../templates/apiProxyLoaderTemplate.ts | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts index 52bd696ece16..1cd7c40181eb 100644 --- a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts @@ -16,14 +16,29 @@ import type { PageConfig } from 'next'; // multiple versions of next. See note in `wrappers/types` for more. import type { NextApiHandler } from '../wrappers'; -type NextApiModule = { - default: NextApiHandler; - config?: PageConfig; -}; +type NextApiModule = ( + | { + // ESM export + default?: NextApiHandler; + } + // CJS export + | NextApiHandler +) & { config?: PageConfig }; const userApiModule = origModule as NextApiModule; -const maybeWrappedHandler = userApiModule.default; +// Default to undefined. It's possible for Next.js users to not define any exports/handlers in an API route. If that is +// the case Next.js wil crash during runtime but the Sentry SDK should definitely not crash so we need tohandle it. +let userProvidedHandler = undefined; + +if ('default' in userApiModule && typeof userApiModule.default === 'function') { + // Handle when user defines via ESM export: `export default myFunction;` + userProvidedHandler = userApiModule.default; +} else if (typeof userApiModule === 'function') { + // Handle when user defines via CJS export: "module.exports = myFunction;" + userProvidedHandler = userApiModule; +} + const origConfig = userApiModule.config || {}; // Setting `externalResolver` to `true` prevents nextjs from throwing a warning in dev about API routes resolving @@ -38,7 +53,7 @@ export const config = { }, }; -export default Sentry.withSentryAPI(maybeWrappedHandler, '__ROUTE__'); +export default userProvidedHandler ? Sentry.withSentryAPI(userProvidedHandler, '__ROUTE__') : undefined; // Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to // not include anything whose name matchs something we've explicitly exported above. From 864033dd3864267fc035669b415c9ac107d0f083 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sat, 1 Oct 2022 16:21:37 +0000 Subject: [PATCH 3/3] Add check for transaction in integration test --- .../test/server/cjsApiEndpoints.js | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js index 19c623685386..e154599fcbd7 100644 --- a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js +++ b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.js @@ -1,12 +1,55 @@ const assert = require('assert'); const { sleep } = require('../utils/common'); -const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server'); +const { getAsync, interceptTracingRequest } = require('../utils/server'); module.exports = async ({ url: urlBase, argv }) => { - const responseUnwrapped = await getAsync(`${urlBase}/api/withSentryAPI/unwrapped/cjsExport`); + const unwrappedRoute = '/api/withSentryAPI/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 responseWrapped = await getAsync(`${urlBase}/api/withSentryAPI/wrapped/cjsExport`); + const wrappedRoute = '/api/withSentryAPI/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'); };