Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(nuxt): Add vue-router instrumentation #13054

Merged
merged 5 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
<script setup>
import { defineProps } from 'vue';

const props = defineProps({
errorText: {
type: String,
required: true
}
})

const triggerError = () => {
throw new Error('Error thrown from Nuxt-3 E2E test app');
throw new Error(props.errorText);
};
</script>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ErrorButton from '../components/ErrorButton.vue';
</script>

<template>
<ErrorButton />
<ErrorButton error-text="Error thrown from Nuxt-3 E2E test app"/>
</template>


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<template>
<p>{{ $route.params.param }} - {{ $route.params.param }}</p>
<ErrorButton errorText="Error thrown from Param Route Button" />
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ test.describe('client-side errors', async () => {

const error = await errorPromise;

expect(error.transaction).toEqual('/client-error');
expect(error).toMatchObject({
exception: {
values: [
Expand All @@ -25,6 +26,33 @@ test.describe('client-side errors', async () => {
],
},
});
expect(error.transaction).toEqual('/client-error');
});

test('shows parametrized route on button error', async ({ page }) => {
const errorPromise = waitForError('nuxt-3', async errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Error thrown from Param Route Button';
});

await page.goto(`/test-param/1234`);
await page.locator('#errorBtn').click();

const error = await errorPromise;

expect(error.sdk.name).toEqual('sentry.javascript.nuxt');
expect(error.transaction).toEqual('/test-param/:param()');
expect(error.request.url).toMatch(/\/test-param\/1234/);
expect(error).toMatchObject({
exception: {
values: [
{
type: 'Error',
value: 'Error thrown from Param Route Button',
mechanism: {
handled: false,
},
},
],
},
});
});
});
9 changes: 3 additions & 6 deletions packages/nuxt/src/client/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
browserTracingIntegration,
getDefaultIntegrations as getBrowserDefaultIntegrations,
init as initBrowser,
} from '@sentry/browser';
import { getDefaultIntegrations as getBrowserDefaultIntegrations, init as initBrowser } from '@sentry/browser';
import { applySdkMetadata } from '@sentry/core';
import type { Client } from '@sentry/types';
import type { SentryNuxtOptions } from '../common/types';
Expand All @@ -14,7 +10,8 @@ import type { SentryNuxtOptions } from '../common/types';
*/
export function init(options: SentryNuxtOptions): Client | undefined {
const sentryOptions = {
defaultIntegrations: [...getBrowserDefaultIntegrations(options), browserTracingIntegration()],
/* BrowserTracing is added later with the Nuxt client plugin */
defaultIntegrations: [...getBrowserDefaultIntegrations(options)],
...options,
};

Expand Down
134 changes: 67 additions & 67 deletions packages/nuxt/src/common/types.ts
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not related - just a restructuring of the type

Original file line number Diff line number Diff line change
Expand Up @@ -5,93 +5,93 @@ export type SentryNuxtOptions = Omit<Parameters<typeof init>[0] & object, 'app'>

type SourceMapsOptions = {
/**
* Options for the Sentry Vite plugin to customize the source maps upload process.
* If this flag is `true`, and an auth token is detected, the Sentry integration will
Copy link
Member

Choose a reason for hiding this comment

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

l (sorry for the bike shedding): Just was wondering if "integration" is a good term here? IIUC users interact with our nuxt module here, correct? So I'd go with module or just SDK.

Suggested change
* If this flag is `true`, and an auth token is detected, the Sentry integration will
* If this flag is `true`, and an auth token is detected, the Sentry integration will

* automatically generate and upload source maps to Sentry during a production build.
*
* These options are always read from the `sentry` module options in the `nuxt.config.(js|ts).
* Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files.
* @default true
*/
sourceMapsUploadOptions?: {
/**
* If this flag is `true`, and an auth token is detected, the Sentry integration will
* automatically generate and upload source maps to Sentry during a production build.
*
* @default true
*/
enabled?: boolean;
enabled?: boolean;

/**
* The auth token to use when uploading source maps to Sentry.
*
* Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable.
*
* To create an auth token, follow this guide:
* @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens
*/
authToken?: string;
/**
* The auth token to use when uploading source maps to Sentry.
*
* Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable.
*
* To create an auth token, follow this guide:
* @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens
*/
authToken?: string;

/**
* The organization slug of your Sentry organization.
* Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable.
*/
org?: string;
/**
* The organization slug of your Sentry organization.
* Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable.
*/
org?: string;

/**
* The project slug of your Sentry project.
* Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable.
*/
project?: string;

/**
* If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry.
* It will not collect any sensitive or user-specific data.
*
* @default true
*/
telemetry?: boolean;

/**
* Options related to sourcemaps
*/
sourcemaps?: {
/**
* The project slug of your Sentry project.
* Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable.
* A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry.
*
* If this option is not specified, sensible defaults based on your adapter and nuxt.config.js
* setup will be used. Use this option to override these defaults, for instance if you have a
* customized build setup that diverges from Nuxt's defaults.
*
* The globbing patterns must follow the implementation of the `glob` package.
* @see https://www.npmjs.com/package/glob#glob-primer
*/
project?: string;
assets?: string | Array<string>;

/**
* If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry.
* It will not collect any sensitive or user-specific data.
* A glob or an array of globs that specifies which build artifacts should not be uploaded to Sentry.
*
* @default true
* @default [] - By default no files are ignored. Thus, all files matching the `assets` glob
* or the default value for `assets` are uploaded.
*
* The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob)
*/
telemetry?: boolean;
ignore?: string | Array<string>;

/**
* Options related to sourcemaps
* A glob or an array of globs that specifies the build artifacts that should be deleted after the artifact
* upload to Sentry has been completed.
*
* @default [] - By default no files are deleted.
*
* The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob)
*/
sourcemaps?: {
/**
* A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry.
*
* If this option is not specified, sensible defaults based on your adapter and nuxt.config.js
* setup will be used. Use this option to override these defaults, for instance if you have a
* customized build setup that diverges from Nuxt's defaults.
*
* The globbing patterns must follow the implementation of the `glob` package.
* @see https://www.npmjs.com/package/glob#glob-primer
*/
assets?: string | Array<string>;

/**
* A glob or an array of globs that specifies which build artifacts should not be uploaded to Sentry.
*
* @default [] - By default no files are ignored. Thus, all files matching the `assets` glob
* or the default value for `assets` are uploaded.
*
* The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob)
*/
ignore?: string | Array<string>;

/**
* A glob or an array of globs that specifies the build artifacts that should be deleted after the artifact
* upload to Sentry has been completed.
*
* @default [] - By default no files are deleted.
*
* The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob)
*/
filesToDeleteAfterUpload?: string | Array<string>;
};
filesToDeleteAfterUpload?: string | Array<string>;
};
};

/**
* Build options for the Sentry module. These options are used during build-time by the Sentry SDK.
*/
export type SentryNuxtModuleOptions = SourceMapsOptions & {
export type SentryNuxtModuleOptions = {
/**
* Options for the Sentry Vite plugin to customize the source maps upload process.
*
* These options are always read from the `sentry` module options in the `nuxt.config.(js|ts).
* Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files.
*/
sourceMapsUploadOptions?: SourceMapsOptions;

/**
* Enable debug functionality of the SDK during build-time.
* Enabling this will give you, for example, logs about source maps.
Expand Down
36 changes: 33 additions & 3 deletions packages/nuxt/src/runtime/plugins/sentry.client.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,41 @@
import { getClient } from '@sentry/core';
import { vueIntegration } from '@sentry/vue';
import { browserTracingIntegration, vueIntegration } from '@sentry/vue';
import { defineNuxtPlugin } from 'nuxt/app';

// --- Types are copied from @sentry/vue (so it does not need to be exported) ---
// 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.
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<string, string | null | (string | null)[]>;
/** Route name (VueRouter provides a way to give routes individual names) */
name?: string | symbol | null | undefined;
/** Evaluated parameters */
params: Record<string, string | string[]>;
/** 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;
}

export default defineNuxtPlugin(nuxtApp => {
nuxtApp.hook('app:created', vueApp => {
const sentryClient = getClient();
const sentryClient = getClient();
Copy link
Member

Choose a reason for hiding this comment

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

l: We can early return if client is not defined to remove the if (sentryClient) checks below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather leave the checks as I'm also accessing the client in a Nuxt lifecycle hook and I cannot be sure that the early return would hinder the lifecycle hook to be called if there is no client. Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

I see what you mean. I think then we need a new call to getClient inside the hook so we ensure that we always have an up-to-date reference.


if (sentryClient && '$router' in nuxtApp) {
sentryClient.addIntegration(
browserTracingIntegration({ router: nuxtApp.$router as VueRouter, routeLabel: 'path' }),
);
}
Copy link
Member

Choose a reason for hiding this comment

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

m: This is runtime code, right? If so, I think we need to add the __SENTRY_TRACING__ guard here to enable tree shaking of browserTracingIntegration.


nuxtApp.hook('app:created', vueApp => {
if (sentryClient) {
sentryClient.addIntegration(vueIntegration({ app: vueApp }));
}
Expand Down
Loading