diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index fd070afa8ca3..e2a1b46f8b2e 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -1,6 +1,5 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ import { captureException } from '@sentry/browser'; -import { Transaction, TransactionContext } from '@sentry/types'; +import { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; export type VueRouterInstrumentation = ( startTransaction: (context: TransactionContext) => T | undefined, @@ -8,14 +7,24 @@ export type VueRouterInstrumentation = ( startTransactionOnLocationChange?: boolean, ) => void; -// This is not great, but kinda necessary to make it work with VueRouter@3 and VueRouter@4 at the same time. -type Route = { - params: any; - query: any; - name?: any; - path: any; - matched: any[]; +// The following type is an intersection of the Route type from VueRouter v2, v3, and v4. +// This is not great, but kinda necessary to make it work with all versions at the same time. +export type Route = { + /** Unparameterized URL */ + path: string; + /** + * Query params (keys map to null when there is no value associated, e.g. "?foo" and to an array when there are + * multiple query params that have the same key, e.g. "?foo&foo=bar") + */ + query: Record; + /** Route name (VueRouter provides a way to give routes individual names) */ + name?: string | symbol | null | undefined; + /** Evaluated parameters */ + params: Record; + /** All the matched route objects as defined in VueRouter constructor */ + matched: { path: string }[]; }; + interface VueRouter { onError: (fn: (err: Error) => void) => void; beforeEach: (fn: (to: Route, from: Route, next: () => void) => void) => void; @@ -39,8 +48,10 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument // https://router.vuejs.org/api/#router-start-location // https://next.router.vuejs.org/api/#start-location - // Vue2 - null - // Vue3 - undefined + // from.name: + // - Vue 2: null + // - Vue 3: undefined + // hence only '==' instead of '===', because `undefined == null` evaluates to `true` const isPageLoadNavigation = from.name == null && from.matched.length === 0; const tags = { @@ -51,23 +62,38 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument query: to.query, }; + // Determine a name for the routing transaction and where that name came from + let transactionName: string = to.path; + let transactionSource: TransactionSource = 'url'; + if (to.name) { + transactionName = to.name.toString(); + transactionSource = 'custom'; + } else if (to.matched[0] && to.matched[0].path) { + transactionName = to.matched[0].path; + transactionSource = 'route'; + } + if (startTransactionOnPageLoad && isPageLoadNavigation) { startTransaction({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - name: (to.name && to.name.toString()) || to.path, + name: transactionName, op: 'pageload', tags, data, + metadata: { + source: transactionSource, + }, }); } if (startTransactionOnLocationChange && !isPageLoadNavigation) { startTransaction({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - name: (to.name && to.name.toString()) || (to.matched[0] && to.matched[0].path) || to.path, + name: transactionName, op: 'navigation', tags, data, + metadata: { + source: transactionSource, + }, }); } diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 30baf205daf5..342b58db2ebd 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -83,7 +83,7 @@ export const createTracingMixins = (options: TracingOptions): Mixins => { // Skip components that we don't want to track to minimize the noise and give a more granular control to the user const name = formatComponentName(this, false); const shouldTrack = Array.isArray(options.trackComponents) - ? options.trackComponents.includes(name) + ? options.trackComponents.indexOf(name) > -1 : options.trackComponents; // We always want to track root component diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts new file mode 100644 index 000000000000..c29aa571dae4 --- /dev/null +++ b/packages/vue/test/router.test.ts @@ -0,0 +1,163 @@ +import * as SentryBrowser from '@sentry/browser'; + +import { vueRouterInstrumentation } from '../src'; +import { Route } from '../src/router'; + +const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); + +const mockVueRouter = { + onError: jest.fn void]>(), + beforeEach: jest.fn void) => void]>(), +}; + +const mockStartTransaction = jest.fn(); +const mockNext = jest.fn(); + +const testRoutes: Record = { + initialPageloadRoute: { matched: [], params: {}, path: '', query: {} }, + normalRoute1: { + matched: [{ path: '/books/:bookId/chapter/:chapterId' }], + params: { + bookId: '12', + chapterId: '3', + }, + path: '/books/12/chapter/3', + query: { + utm_source: 'google', + }, + }, + normalRoute2: { + matched: [{ path: '/accounts/:accountId' }], + params: { + accountId: '4', + }, + path: '/accounts/4', + query: {}, + }, + namedRoute: { + matched: [{ path: '/login' }], + name: 'login-screen', + params: {}, + path: '/login', + query: {}, + }, + unmatchedRoute: { + matched: [], + params: {}, + path: '/e8733846-20ac-488c-9871-a5cbcb647294', + query: {}, + }, +}; + +describe('vueRouterInstrumentation()', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return instrumentation that instruments VueRouter.onError', () => { + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(mockStartTransaction); + + // check + expect(mockVueRouter.onError).toHaveBeenCalledTimes(1); + + const onErrorCallback = mockVueRouter.onError.mock.calls[0][0]; + + const testError = new Error(); + onErrorCallback(testError); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(testError); + }); + + it.each([ + ['initialPageloadRoute', 'normalRoute1', 'pageload', '/books/:bookId/chapter/:chapterId', 'route'], + ['normalRoute1', 'normalRoute2', 'navigation', '/accounts/:accountId', 'route'], + ['normalRoute2', 'namedRoute', 'navigation', 'login-screen', 'custom'], + ['normalRoute2', 'unmatchedRoute', 'navigation', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'], + ])( + 'should return instrumentation that instruments VueRouter.beforeEach(%s, %s)', + (fromKey, toKey, op, transactionName, transactionSource) => { + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(mockStartTransaction, true, true); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes[fromKey]; + const to = testRoutes[toKey]; + beforeEachCallback(to, from, mockNext); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenCalledWith({ + name: transactionName, + metadata: { + source: transactionSource, + }, + data: { + params: to.params, + query: to.query, + }, + op: op, + tags: { + 'routing.instrumentation': 'vue-router', + }, + }); + + expect(mockNext).toHaveBeenCalledTimes(1); + }, + ); + + test.each([ + [undefined, 1], + [false, 0], + [true, 1], + ])( + 'should return instrumentation that considers the startTransactionOnPageLoad option = %p', + (startTransactionOnPageLoad, expectedCallsAmount) => { + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(mockStartTransaction, startTransactionOnPageLoad, true); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext); + + expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount); + }, + ); + + test.each([ + [undefined, 1], + [false, 0], + [true, 1], + ])( + 'should return instrumentation that considers the startTransactionOnLocationChange option = %p', + (startTransactionOnLocationChange, expectedCallsAmount) => { + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(mockStartTransaction, true, startTransactionOnLocationChange); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + beforeEachCallback(testRoutes['normalRoute2'], testRoutes['normalRoute1'], mockNext); + + expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount); + }, + ); +});