Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(nuxt): Detect pageload by adding flag in Vue router #13171

Merged
merged 11 commits into from
Aug 5, 2024
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' */

Comment on lines +11 to +13
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this just as a heads-up in case someone adds new tests later on.

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',
},
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test } from '@playwright/test';
import { expect, test } from '@nuxt/test-utils/playwright';
import { waitForTransaction } from '@sentry-internal/test-utils';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';

Expand Down
12 changes: 11 additions & 1 deletion 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
// 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: IS_FIRST_PAGE_LOAD

// 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
Loading