Skip to content
Closed
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
74 changes: 51 additions & 23 deletions packages/vue/src/errorhandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,31 @@ import { formatComponentName, generateComponentTrace } from './vendor/components

type UnknownFunc = (...args: unknown[]) => void;

/**
* Captures an exception with Vue component metadata.
*
* This can be used from a Vue `onErrorCaptured` hook when the automatic error handler is disabled or when an error
* boundary stops the error from propagating to the application-level handler.
*
* @param handled - Whether the boundary stops the error from propagating.
*/
export const captureVueException = (
error: unknown,
vm: ViewModel | null,
lifecycleHook: string,
handled: boolean,
options?: Partial<VueOptions>,
): void => {
Comment thread
vansh-nagar marked this conversation as resolved.
captureVueExceptionWithMechanism(error, vm, lifecycleHook, handled, options);
};

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

Comment thread
vansh-nagar marked this conversation as resolved.
app.config.errorHandler = (error: Error, vm: ViewModel, lifecycleHook: string): void => {
const componentName = formatComponentName(vm, false);
const trace = vm ? generateComponentTrace(vm) : '';
const metadata: Record<string, unknown> = {
componentName,
lifecycleHook,
trace,
};

if (options?.attachProps !== false && vm) {
// Vue2 - $options.propsData
// Vue3 - $props
if (vm.$options?.propsData) {
metadata.propsData = vm.$options.propsData;
} else if (vm.$props) {
metadata.propsData = vm.$props;
}
}

app.config.errorHandler = (error: unknown, vm: ViewModel | null, lifecycleHook: string): void => {
// Capture exception in the next event loop, to make sure that all breadcrumbs are recorded in time.
setTimeout(() => {
captureException(error, {
captureContext: { contexts: { vue: metadata } },
mechanism: { handled: !!originalErrorHandler, type: 'auto.function.vue.error_handler' },
});
captureVueExceptionWithMechanism(error, vm, lifecycleHook, !!originalErrorHandler, options);
});
Comment on lines 29 to 32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Metadata computation is now deferred via setTimeout, which can lead to capturing stale or incomplete component information if the component tree changes before the timer fires.
Severity: LOW

Suggested Fix

Capture the component metadata synchronously when the error is first handled, before scheduling the setTimeout. Pass the collected metadata object into the setTimeout callback. This ensures the metadata reflects the component's state at the exact moment of the error, restoring the previous, more reliable behavior.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/vue/src/errorhandler.ts#L29-L32

Potential issue: The automatic error handler in `attachErrorHandler` now defers the
computation of Vue component metadata to a `setTimeout` callback. Previously, this
metadata was captured synchronously when an error occurred. This change introduces a
potential race condition. If a component's state changes or it unmounts between the
error event and the execution of the `setTimeout` callback on the next event loop tick,
the captured metadata could be stale or incomplete. This can result in a less accurate
component trace, as the logic relies on walking the `$parent` chain which might be
altered, degrading the quality of the error report.


// Check if the current `app.config.errorHandler` is explicitly set by the user before calling it.
Expand All @@ -42,3 +39,34 @@ export const attachErrorHandler = (app: Vue, options?: Partial<VueOptions>): voi
}
};
};

function captureVueExceptionWithMechanism(
error: unknown,
vm: ViewModel | null,
lifecycleHook: string,
handled: boolean,
options?: Partial<VueOptions>,
): void {
const componentName = formatComponentName(vm || undefined, false);
const trace = vm ? generateComponentTrace(vm) : '';
const metadata: Record<string, unknown> = {
componentName,
lifecycleHook,
trace,
};

if (options?.attachProps !== false && vm) {
// Vue2 - $options.propsData
// Vue3 - $props
if (vm.$options?.propsData) {
metadata.propsData = vm.$options.propsData;
} else if (vm.$props) {
metadata.propsData = vm.$props;
}
}

captureException(error, {
captureContext: { contexts: { vue: metadata } },
mechanism: { handled, type: 'auto.function.vue.error_handler' },
});
}
3 changes: 2 additions & 1 deletion packages/vue/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ export * from '@sentry/browser';

export { init } from './sdk';
export { browserTracingIntegration } from './browserTracingIntegration';
export { attachErrorHandler } from './errorhandler';
export { attachErrorHandler, captureVueException } from './errorhandler';
export type { ViewModel } from './types';
export { createTracingMixins } from './tracing';
export { vueIntegration } from './integration';
export type { VueIntegrationOptions } from './integration';
Expand Down
6 changes: 3 additions & 3 deletions packages/vue/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ export interface Vue {
export type ViewModel = {
_isVue?: boolean;
__isVue?: boolean;
$root: ViewModel;
$parent?: ViewModel;
$props: { [key: string]: any };
$root?: ViewModel | null;
$parent?: ViewModel | null;
$props?: { [key: string]: any };
$options?: {
name?: string;
propsData?: { [key: string]: any };
Expand Down
4 changes: 2 additions & 2 deletions packages/vue/src/vendor/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const repeat = (str: string, n: number): string => {
return str.repeat(n);
};

export const formatComponentName = (vm?: ViewModel, includeFile?: boolean): string => {
export const formatComponentName = (vm?: ViewModel | null, includeFile?: boolean): string => {
if (!vm) {
return ANONYMOUS_COMPONENT_NAME;
}
Expand Down Expand Up @@ -46,7 +46,7 @@ export const formatComponentName = (vm?: ViewModel, includeFile?: boolean): stri
);
};

export const generateComponentTrace = (vm?: ViewModel): string => {
export const generateComponentTrace = (vm?: ViewModel | null): string => {
if (vm && (vm._isVue || vm.__isVue) && vm.$parent) {
const tree = [];
let currentRecursiveSequence = 0;
Expand Down
34 changes: 33 additions & 1 deletion packages/vue/test/errorHandler.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { setCurrentClient } from '@sentry/browser';
import { afterEach, describe, expect, it, test, vi } from 'vitest';
import { attachErrorHandler } from '../src/errorhandler';
import { attachErrorHandler, captureVueException } from '../src/errorhandler';
import type { Operation, Options, ViewModel, Vue } from '../src/types';

describe('attachErrorHandler', () => {
Expand Down Expand Up @@ -231,6 +231,38 @@ describe('attachErrorHandler', () => {
});
});

describe('captureVueException', () => {
it.each([true, false])('captures an exception synchronously with handled=%s', handled => {
const captureException = vi.fn();
setCurrentClient({ captureException } as any);
const error = new DummyError();
const vm = {
$options: { name: 'error-boundary' },
$props: { source: 'checkout' },
} as ViewModel;

captureVueException(error, vm, 'render', handled);

expect(captureException).toHaveBeenCalledWith(
error,
expect.objectContaining({
captureContext: {
contexts: {
vue: {
componentName: '<ErrorBoundary>',
lifecycleHook: 'render',
propsData: { source: 'checkout' },
trace: '\n\n(found in <ErrorBoundary>)',
},
},
},
mechanism: { handled, type: 'auto.function.vue.error_handler' },
}),
expect.anything(),
);
});
});
Comment thread
vansh-nagar marked this conversation as resolved.

type TestHarnessOpts = {
// I don't need everything in the tests
vm: Partial<ViewModel> | null;
Expand Down
67 changes: 67 additions & 0 deletions packages/vue/test/integration/errorHandler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* @vitest-environment jsdom
*/

import type { Scope } from '@sentry/core';
import { setCurrentClient } from '@sentry/browser';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { createApp, defineComponent, h, onErrorCaptured } from 'vue';
import { captureVueException, withScope } from '../../src';

describe('captureVueException', () => {
afterEach(() => {
vi.resetAllMocks();
});

it('captures a Vue error boundary exception with its local scope', () => {
const error = 'render failed';
const captureException = vi.fn((_error: unknown, _hint: unknown, scope?: Scope) => {
expect(scope?.getScopeData().tags).toEqual({ boundary: 'checkout' });
});
setCurrentClient({ captureException } as any);

const child = defineComponent({
name: 'Checkout',
setup() {
throw error;
},
render: () => h('div'),
});
const boundary = defineComponent({
name: 'ErrorBoundary',
setup() {
onErrorCaptured((caughtError, instance, info) => {
withScope(scope => {
scope.setTag('boundary', 'checkout');
captureVueException(caughtError, instance, info, true);
});

expect(captureException).toHaveBeenCalledTimes(1);
return false;
});

return () => h(child);
},
});
const app = createApp(boundary);

app.mount(document.createElement('div'));

expect(captureException).toHaveBeenCalledWith(
error,
expect.objectContaining({
captureContext: {
contexts: {
vue: expect.objectContaining({
componentName: '<Checkout>',
lifecycleHook: 'setup function',
}),
},
},
mechanism: { handled: true, type: 'auto.function.vue.error_handler' },
}),
expect.anything(),
);
app.unmount();
});
});