From 6f4b0285545f310798ef9f7d1ba5b35302b96dc1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Jan 2023 16:51:32 +0000 Subject: [PATCH 1/6] feat(nextjs): Add edge route and middleware wrappers --- packages/nextjs/jest.config.js | 1 + packages/nextjs/package.json | 3 +- packages/nextjs/src/edge/index.ts | 21 +--- packages/nextjs/src/edge/types.ts | 5 + .../nextjs/src/edge/utils/edgeWrapperUtils.ts | 102 ++++++++++++++++++ packages/nextjs/src/edge/utils/flush.ts | 20 ++++ packages/nextjs/src/edge/withSentryAPI.ts | 29 +++++ .../nextjs/src/edge/withSentryMiddleware.ts | 15 +++ packages/nextjs/src/index.types.ts | 7 ++ .../nextjs/test/edge/edgeWrapperUtils.test.ts | 76 +++++++++++++ packages/nextjs/test/setupUnitTests.ts | 1 + yarn.lock | 2 +- 12 files changed, 263 insertions(+), 19 deletions(-) create mode 100644 packages/nextjs/src/edge/types.ts create mode 100644 packages/nextjs/src/edge/utils/edgeWrapperUtils.ts create mode 100644 packages/nextjs/src/edge/utils/flush.ts create mode 100644 packages/nextjs/src/edge/withSentryAPI.ts create mode 100644 packages/nextjs/src/edge/withSentryMiddleware.ts create mode 100644 packages/nextjs/test/edge/edgeWrapperUtils.test.ts create mode 100644 packages/nextjs/test/setupUnitTests.ts diff --git a/packages/nextjs/jest.config.js b/packages/nextjs/jest.config.js index 70485db447fa..a66844865a51 100644 --- a/packages/nextjs/jest.config.js +++ b/packages/nextjs/jest.config.js @@ -5,4 +5,5 @@ module.exports = { // 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/'], + setupFiles: ['/test/setupUnitTests.ts'], }; diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 1b5ebf5da4c1..8a83479d86b1 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -32,7 +32,8 @@ "devDependencies": { "@types/webpack": "^4.41.31", "eslint-plugin-react": "^7.31.11", - "next": "10.1.3" + "next": "10.1.3", + "whatwg-fetch": "3.6.2" }, "peerDependencies": { "next": "^10.0.8 || ^11.0 || ^12.0 || ^13.0", diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 42b48094d966..b0ad8ec23901 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -119,23 +119,6 @@ export async function close(timeout?: number): Promise { return Promise.resolve(false); } -/** - * Call `flush()` on the current client, if there is one. See {@link Client.flush}. - * - * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause - * the client to wait until all events are sent before resolving the promise. - * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it - * doesn't (or if there's no client defined). - */ -export async function flush(timeout?: number): Promise { - const client = getCurrentHub().getClient(); - if (client) { - return client.flush(timeout); - } - __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.'); - return Promise.resolve(false); -} - /** * This is the getter for lastEventId. * @@ -145,4 +128,8 @@ export function lastEventId(): string | undefined { return getCurrentHub().lastEventId(); } +export { flush } from './utils/flush'; + export * from '@sentry/core'; +export { withSentryAPI } from './withSentryAPI'; +export { withSentryMiddleware } from './withSentryMiddleware'; diff --git a/packages/nextjs/src/edge/types.ts b/packages/nextjs/src/edge/types.ts new file mode 100644 index 000000000000..81bc6796d6e0 --- /dev/null +++ b/packages/nextjs/src/edge/types.ts @@ -0,0 +1,5 @@ +// We cannot make any assumptions about what users define as their handler except maybe that it is a function +export interface EdgeRouteHandler { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (req: any): any | Promise; +} diff --git a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts new file mode 100644 index 000000000000..5ccda34a6ddf --- /dev/null +++ b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts @@ -0,0 +1,102 @@ +import { captureException, getCurrentHub, startTransaction } from '@sentry/core'; +import { hasTracingEnabled } from '@sentry/tracing'; +import type { Span } from '@sentry/types'; +import { + addExceptionMechanism, + baggageHeaderToDynamicSamplingContext, + extractTraceparentData, + logger, + objectify, +} from '@sentry/utils'; + +import type { EdgeRouteHandler } from '../types'; +import { flush } from './flush'; + +/** + * Wraps a function on the edge runtime with error and performance monitoring. + */ +export function withEdgeWrapping( + handler: H, + options: { spanLabel: string; spanOp: string; mechanismFunctionName: string }, +): (...params: Parameters) => Promise> { + return async function (this: unknown, ...args) { + const req = args[0]; + const currentScope = getCurrentHub().getScope(); + const prevSpan = currentScope?.getSpan(); + + let span: Span | undefined; + + if (hasTracingEnabled()) { + if (prevSpan) { + span = prevSpan.startChild({ + description: options.spanLabel, + op: options.spanOp, + }); + } else if (req instanceof Request) { + // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) + let traceparentData; + + const sentryTraceHeader = req.headers.get('sentry-trace'); + if (sentryTraceHeader) { + traceparentData = extractTraceparentData(sentryTraceHeader); + __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); + } + + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(req.headers.get('baggage')); + + span = startTransaction( + { + name: options.spanLabel, + op: options.spanOp, + ...traceparentData, + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source: 'route', + }, + }, + // extra context passed to the `tracesSampler` + { request: req }, + ); + } + + currentScope?.setSpan(span); + } + + try { + const handlerResult: ReturnType = await handler.apply(this, args); + + if ((handlerResult as unknown) instanceof Response) { + span?.setHttpStatus(handlerResult.status); + } else { + span?.setStatus('ok'); + } + + return handlerResult; + } catch (e) { + // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can + // store a seen flag on it. + const objectifiedErr = objectify(e); + + currentScope?.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, + }); + return event; + }); + + span?.setStatus('internal_error'); + + captureException(objectifiedErr); + + throw objectifiedErr; + } finally { + span?.finish(); + currentScope?.setSpan(prevSpan); + await flush(2000); + } + }; +} diff --git a/packages/nextjs/src/edge/utils/flush.ts b/packages/nextjs/src/edge/utils/flush.ts new file mode 100644 index 000000000000..5daa52936391 --- /dev/null +++ b/packages/nextjs/src/edge/utils/flush.ts @@ -0,0 +1,20 @@ +import { getCurrentHub } from '@sentry/core'; +import type { Client } from '@sentry/types'; +import { logger } from '@sentry/utils'; + +/** + * Call `flush()` on the current client, if there is one. See {@link Client.flush}. + * + * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause + * the client to wait until all events are sent before resolving the promise. + * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it + * doesn't (or if there's no client defined). + */ +export async function flush(timeout?: number): Promise { + const client = getCurrentHub().getClient(); + if (client) { + return client.flush(timeout); + } + __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.'); + return Promise.resolve(false); +} diff --git a/packages/nextjs/src/edge/withSentryAPI.ts b/packages/nextjs/src/edge/withSentryAPI.ts new file mode 100644 index 000000000000..26e567903acf --- /dev/null +++ b/packages/nextjs/src/edge/withSentryAPI.ts @@ -0,0 +1,29 @@ +import { getCurrentHub } from '@sentry/core'; + +import type { EdgeRouteHandler } from './types'; +import { withEdgeWrapping } from './utils/edgeWrapperUtils'; + +/** + * Wraps a Next.js edge route handler with Sentry error and performance instrumentation. + */ +export function withSentryAPI( + handler: H, + parameterizedRoute: string, +): (...params: Parameters) => Promise> { + return async function (this: unknown, ...args: Parameters): Promise> { + const req = args[0]; + + const isCalledByUser = getCurrentHub().getScope()?.getTransaction(); + + const wrappedHandler = withEdgeWrapping(handler, { + spanLabel: + isCalledByUser || !(req instanceof Request) + ? `handler (${parameterizedRoute})` + : `${req.method} ${parameterizedRoute}`, + spanOp: isCalledByUser ? 'function' : 'http.server', + mechanismFunctionName: 'withSentryAPI', + }); + + return await wrappedHandler.apply(this, args); + }; +} diff --git a/packages/nextjs/src/edge/withSentryMiddleware.ts b/packages/nextjs/src/edge/withSentryMiddleware.ts new file mode 100644 index 000000000000..5bc2480eacb2 --- /dev/null +++ b/packages/nextjs/src/edge/withSentryMiddleware.ts @@ -0,0 +1,15 @@ +import type { EdgeRouteHandler } from './types'; +import { withEdgeWrapping } from './utils/edgeWrapperUtils'; + +/** + * Wraps Next.js middleware with Sentry error and performance instrumentation. + */ +export function withSentryMiddleware( + middleware: H, +): (...params: Parameters) => Promise> { + return withEdgeWrapping(middleware, { + spanLabel: 'middleware', + spanOp: 'middleware.nextjs', + mechanismFunctionName: 'withSentryMiddleware', + }); +} diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index fcce6708a293..9f82be80a67c 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -29,3 +29,10 @@ export declare function close(timeout?: number | undefined): PromiseLike; export declare function lastEventId(): string | undefined; export declare function getSentryRelease(fallback?: string): string | undefined; + +export declare function withSentryAPI any>( + handler: APIHandler, + parameterizedRoute: string, +): ( + ...args: Parameters +) => ReturnType extends Promise ? ReturnType : Promise>; diff --git a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts new file mode 100644 index 000000000000..1f34e81aea57 --- /dev/null +++ b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts @@ -0,0 +1,76 @@ +import * as coreSdk from '@sentry/core'; +import * as sentryTracing from '@sentry/tracing'; + +import { withEdgeWrapping } from '../../src/edge/utils/edgeWrapperUtils'; + +jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true); + +describe('withEdgeWrapping', () => { + it('should return a function that calls the passed function', async () => { + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(_req => origFunctionReturnValue); + + const wrappedFunction = withEdgeWrapping(origFunction, { + spanLabel: 'some label', + mechanismFunctionName: 'some name', + spanOp: 'some op', + }); + + const returnValue = await wrappedFunction(new Request('https://sentry.io/')); + + expect(returnValue).toBe(origFunctionReturnValue); + expect(origFunction).toHaveBeenCalledTimes(1); + }); + + it('should return a function that calls captureException on error', async () => { + const captureExceptionSpy = jest.spyOn(coreSdk, 'captureException'); + const error = new Error(); + const origFunction = jest.fn(_req => { + throw error; + }); + + const wrappedFunction = withEdgeWrapping(origFunction, { + spanLabel: 'some label', + mechanismFunctionName: 'some name', + spanOp: 'some op', + }); + + await expect(wrappedFunction(new Request('https://sentry.io/'))).rejects.toBe(error); + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + }); + + it('should return a function that starts a transaction when a request object is passed', async () => { + const startTransactionSpy = jest.spyOn(coreSdk, 'startTransaction'); + + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(_req => origFunctionReturnValue); + + const wrappedFunction = withEdgeWrapping(origFunction, { + spanLabel: 'some label', + mechanismFunctionName: 'some name', + spanOp: 'some op', + }); + + const request = new Request('https://sentry.io/'); + await wrappedFunction(request); + expect(startTransactionSpy).toHaveBeenCalledTimes(1); + expect(startTransactionSpy).toHaveBeenCalledWith( + expect.objectContaining({ metadata: { source: 'route' }, name: 'some label', op: 'some op' }), + { request }, + ); + }); + + it("should return a function that doesn't crash when req isn't passed", async () => { + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(() => origFunctionReturnValue); + + const wrappedFunction = withEdgeWrapping(origFunction, { + spanLabel: 'some label', + mechanismFunctionName: 'some name', + spanOp: 'some op', + }); + + await expect(wrappedFunction()).resolves.toBe(origFunctionReturnValue); + expect(origFunction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/nextjs/test/setupUnitTests.ts b/packages/nextjs/test/setupUnitTests.ts new file mode 100644 index 000000000000..754f5df863af --- /dev/null +++ b/packages/nextjs/test/setupUnitTests.ts @@ -0,0 +1 @@ +import 'whatwg-fetch'; // polyfill fetch/Request/Response globals which edge routes need diff --git a/yarn.lock b/yarn.lock index c2dbdbaef239..dd3dd1b862d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -25053,7 +25053,7 @@ whatwg-encoding@^2.0.0: dependencies: iconv-lite "0.6.3" -whatwg-fetch@>=0.10.0: +whatwg-fetch@3.6.2, whatwg-fetch@>=0.10.0: version "3.6.2" resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.6.2.tgz#dced24f37f2624ed0281725d51d0e2e3fe677f8c" integrity sha512-bJlen0FcuU/0EMLrdbJ7zOnW6ITZLrZMIarMUVmdKtsGvZna8vxKYaexICWPfZ8qwf9fzNq+UEIZrnSaApt6RA== From a7c103dae79229c6a35642a999d676c845042071 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Jan 2023 17:58:19 +0000 Subject: [PATCH 2/6] Fix tests on node 10 --- packages/nextjs/jest.config.js | 1 - packages/nextjs/package.json | 3 +-- .../nextjs/test/edge/edgeWrapperUtils.test.ts | 24 +++++++++++++++++++ packages/nextjs/test/setupUnitTests.ts | 1 - yarn.lock | 2 +- 5 files changed, 26 insertions(+), 5 deletions(-) delete mode 100644 packages/nextjs/test/setupUnitTests.ts diff --git a/packages/nextjs/jest.config.js b/packages/nextjs/jest.config.js index a66844865a51..70485db447fa 100644 --- a/packages/nextjs/jest.config.js +++ b/packages/nextjs/jest.config.js @@ -5,5 +5,4 @@ module.exports = { // 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/'], - setupFiles: ['/test/setupUnitTests.ts'], }; diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 8a83479d86b1..1b5ebf5da4c1 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -32,8 +32,7 @@ "devDependencies": { "@types/webpack": "^4.41.31", "eslint-plugin-react": "^7.31.11", - "next": "10.1.3", - "whatwg-fetch": "3.6.2" + "next": "10.1.3" }, "peerDependencies": { "next": "^10.0.8 || ^11.0 || ^12.0 || ^13.0", diff --git a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts index 1f34e81aea57..1dc82049ed81 100644 --- a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts +++ b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts @@ -5,6 +5,30 @@ import { withEdgeWrapping } from '../../src/edge/utils/edgeWrapperUtils'; jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true); +// @ts-ignore Request does not exist on type Global +const origRequest = global.Request; +// @ts-ignore Response does not exist on type Global +const origResponse = global.Response; + +// @ts-ignore Request does not exist on type Global +global.Request = class Request { + headers = { + get() { + return null; + }, + }; +}; + +// @ts-ignore Response does not exist on type Global +global.Response = class Request {}; + +afterAll(() => { + // @ts-ignore Request does not exist on type Global + global.Request = origRequest; + // @ts-ignore Response does not exist on type Global + global.Response = origResponse; +}); + describe('withEdgeWrapping', () => { it('should return a function that calls the passed function', async () => { const origFunctionReturnValue = new Response(); diff --git a/packages/nextjs/test/setupUnitTests.ts b/packages/nextjs/test/setupUnitTests.ts deleted file mode 100644 index 754f5df863af..000000000000 --- a/packages/nextjs/test/setupUnitTests.ts +++ /dev/null @@ -1 +0,0 @@ -import 'whatwg-fetch'; // polyfill fetch/Request/Response globals which edge routes need diff --git a/yarn.lock b/yarn.lock index dd3dd1b862d6..c2dbdbaef239 100644 --- a/yarn.lock +++ b/yarn.lock @@ -25053,7 +25053,7 @@ whatwg-encoding@^2.0.0: dependencies: iconv-lite "0.6.3" -whatwg-fetch@3.6.2, whatwg-fetch@>=0.10.0: +whatwg-fetch@>=0.10.0: version "3.6.2" resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.6.2.tgz#dced24f37f2624ed0281725d51d0e2e3fe677f8c" integrity sha512-bJlen0FcuU/0EMLrdbJ7zOnW6ITZLrZMIarMUVmdKtsGvZna8vxKYaexICWPfZ8qwf9fzNq+UEIZrnSaApt6RA== From 924850f6362b4a9fcb2bf91c3f81f48eefa1fa6f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Jan 2023 09:19:07 +0000 Subject: [PATCH 3/6] s/spanLabel/spanDescription/ --- packages/nextjs/src/edge/utils/edgeWrapperUtils.ts | 6 +++--- packages/nextjs/src/edge/withSentryAPI.ts | 2 +- packages/nextjs/src/edge/withSentryMiddleware.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts index 5ccda34a6ddf..89c11eef5c54 100644 --- a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts @@ -17,7 +17,7 @@ import { flush } from './flush'; */ export function withEdgeWrapping( handler: H, - options: { spanLabel: string; spanOp: string; mechanismFunctionName: string }, + options: { spanDescription: string; spanOp: string; mechanismFunctionName: string }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args) { const req = args[0]; @@ -29,7 +29,7 @@ export function withEdgeWrapping( if (hasTracingEnabled()) { if (prevSpan) { span = prevSpan.startChild({ - description: options.spanLabel, + description: options.spanDescription, op: options.spanOp, }); } else if (req instanceof Request) { @@ -46,7 +46,7 @@ export function withEdgeWrapping( span = startTransaction( { - name: options.spanLabel, + name: options.spanDescription, op: options.spanOp, ...traceparentData, metadata: { diff --git a/packages/nextjs/src/edge/withSentryAPI.ts b/packages/nextjs/src/edge/withSentryAPI.ts index 26e567903acf..dbd9cc88ec55 100644 --- a/packages/nextjs/src/edge/withSentryAPI.ts +++ b/packages/nextjs/src/edge/withSentryAPI.ts @@ -16,7 +16,7 @@ export function withSentryAPI( const isCalledByUser = getCurrentHub().getScope()?.getTransaction(); const wrappedHandler = withEdgeWrapping(handler, { - spanLabel: + spanDescription: isCalledByUser || !(req instanceof Request) ? `handler (${parameterizedRoute})` : `${req.method} ${parameterizedRoute}`, diff --git a/packages/nextjs/src/edge/withSentryMiddleware.ts b/packages/nextjs/src/edge/withSentryMiddleware.ts index 5bc2480eacb2..74dd6619ed62 100644 --- a/packages/nextjs/src/edge/withSentryMiddleware.ts +++ b/packages/nextjs/src/edge/withSentryMiddleware.ts @@ -8,7 +8,7 @@ export function withSentryMiddleware( middleware: H, ): (...params: Parameters) => Promise> { return withEdgeWrapping(middleware, { - spanLabel: 'middleware', + spanDescription: 'middleware', spanOp: 'middleware.nextjs', mechanismFunctionName: 'withSentryMiddleware', }); From 1835aae13596bb1da4be073262164bf90d719211 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Jan 2023 09:22:34 +0000 Subject: [PATCH 4/6] Pass in mechanism processor to captureException scope --- .../nextjs/src/edge/utils/edgeWrapperUtils.ts | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts index 89c11eef5c54..4727c1867d9c 100644 --- a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts @@ -77,20 +77,22 @@ export function withEdgeWrapping( // store a seen flag on it. const objectifiedErr = objectify(e); - currentScope?.addEventProcessor(event => { - addExceptionMechanism(event, { - type: 'instrument', - handled: false, - data: { - function: options.mechanismFunctionName, - }, - }); - return event; - }); - span?.setStatus('internal_error'); - captureException(objectifiedErr); + captureException(objectifiedErr, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, + }); + return event; + }); + + return scope; + }); throw objectifiedErr; } finally { From e40883a294a37f5d6e9ab11db5b01192fac0444e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Jan 2023 09:23:07 +0000 Subject: [PATCH 5/6] Fix tests --- packages/nextjs/test/edge/edgeWrapperUtils.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts index 1dc82049ed81..5335c9e7c7e1 100644 --- a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts +++ b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts @@ -35,7 +35,7 @@ describe('withEdgeWrapping', () => { const origFunction = jest.fn(_req => origFunctionReturnValue); const wrappedFunction = withEdgeWrapping(origFunction, { - spanLabel: 'some label', + spanDescription: 'some label', mechanismFunctionName: 'some name', spanOp: 'some op', }); @@ -54,7 +54,7 @@ describe('withEdgeWrapping', () => { }); const wrappedFunction = withEdgeWrapping(origFunction, { - spanLabel: 'some label', + spanDescription: 'some label', mechanismFunctionName: 'some name', spanOp: 'some op', }); @@ -70,7 +70,7 @@ describe('withEdgeWrapping', () => { const origFunction = jest.fn(_req => origFunctionReturnValue); const wrappedFunction = withEdgeWrapping(origFunction, { - spanLabel: 'some label', + spanDescription: 'some label', mechanismFunctionName: 'some name', spanOp: 'some op', }); @@ -89,7 +89,7 @@ describe('withEdgeWrapping', () => { const origFunction = jest.fn(() => origFunctionReturnValue); const wrappedFunction = withEdgeWrapping(origFunction, { - spanLabel: 'some label', + spanDescription: 'some label', mechanismFunctionName: 'some name', spanOp: 'some op', }); From 76e4815b145da9cbe2de24256d6868e7636d72a5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Jan 2023 10:54:15 +0000 Subject: [PATCH 6/6] Add unit tests --- packages/nextjs/src/edge/withSentryAPI.ts | 6 +- .../nextjs/test/edge/edgeWrapperUtils.test.ts | 8 +- .../nextjs/test/edge/withSentryAPI.test.ts | 95 +++++++++++++++++++ 3 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 packages/nextjs/test/edge/withSentryAPI.test.ts diff --git a/packages/nextjs/src/edge/withSentryAPI.ts b/packages/nextjs/src/edge/withSentryAPI.ts index dbd9cc88ec55..d18deac1c3c4 100644 --- a/packages/nextjs/src/edge/withSentryAPI.ts +++ b/packages/nextjs/src/edge/withSentryAPI.ts @@ -13,14 +13,14 @@ export function withSentryAPI( return async function (this: unknown, ...args: Parameters): Promise> { const req = args[0]; - const isCalledByUser = getCurrentHub().getScope()?.getTransaction(); + const activeSpan = !!getCurrentHub().getScope()?.getSpan(); const wrappedHandler = withEdgeWrapping(handler, { spanDescription: - isCalledByUser || !(req instanceof Request) + activeSpan || !(req instanceof Request) ? `handler (${parameterizedRoute})` : `${req.method} ${parameterizedRoute}`, - spanOp: isCalledByUser ? 'function' : 'http.server', + spanOp: activeSpan ? 'function' : 'http.server', mechanismFunctionName: 'withSentryAPI', }); diff --git a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts index 5335c9e7c7e1..f2fd71a10a31 100644 --- a/packages/nextjs/test/edge/edgeWrapperUtils.test.ts +++ b/packages/nextjs/test/edge/edgeWrapperUtils.test.ts @@ -3,8 +3,6 @@ import * as sentryTracing from '@sentry/tracing'; import { withEdgeWrapping } from '../../src/edge/utils/edgeWrapperUtils'; -jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true); - // @ts-ignore Request does not exist on type Global const origRequest = global.Request; // @ts-ignore Response does not exist on type Global @@ -29,6 +27,12 @@ afterAll(() => { global.Response = origResponse; }); +beforeEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true); +}); + describe('withEdgeWrapping', () => { it('should return a function that calls the passed function', async () => { const origFunctionReturnValue = new Response(); diff --git a/packages/nextjs/test/edge/withSentryAPI.test.ts b/packages/nextjs/test/edge/withSentryAPI.test.ts new file mode 100644 index 000000000000..aacef0fa3f79 --- /dev/null +++ b/packages/nextjs/test/edge/withSentryAPI.test.ts @@ -0,0 +1,95 @@ +import * as coreSdk from '@sentry/core'; +import * as sentryTracing from '@sentry/tracing'; + +import { withSentryAPI } from '../../src/edge'; + +// @ts-ignore Request does not exist on type Global +const origRequest = global.Request; +// @ts-ignore Response does not exist on type Global +const origResponse = global.Response; + +// @ts-ignore Request does not exist on type Global +global.Request = class Request { + headers = { + get() { + return null; + }, + }; + + method = 'POST'; +}; + +// @ts-ignore Response does not exist on type Global +global.Response = class Request {}; + +afterAll(() => { + // @ts-ignore Request does not exist on type Global + global.Request = origRequest; + // @ts-ignore Response does not exist on type Global + global.Response = origResponse; +}); + +beforeEach(() => { + jest.resetAllMocks(); + jest.restoreAllMocks(); + jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true); +}); + +describe('withSentryAPI', () => { + it('should return a function that starts a transaction with the correct name when there is no active transaction and a request is being passed', async () => { + const startTransactionSpy = jest.spyOn(coreSdk, 'startTransaction'); + + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(_req => origFunctionReturnValue); + + const wrappedFunction = withSentryAPI(origFunction, '/user/[userId]/post/[postId]'); + + const request = new Request('https://sentry.io/'); + await wrappedFunction(request); + expect(startTransactionSpy).toHaveBeenCalledTimes(1); + expect(startTransactionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + metadata: { source: 'route' }, + name: 'POST /user/[userId]/post/[postId]', + op: 'http.server', + }), + { request }, + ); + }); + + it('should return a function that should not start a transaction when there is no active span and no request is being passed', async () => { + const startTransactionSpy = jest.spyOn(coreSdk, 'startTransaction'); + + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(() => origFunctionReturnValue); + + const wrappedFunction = withSentryAPI(origFunction, '/user/[userId]/post/[postId]'); + + await wrappedFunction(); + expect(startTransactionSpy).not.toHaveBeenCalled(); + }); + + it('should return a function that starts a span on the current transaction with the correct description when there is an active transaction and no request is being passed', async () => { + const testTransaction = coreSdk.startTransaction({ name: 'testTransaction' }); + coreSdk.getCurrentHub().getScope()?.setSpan(testTransaction); + + const startChildSpy = jest.spyOn(testTransaction, 'startChild'); + + const origFunctionReturnValue = new Response(); + const origFunction = jest.fn(() => origFunctionReturnValue); + + const wrappedFunction = withSentryAPI(origFunction, '/user/[userId]/post/[postId]'); + + await wrappedFunction(); + expect(startChildSpy).toHaveBeenCalledTimes(1); + expect(startChildSpy).toHaveBeenCalledWith( + expect.objectContaining({ + description: 'handler (/user/[userId]/post/[postId])', + op: 'function', + }), + ); + + testTransaction.finish(); + coreSdk.getCurrentHub().getScope()?.setSpan(undefined); + }); +});