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): Expose vueIntegration #14526

Merged
merged 4 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -6,5 +6,11 @@ Sentry.init({
dsn: useRuntimeConfig().public.sentry.dsn,
tunnel: `http://localhost:3031/`, // proxy server
tracesSampleRate: 1.0,
trackComponents: true,
integrations: [
Sentry.vueIntegration({
tracingOptions: {
trackComponents: true,
},
}),
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,11 @@ Sentry.init({
dsn: useRuntimeConfig().public.sentry.dsn,
tunnel: `http://localhost:3031/`, // proxy server
tracesSampleRate: 1.0,
trackComponents: true,
integrations: [
Sentry.vueIntegration({
tracingOptions: {
trackComponents: true,
},
}),
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ Sentry.init({
dsn: useRuntimeConfig().public.sentry.dsn,
tunnel: `http://localhost:3031/`, // proxy server
tracesSampleRate: 1.0,
trackComponents: true,
integrations: [
Sentry.piniaIntegration(usePinia(), {
actionTransformer: action => `Transformed: ${action}`,
Expand All @@ -15,5 +14,10 @@ Sentry.init({
...state,
}),
}),
Sentry.vueIntegration({
tracingOptions: {
trackComponents: true,
},
}),
],
});
1 change: 1 addition & 0 deletions packages/nuxt/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from '@sentry/vue';

export { init } from './sdk';
export { piniaIntegration } from './piniaIntegration';
export { vueIntegration } from './vueIntegration';
36 changes: 36 additions & 0 deletions packages/nuxt/src/client/vueIntegration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { GLOBAL_OBJ, defineIntegration } from '@sentry/core';
import type { VueIntegrationOptions } from '@sentry/vue';

type Options = Omit<
VueIntegrationOptions,
| 'app'
| 'Vue'
// TODO(v9): Should be removed from parent type so we can remove it here
| 'hooks'
// TODO(v9): Should be removed from parent type so we can remove it here
| 'timeout'
// TODO(v9): Should be removed from parent type so we can remove it here
| 'trackComponents'
>;

// Since the options object needs to cross the boundary between some builds (i.e. the nuxt module build and our client
// SDK build) we cannot use a getter that is exported from here. Instead we'll pass the options object through a global
// to the module.
export type GlobalObjWithIntegrationOptions = { _sentryNuxtVueIntegrationOptions?: Options };

// The vue integration is actually set up in the Sentry Client Module. There it is set up as soon as the nuxt app object is available.
// However, we need to export the vueIntegration from the Client SDK. This means all this integration does is store away
// its options for the Sentry Client Module to pick them up when initializing the actual vueIntegration.

/**
* Add additional error and span instrumentation specialized for Vue.
*/
export const vueIntegration = defineIntegration((options: Options = {}) => {
return {
// NOTE: This name is different from the original vueIntegration's name.
name: 'NuxtVueIntegration',
setup() {
(GLOBAL_OBJ as GlobalObjWithIntegrationOptions)._sentryNuxtVueIntegrationOptions = options;
},
};
});
12 changes: 10 additions & 2 deletions packages/nuxt/src/runtime/plugins/sentry.client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getClient } from '@sentry/core';
import { GLOBAL_OBJ, getClient } from '@sentry/core';
import { browserTracingIntegration, vueIntegration } from '@sentry/vue';
import { defineNuxtPlugin } from 'nuxt/app';
import type { GlobalObjWithIntegrationOptions } from '../../client/vueIntegration';
import { reportNuxtError } from '../utils';

// --- Types are copied from @sentry/vue (so it does not need to be exported) ---
Expand Down Expand Up @@ -53,7 +54,14 @@ export default defineNuxtPlugin({
// Adding the Vue integration without the Vue error handler
// Nuxt is registering their own error handler, which is unset after hydration: https://github.com/nuxt/nuxt/blob/d3fdbcaac6cf66d21e25d259390d7824696f1a87/packages/nuxt/src/app/entry.ts#L64-L73
// We don't want to wrap the existing error handler, as it leads to a 500 error: https://github.com/getsentry/sentry-javascript/issues/12515
sentryClient.addIntegration(vueIntegration({ app: vueApp, attachErrorHandler: false }));
sentryClient.addIntegration(
vueIntegration({
// We pick up the options from the "fake" vueIntegration exported by the SDK.
...(GLOBAL_OBJ as GlobalObjWithIntegrationOptions)._sentryNuxtVueIntegrationOptions,
app: vueApp,
attachErrorHandler: false,
}),
);
}
Comment on lines +58 to +64
Copy link
Member

Choose a reason for hiding this comment

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

Just a question: If users add a vueIntegration as well, can this even be another another time? I remember it's not possible to add the same integration twice.

What we could do is removing this addIntegration part here and add the vueIntegration as default integration in the default integration array (users can overwrite those integrations). As we need to get the vueApp in the integration, we can get it from window.useNuxtApp().vueApp.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason why I immediately went with this solution is because I thought the vueApp is initialized rather late - at least later than Sentry.init() is called. In that case, we would initialize the SDK (and call the integrations' setup method) before window.useNuxtApp() is even available. Is my concern valid? I think you know more than me in this regard.

If users add a vueIntegration as well

I don't think I fully understand this question. Can you clarify a bit?

How this PR is currently implemented, the configuration added by the user would "win" and their options are used when we add the vueIntegration() in the nuxt plugin setup hook.

The entire point of this PR is that users can add their own vueIntegration(). Maybe I am also misunderstanding. In case you are worried about "us adding the integration twice", that is actually not the case in this PR - the name between the @sentry/nuxt vueIntegration() and the @sentry/vue integration differs slightly, so they will both be set up.

Copy link
Member

Choose a reason for hiding this comment

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

Is my concern valid?

In the client-case, the Nuxt app is initialized before Sentry. I haven't tried so far but it should work in theory.

integration differs slightly

True, I wasn't thinking about that...those are two different integrations and not the same. And the new Nuxt vueIntegration is only setting the options, the logic is inside Vue vueIntegration which will still be added but with the added options? I understood it now :D
All good - I should have read the comment in the integration

});

Expand Down
1 change: 1 addition & 0 deletions packages/vue/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export { browserTracingIntegration } from './browserTracingIntegration';
export { attachErrorHandler } from './errorhandler';
export { createTracingMixins } from './tracing';
export { vueIntegration } from './integration';
export type { VueIntegrationOptions } from './integration';
export { createSentryPiniaPlugin } from './pinia';
2 changes: 2 additions & 0 deletions packages/vue/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const DEFAULT_CONFIG: VueOptions = {

const INTEGRATION_NAME = 'Vue';

export type VueIntegrationOptions = Partial<VueOptions>;

export const vueIntegration = defineIntegration((integrationOptions: Partial<VueOptions> = {}) => {
return {
name: INTEGRATION_NAME,
Expand Down
Loading