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

fix(nuxt): Use Nuxt error hooks instead of errorHandler to prevent 500 #13748

Merged
merged 3 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -5,6 +5,10 @@ const props = defineProps({
errorText: {
type: String,
required: true
},
id: {
type: String,
required: true
}
})
Expand All @@ -14,5 +18,5 @@ const triggerError = () => {
</script>

<template>
<button id="errorBtn" @click="triggerError">Trigger Error</button>
<button :id="props.id" @click="triggerError">Trigger Error</button>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import ErrorButton from '../components/ErrorButton.vue';
</script>

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


Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<p>{{ $route.params.param }} - {{ $route.params.param }}</p>

<ErrorButton errorText="Error thrown from Param Route Button" />
<ErrorButton id="errorBtn" errorText="Error thrown from Param Route Button" />
<button @click="fetchData">Fetch Server Data</button>
</template>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,51 @@ test.describe('client-side errors', async () => {
},
});
});

test('page is still interactive after client error', async ({ page }) => {
const error1Promise = waitForError('nuxt-3', async errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Error thrown from Nuxt-3 E2E test app';
});

await page.goto(`/client-error`);
await page.locator('#errorBtn').click();

const error1 = await error1Promise;

const error2Promise = waitForError('nuxt-3', async errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Another Error thrown from Nuxt-3 E2E test app';
});

await page.locator('#errorBtn2').click();

const error2 = await error2Promise;

expect(error1).toMatchObject({
exception: {
values: [
{
type: 'Error',
value: 'Error thrown from Nuxt-3 E2E test app',
mechanism: {
handled: false,
},
},
],
},
});

expect(error2).toMatchObject({
exception: {
values: [
{
type: 'Error',
value: 'Another Error thrown from Nuxt-3 E2E test app',
mechanism: {
handled: false,
},
},
],
},
});
});
});
14 changes: 13 additions & 1 deletion 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 { browserTracingIntegration, vueIntegration } from '@sentry/vue';
import { defineNuxtPlugin } from 'nuxt/app';
import { reportNuxtError } from '../utils';

// --- 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.
Expand Down Expand Up @@ -49,8 +50,19 @@ export default defineNuxtPlugin({
const sentryClient = getClient();

if (sentryClient) {
sentryClient.addIntegration(vueIntegration({ app: vueApp }));
// 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 }));
}
});

nuxtApp.hook('app:error', error => {
reportNuxtError({ error });
});

nuxtApp.hook('vue:error', (error, instance, info) => {
reportNuxtError({ error, instance, info });
});
},
});
43 changes: 41 additions & 2 deletions packages/nuxt/src/runtime/utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { getTraceMetaTags } from '@sentry/core';
import type { Context } from '@sentry/types';
import { captureException, getClient, getTraceMetaTags } from '@sentry/core';
import type { ClientOptions, Context } from '@sentry/types';
import { dropUndefinedKeys } from '@sentry/utils';
import type { VueOptions } from '@sentry/vue/src/types';
import type { CapturedErrorContext } from 'nitropack';
import type { NuxtRenderHTMLContext } from 'nuxt/app';
import type { ComponentPublicInstance } from 'vue';

/**
* Extracts the relevant context information from the error context (H3Event in Nitro Error)
Expand Down Expand Up @@ -41,3 +43,40 @@ export function addSentryTracingMetaTags(head: NuxtRenderHTMLContext['head']): v
head.push(metaTags);
}
}

/**
* Reports an error to Sentry. This function is similar to `attachErrorHandler` in `@sentry/vue`.
* The Nuxt SDK does not register an error handler, but uses the Nuxt error hooks to report errors.
*
* We don't want to use the error handling from `@sentry/vue` as it wraps the existing error handler, which leads to a 500 error: https://github.com/getsentry/sentry-javascript/issues/12515
*/
export function reportNuxtError(options: {
error: unknown;
instance?: ComponentPublicInstance | null;
info?: string;
}): void {
const { error, instance, info } = options;

const metadata: Record<string, unknown> = {
info,
// todo: add component name and trace (like in the vue integration)
};

if (instance && instance.$props) {
const sentryClient = getClient();
const sentryOptions = sentryClient ? (sentryClient.getOptions() as ClientOptions & VueOptions) : null;

// `attachProps` is enabled by default and props should only not be attached if explicitly disabled (see DEFAULT_CONFIG in `vueIntegration`).
if (sentryOptions && sentryOptions.attachProps && instance.$props !== false) {
metadata.propsData = instance.$props;
}
}

// Capture exception in the next event loop, to make sure that all breadcrumbs are recorded in time.
setTimeout(() => {
captureException(error, {
captureContext: { contexts: { nuxt: metadata } },
mechanism: { handled: false },
});
});
}
76 changes: 74 additions & 2 deletions packages/nuxt/test/runtime/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, expect, it } from 'vitest';
import { extractErrorContext } from '../../src/runtime/utils';
import { captureException, getClient } from '@sentry/core';
import { type Mock, afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest';
import type { ComponentPublicInstance } from 'vue';
import { extractErrorContext, reportNuxtError } from '../../src/runtime/utils';

describe('extractErrorContext', () => {
it('returns empty object for undefined or empty context', () => {
Expand Down Expand Up @@ -77,3 +79,73 @@ describe('extractErrorContext', () => {
expect(() => extractErrorContext(weirdContext3)).not.toThrow();
});
});

describe('reportNuxtError', () => {
vi.mock('@sentry/core', () => ({
captureException: vi.fn(),
getClient: vi.fn(),
}));

const mockError = new Error('Test error');

const mockInstance: ComponentPublicInstance = {
$props: { foo: 'bar' },
} as any;

const mockClient = {
getOptions: vi.fn().mockReturnValue({ attachProps: true }),
};

beforeEach(() => {
// Using fake timers as setTimeout is used in `reportNuxtError`
vi.useFakeTimers();
vi.clearAllMocks();
(getClient as Mock).mockReturnValue(mockClient);
});

afterEach(() => {
vi.clearAllMocks();
});

test('captures exception with correct error and metadata', () => {
reportNuxtError({ error: mockError });
vi.runAllTimers();

expect(captureException).toHaveBeenCalledWith(mockError, {
captureContext: { contexts: { nuxt: { info: undefined } } },
mechanism: { handled: false },
});
});

test('includes instance props if attachProps is not explicitly defined', () => {
reportNuxtError({ error: mockError, instance: mockInstance });
vi.runAllTimers();

expect(captureException).toHaveBeenCalledWith(mockError, {
captureContext: { contexts: { nuxt: { info: undefined, propsData: { foo: 'bar' } } } },
mechanism: { handled: false },
});
});

test('does not include instance props if attachProps is disabled', () => {
mockClient.getOptions.mockReturnValue({ attachProps: false });

reportNuxtError({ error: mockError, instance: mockInstance });
vi.runAllTimers();

expect(captureException).toHaveBeenCalledWith(mockError, {
captureContext: { contexts: { nuxt: { info: undefined } } },
mechanism: { handled: false },
});
});

test('handles absence of instance correctly', () => {
reportNuxtError({ error: mockError, info: 'Some info' });
vi.runAllTimers();

expect(captureException).toHaveBeenCalledWith(mockError, {
captureContext: { contexts: { nuxt: { info: 'Some info' } } },
mechanism: { handled: false },
});
});
});
7 changes: 4 additions & 3 deletions packages/vue/src/errorhandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { formatComponentName, generateComponentTrace } from './vendor/components
type UnknownFunc = (...args: unknown[]) => void;

export const attachErrorHandler = (app: Vue, options: VueOptions): void => {
const { errorHandler, warnHandler, silent } = app.config;
const { errorHandler: originalErrorHandler, warnHandler, silent } = app.config;

app.config.errorHandler = (error: Error, vm: ViewModel, lifecycleHook: string): void => {
const componentName = formatComponentName(vm, false);
Expand Down Expand Up @@ -36,8 +36,9 @@ export const attachErrorHandler = (app: Vue, options: VueOptions): void => {
});
});

if (typeof errorHandler === 'function') {
(errorHandler as UnknownFunc).call(app, error, vm, lifecycleHook);
// Check if the current `app.config.errorHandler` is explicitly set by the user before calling it.
if (typeof originalErrorHandler === 'function' && app.config.errorHandler) {
(originalErrorHandler as UnknownFunc).call(app, error, vm, lifecycleHook);
Comment on lines +39 to +41
Copy link
Member

Choose a reason for hiding this comment

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

l: I guess technically, we don't need this check here for Nuxt, right? Given we don't attach the error handler anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this part is not related to Nuxt anymore. But I thought it still make sense to check if currently there is an error handler attached.

}

if (options.logErrors) {
Expand Down
5 changes: 4 additions & 1 deletion packages/vue/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const DEFAULT_CONFIG: VueOptions = {
Vue: globalWithVue.Vue,
attachProps: true,
logErrors: true,
attachErrorHandler: true,
hooks: DEFAULT_HOOKS,
timeout: 2000,
trackComponents: false,
Expand Down Expand Up @@ -76,7 +77,9 @@ const vueInit = (app: Vue, options: Options): void => {
}
}

attachErrorHandler(app, options);
if (options.attachErrorHandler) {
attachErrorHandler(app, options);
}

if (hasTracingEnabled(options)) {
app.mixin(
Expand Down
11 changes: 11 additions & 0 deletions packages/vue/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ export interface VueOptions extends TracingOptions {
*/
logErrors: boolean;

/**
* By default, Sentry attaches an error handler to capture exceptions and report them to Sentry.
* When `attachErrorHandler` is set to `false`, automatic error reporting is disabled.
*
* Usually, this option should stay enabled, unless you want to set up Sentry error reporting yourself.
* For example, the Sentry Nuxt SDK does not attach an error handler as it has its using the error hooks provided by Nuxt.
s1gr1d marked this conversation as resolved.
Show resolved Hide resolved
*
* @default true
*/
attachErrorHandler: boolean;

/** {@link TracingOptions} */
tracingOptions?: Partial<TracingOptions>;
}
Expand Down
Loading