diff --git a/dev-packages/e2e-tests/test-applications/vue-3/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/vue-3/tests/errors.test.ts index 262cda11b366..b86e56eb4b83 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/tests/errors.test.ts @@ -19,7 +19,7 @@ test('sends an error', async ({ page }) => { type: 'Error', value: 'This is a Vue test error', mechanism: { - type: 'generic', + type: 'vue', handled: false, }, }, @@ -47,7 +47,7 @@ test('sends an error with a parameterized transaction name', async ({ page }) => type: 'Error', value: 'This is a Vue test error', mechanism: { - type: 'generic', + type: 'vue', handled: false, }, }, diff --git a/packages/vue/src/errorhandler.ts b/packages/vue/src/errorhandler.ts index 6a3951cc1855..ba7819e1f2f5 100644 --- a/packages/vue/src/errorhandler.ts +++ b/packages/vue/src/errorhandler.ts @@ -30,16 +30,22 @@ export const attachErrorHandler = (app: Vue, options: VueOptions): void => { setTimeout(() => { captureException(error, { captureContext: { contexts: { vue: metadata } }, - mechanism: { handled: false }, + mechanism: { handled: !!originalErrorHandler, type: 'vue' }, }); }); // 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); - } - - if (options.logErrors) { + } // TODO(v9): Always throw when no user-defined error handler is provided (this will log the error to the console as well). Delete the Code with logErrors below + /* else { + throw error; + } */ + + if (!originalErrorHandler) { + throw error; + // eslint-disable-next-line deprecation/deprecation + } else if (options.logErrors) { const hasConsole = typeof console !== 'undefined'; const message = `Error in ${lifecycleHook}: "${error && error.toString()}"`; diff --git a/packages/vue/src/types.ts b/packages/vue/src/types.ts index 8b23a2389e69..d5f7c05c0216 100644 --- a/packages/vue/src/types.ts +++ b/packages/vue/src/types.ts @@ -44,6 +44,8 @@ export interface VueOptions { /** * When set to `true`, original Vue's `logError` will be called as well. * https://github.com/vuejs/vue/blob/c2b1cfe9ccd08835f2d99f6ce60f67b4de55187f/src/core/util/error.js#L38-L48 + * + * @deprecated Will be removed in future versions of the SDK. The error will always be logged unless you define a custom Vue errorHandler. */ logErrors: boolean; diff --git a/packages/vue/test/errorHandler.test.ts b/packages/vue/test/errorHandler.test.ts index 273d1dfecd0e..cbfcf5ce5be8 100644 --- a/packages/vue/test/errorHandler.test.ts +++ b/packages/vue/test/errorHandler.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, describe, expect, it, test, vi } from 'vitest'; import { setCurrentClient } from '@sentry/browser'; @@ -7,26 +7,29 @@ import type { Operation, Options, ViewModel, Vue } from '../src/types'; import { generateComponentTrace } from '../src/vendor/components'; describe('attachErrorHandler', () => { - describe('attachProps', () => { + describe('attach data to captureException', () => { afterEach(() => { vi.resetAllMocks(); + // we need timers to still call captureException wrapped inside setTimeout after the error throws + vi.useRealTimers(); }); describe("given I don't want to `attachProps`", () => { test('no `propsData` is added to the metadata', () => { - // arrange const t = testHarness({ - enableErrorHandler: false, enableWarnHandler: false, attachProps: false, vm: null, + enableConsole: true, }); - // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withoutProps(); + t.expect.errorToHaveBeenCaptured().withMechanismMetadata({ handled: false, type: 'vue' }); }); }); @@ -41,10 +44,13 @@ describe('attachErrorHandler', () => { }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withoutProps(); + t.expect.errorToHaveBeenCaptured().withMechanismMetadata({ handled: false, type: 'vue' }); }); }); @@ -58,7 +64,9 @@ describe('attachErrorHandler', () => { }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withoutProps(); @@ -76,7 +84,9 @@ describe('attachErrorHandler', () => { }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withoutProps(); @@ -94,7 +104,9 @@ describe('attachErrorHandler', () => { }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withProps(props); @@ -114,7 +126,9 @@ describe('attachErrorHandler', () => { }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert t.expect.errorToHaveBeenCaptured().withProps(props); @@ -123,31 +137,22 @@ describe('attachErrorHandler', () => { }); }); }); - }); - describe('provided errorHandler', () => { - describe('given I did not provide an `errorHandler`', () => { - test('it is not called', () => { + describe('attach mechanism metadata', () => { + it('should mark error as unhandled and capture correct metadata', () => { // arrange - const t = testHarness({ - enableErrorHandler: false, - vm: { - $options: { - name: 'stub-vm', - }, - }, - }); + const t = testHarness({ vm: null }); // act - t.run(); + vi.useFakeTimers(); + expect(() => t.run()).toThrow(DummyError); + vi.runAllTimers(); // assert - t.expect.errorHandlerSpy.not.toHaveBeenCalled(); + t.expect.errorToHaveBeenCaptured().withMechanismMetadata({ handled: false, type: 'vue' }); }); - }); - describe('given I provided an `errorHandler`', () => { - test('it is called', () => { + it('should mark error as handled and properly delegate to error handler', () => { // arrange const vm = { $options: { @@ -156,6 +161,7 @@ describe('attachErrorHandler', () => { }; const t = testHarness({ enableErrorHandler: true, + enableConsole: true, vm, }); @@ -164,137 +170,102 @@ describe('attachErrorHandler', () => { // assert t.expect.errorHandlerSpy.toHaveBeenCalledWith(expect.any(Error), vm, 'stub-lifecycle-hook'); + t.expect.errorToHaveBeenCaptured().withMechanismMetadata({ handled: true, type: 'vue' }); }); }); }); - describe('error logging', () => { - describe('given I disabled error logging', () => { - describe('when an error is captured', () => { - test('it logs nothing', () => { - // arrange - const vm = { - $options: { - name: 'stub-vm', - }, - }; - const t = testHarness({ - enableWarnHandler: false, - logErrors: false, - vm, - }); - - // act - t.run(); + describe('error re-throwing and logging', () => { + afterEach(() => { + vi.resetAllMocks(); + }); - // assert - t.expect.consoleErrorSpy.not.toHaveBeenCalled(); - t.expect.warnHandlerSpy.not.toHaveBeenCalled(); + describe('error re-throwing', () => { + it('should re-throw error when no error handler exists', () => { + const t = testHarness({ + enableErrorHandler: false, + enableConsole: true, + vm: { $options: { name: 'stub-vm' } }, }); + + expect(() => t.run()).toThrow(DummyError); }); - }); - describe('given I enabled error logging', () => { - describe('when I provide a `warnHandler`', () => { - describe('when a error is captured', () => { - test.each([ - [ - 'with wm', - { - $options: { - name: 'stub-vm', - }, - }, - generateComponentTrace({ - $options: { - name: 'stub-vm', - }, - } as ViewModel), - ], - ['without vm', null, ''], - ])('it calls my `warnHandler` (%s)', (_, vm, expectedTrace) => { - // arrange - const t = testHarness({ - vm, - logErrors: true, - enableWarnHandler: true, - }); + it('should call user-defined error handler when provided', () => { + const vm = { $options: { name: 'stub-vm' } }; + const t = testHarness({ + enableErrorHandler: true, + enableConsole: true, + vm, + }); - // act - t.run(); + t.run(); - // assert - t.expect.consoleErrorSpy.not.toHaveBeenCalled(); - t.expect.warnHandlerSpy.toHaveBeenCalledWith( - 'Error in stub-lifecycle-hook: "DummyError: just an error"', - vm, - expectedTrace, - ); - }); - }); + t.expect.errorHandlerSpy.toHaveBeenCalledWith(expect.any(Error), vm, 'stub-lifecycle-hook'); }); + }); - describe('when I do not provide a `warnHandler`', () => { - describe("and I don't have a console", () => { - test('it logs nothing', () => { - // arrange - const vm = { - $options: { - name: 'stub-vm', - }, - }; - const t = testHarness({ - vm, - logErrors: true, - enableConsole: false, - }); + describe('error logging enabled', () => { + it('should use console.error when an `errorHandler` is available', () => { + const t = testHarness({ + vm: null, + logErrors: true, + enableConsole: true, + enableErrorHandler: true, + enableWarnHandler: false, + }); - // act - t.run(); + t.run(); - // assert - t.expect.consoleErrorSpy.not.toHaveBeenCalled(); - }); + t.expect.consoleErrorSpy.toHaveBeenCalledWith( + '[Vue warn]: Error in stub-lifecycle-hook: "DummyError: just an error"', + ); + }); + + it('should prefer warn handler over console.error when both are available', () => { + const vm = { $options: { name: 'stub-vm' } }; + const t = testHarness({ + vm, + logErrors: true, + enableErrorHandler: true, + enableWarnHandler: true, + enableConsole: true, }); - describe('and I silenced logging in Vue', () => { - test('it logs nothing', () => { - // arrange - const vm = { - $options: { - name: 'stub-vm', - }, - }; - const t = testHarness({ - vm, - logErrors: true, - silent: true, - }); + t.run(); - // act - t.run(); + t.expect.consoleErrorSpy.not.toHaveBeenCalled(); + t.expect.warnHandlerSpy.toHaveBeenCalledWith( + 'Error in stub-lifecycle-hook: "DummyError: just an error"', + vm, + generateComponentTrace(vm as ViewModel), + ); + }); - // assert - t.expect.consoleErrorSpy.not.toHaveBeenCalled(); - }); + it('should throw error when no handler is available', () => { + const vm = { $options: { name: 'stub-vm' } }; + const t = testHarness({ + vm, + logErrors: true, + silent: true, }); - test('it call `console.error`', () => { - // arrange - const t = testHarness({ - vm: null, - logErrors: true, - enableConsole: true, - }); - - // act - t.run(); + expect(() => t.run()).toThrow(DummyError); + }); - // assert - t.expect.consoleErrorSpy.toHaveBeenCalledWith( - '[Vue warn]: Error in stub-lifecycle-hook: "DummyError: just an error"', - ); + it('should fallback to console.error when warn handler is not available', () => { + const t = testHarness({ + vm: null, + logErrors: true, + enableConsole: true, + enableErrorHandler: true, }); + + t.run(); + + t.expect.consoleErrorSpy.toHaveBeenCalledWith( + '[Vue warn]: Error in stub-lifecycle-hook: "DummyError: just an error"', + ); }); }); }); @@ -393,6 +364,7 @@ const testHarness = ({ expect(captureExceptionSpy).toHaveBeenCalledTimes(1); const error = captureExceptionSpy.mock.calls[0][0]; const contexts = captureExceptionSpy.mock.calls[0][1]?.captureContext.contexts; + const mechanismMetadata = captureExceptionSpy.mock.calls[0][1]?.mechanism; expect(error).toBeInstanceOf(DummyError); @@ -403,6 +375,9 @@ const testHarness = ({ withoutProps: () => { expect(contexts).not.toHaveProperty('vue.propsData'); }, + withMechanismMetadata: (mechanism: { handled: boolean; type: 'vue' }) => { + expect(mechanismMetadata).toEqual(mechanism); + }, }; }, },