Skip to content

Commit

Permalink
fix(nuxt): Detect pageload by adding flag in Vue router (#13171)
Browse files Browse the repository at this point in the history
Nuxt is using the Vue router under the hood, but the previous condition
to detect a page load (`from.name == null && from.matched.length === 0`)
does not work with Nuxt, as `from.matched` is never empty.
  • Loading branch information
s1gr1d authored Aug 5, 2024
1 parent 5f4a71c commit 9289200
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const nuxtConfigOptions: ConfigOptions = {
},
};

/* Make sure to import from '@nuxt/test-utils/playwright' in the tests
* Like this: import { expect, test } from '@nuxt/test-utils/playwright' */

const config = getPlaywrightConfig({
startCommand: `pnpm preview`,
use: { ...nuxtConfigOptions },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, test } from '@nuxt/test-utils/playwright';
import { waitForTransaction } from '@sentry-internal/test-utils';

test('sends a pageload root span with a parameterized URL', async ({ page }) => {
const transactionPromise = waitForTransaction('nuxt-3', async transactionEvent => {
return transactionEvent.transaction === '/test-param/:param()';
});

await page.goto(`/test-param/1234`);

const rootSpan = await transactionPromise;

expect(rootSpan).toMatchObject({
contexts: {
trace: {
data: {
'sentry.source': 'route',
'sentry.origin': 'auto.pageload.vue',
'sentry.op': 'pageload',
'params.param': '1234',
},
op: 'pageload',
origin: 'auto.pageload.vue',
},
},
transaction: '/test-param/:param()',
transaction_info: {
source: 'route',
},
});
});
6 changes: 4 additions & 2 deletions packages/nuxt/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { applySdkMetadata, getGlobalScope } from '@sentry/core';
import { init as initNode } from '@sentry/node';
import type { Client, EventProcessor } from '@sentry/types';
import { logger } from '@sentry/utils';
import { DEBUG_BUILD } from '../common/debug-build';
import type { SentryNuxtOptions } from '../common/types';

/**
Expand All @@ -26,8 +28,8 @@ export function init(options: SentryNuxtOptions): Client | undefined {
// todo: the buildAssetDir could be changed in the nuxt config - change this to a more generic solution
if (event.transaction?.match(/^GET \/_nuxt\//)) {
options.debug &&
// eslint-disable-next-line no-console
console.log('[Sentry] NuxtLowQualityTransactionsFilter filtered transaction: ', event.transaction);
DEBUG_BUILD &&
logger.log('NuxtLowQualityTransactionsFilter filtered transaction: ', event.transaction);
return null;
}

Expand Down
14 changes: 12 additions & 2 deletions packages/vue/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,28 @@ export function instrumentVueRouter(
},
startNavigationSpanFn: (context: StartSpanOptions) => void,
): void {
let isFirstPageLoad = true;

router.onError(error => captureException(error, { mechanism: { handled: false } }));

router.beforeEach((to, from, next) => {
// According to docs we could use `from === VueRouter.START_LOCATION` but I couldnt get it working for Vue 2
// According to docs we could use `from === VueRouter.START_LOCATION` but I couldn't get it working for Vue 2
// https://router.vuejs.org/api/#router-start-location
// https://next.router.vuejs.org/api/#start-location
// Additionally, Nuxt does not provide the possibility to check for `from.matched.length === 0` (this is never 0).
// Therefore, a flag was added to track the page-load: isFirstPageLoad

// from.name:
// - Vue 2: null
// - Vue 3: undefined
// - Nuxt: undefined
// hence only '==' instead of '===', because `undefined == null` evaluates to `true`
const isPageLoadNavigation = from.name == null && from.matched.length === 0;
const isPageLoadNavigation =
(from.name == null && from.matched.length === 0) || (from.name === undefined && isFirstPageLoad);

if (isFirstPageLoad) {
isFirstPageLoad = false;
}

const attributes: SpanAttributes = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue',
Expand Down
7 changes: 6 additions & 1 deletion packages/vue/test/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ describe('instrumentVueRouter()', () => {

const from = testRoutes[fromKey]!;
const to = testRoutes[toKey]!;
beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
beforeEachCallback(to, from, mockNext);

expect(mockStartSpan).toHaveBeenCalledTimes(1);
Expand All @@ -127,7 +128,7 @@ describe('instrumentVueRouter()', () => {
op: 'navigation',
});

expect(mockNext).toHaveBeenCalledTimes(1);
expect(mockNext).toHaveBeenCalledTimes(2);
},
);

Expand Down Expand Up @@ -192,6 +193,7 @@ describe('instrumentVueRouter()', () => {

const from = testRoutes.normalRoute1!;
const to = testRoutes.namedRoute!;
beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
beforeEachCallback(to, from, mockNext);

// first startTx call happens when the instrumentation is initialized (for pageloads)
Expand Down Expand Up @@ -219,6 +221,7 @@ describe('instrumentVueRouter()', () => {

const from = testRoutes.normalRoute1!;
const to = testRoutes.namedRoute!;
beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
beforeEachCallback(to, from, mockNext);

// first startTx call happens when the instrumentation is initialized (for pageloads)
Expand Down Expand Up @@ -373,6 +376,7 @@ describe('instrumentVueRouter()', () => {
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);

const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!;
beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
beforeEachCallback(testRoutes['normalRoute2']!, testRoutes['normalRoute1']!, mockNext);

expect(mockStartSpan).toHaveBeenCalledTimes(expectedCallsAmount);
Expand All @@ -391,6 +395,7 @@ describe('instrumentVueRouter()', () => {

const from = testRoutes.normalRoute1!;
const to = testRoutes.namedRoute!;
beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
beforeEachCallback(to, from, undefined);

// first startTx call happens when the instrumentation is initialized (for pageloads)
Expand Down

0 comments on commit 9289200

Please sign in to comment.