From f2f15ad6b486f8b4529552ce6e2004c83d4ed8c8 Mon Sep 17 00:00:00 2001 From: "boros.gergely" Date: Tue, 12 Feb 2019 22:04:19 +0100 Subject: [PATCH 1/4] feat(errorHandler): async error handling for watchers --- src/core/instance/state.js | 10 ++-- src/core/observer/watcher.js | 8 ++- test/unit/features/error-handling.spec.js | 66 +++++++++++++++++------ 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/core/instance/state.js b/src/core/instance/state.js index cc1401a3353..cd9ca0bc74d 100644 --- a/src/core/instance/state.js +++ b/src/core/instance/state.js @@ -25,7 +25,8 @@ import { validateProp, isPlainObject, isServerRendering, - isReservedAttribute + isReservedAttribute, + invokeWithErrorHandling } from '../util/index' const sharedPropertyDefinition = { @@ -355,11 +356,8 @@ export function stateMixin (Vue: Class) { options.user = true const watcher = new Watcher(vm, expOrFn, cb, options) if (options.immediate) { - try { - cb.call(vm, watcher.value) - } catch (error) { - handleError(error, vm, `callback for immediate watcher "${watcher.expression}"`) - } + const info = `callback for immediate watcher "${watcher.expression}"` + invokeWithErrorHandling(cb, vm, [watcher.value], vm, info) } return function unwatchFn () { watcher.teardown() diff --git a/src/core/observer/watcher.js b/src/core/observer/watcher.js index 5fbcdf347f2..1b1c0534c00 100644 --- a/src/core/observer/watcher.js +++ b/src/core/observer/watcher.js @@ -7,6 +7,7 @@ import { parsePath, _Set as Set, handleError, + invokeWithErrorHandling, noop } from '../util/index' @@ -191,11 +192,8 @@ export default class Watcher { const oldValue = this.value this.value = value if (this.user) { - try { - this.cb.call(this.vm, value, oldValue) - } catch (e) { - handleError(e, this.vm, `callback for watcher "${this.expression}"`) - } + const info = `callback for watcher "${this.expression}"` + invokeWithErrorHandling(this.cb, this.vm, [value, oldValue], this.vm, info) } else { this.cb.call(this.vm, value, oldValue) } diff --git a/test/unit/features/error-handling.spec.js b/test/unit/features/error-handling.spec.js index 7d0eaa512ab..8e3b0236f50 100644 --- a/test/unit/features/error-handling.spec.js +++ b/test/unit/features/error-handling.spec.js @@ -127,25 +127,30 @@ describe('Error handling', () => { }).then(done) }) - it('should recover from errors in user watcher callback', done => { - const vm = createTestInstance(components.userWatcherCallback) - vm.n++ - waitForUpdate(() => { - expect(`Error in callback for watcher "n"`).toHaveBeenWarned() - expect(`Error: userWatcherCallback`).toHaveBeenWarned() - }).thenWaitFor(next => { - assertBothInstancesActive(vm).end(next) - }).then(done) + ;[ + ['userWatcherCallback', 'watcher'], + ['userImmediateWatcherCallback', 'immediate watcher'] + ].forEach(([type, description]) => { + it(`should recover from errors in user ${description} callback`, done => { + const vm = createTestInstance(components[type]) + assertBothInstancesActive(vm).then(() => { + expect(`Error in callback for ${description} "n"`).toHaveBeenWarned() + expect(`Error: ${type} error`).toHaveBeenWarned() + }).then(done) + }) }) - it('should recover from errors in user immediate watcher callback', done => { - const vm = createTestInstance(components.userImmediateWatcherCallback) - waitForUpdate(() => { - expect(`Error in callback for immediate watcher "n"`).toHaveBeenWarned() - expect(`Error: userImmediateWatcherCallback error`).toHaveBeenWarned() - }).thenWaitFor(next => { - assertBothInstancesActive(vm).end(next) - }).then(done) + ;[ + ['userWatcherCallback', 'watcher'], + ['userImmediateWatcherCallback', 'immediate watcher'] + ].forEach(([type, description]) => { + it(`should recover from promise errors in user ${description} callback`, done => { + const vm = createTestInstance(components[`${type}Async`]) + assertBothInstancesActive(vm).then(() => { + expect(`Error in callback for ${description} "n" (Promise/async)`).toHaveBeenWarned() + expect(`Error: ${type} error`).toHaveBeenWarned() + }).then(done) + }) }) it('config.errorHandler should capture render errors', done => { @@ -359,6 +364,33 @@ function createErrorTestComponents () { } } + components.userWatcherCallbackAsync = { + props: ['n'], + watch: { + n () { + return Promise.reject(new Error('userWatcherCallback error')) + } + }, + render (h) { + return h('div', this.n) + } + } + + components.userImmediateWatcherCallbackAsync = { + props: ['n'], + watch: { + n: { + immediate: true, + handler () { + return Promise.reject(new Error('userImmediateWatcherCallback error')) + } + } + }, + render (h) { + return h('div', this.n) + } + } + // event errors components.event = { beforeCreate () { From ccb318a2e97214bd8f4feab3cda1831ed3b26377 Mon Sep 17 00:00:00 2001 From: "boros.gergely" Date: Wed, 13 Feb 2019 18:05:14 +0100 Subject: [PATCH 2/4] refactor(errorHandler): refact errorHandler tests for watchers --- test/unit/features/error-handling.spec.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/unit/features/error-handling.spec.js b/test/unit/features/error-handling.spec.js index 8e3b0236f50..7dd36dc9f1f 100644 --- a/test/unit/features/error-handling.spec.js +++ b/test/unit/features/error-handling.spec.js @@ -138,12 +138,7 @@ describe('Error handling', () => { expect(`Error: ${type} error`).toHaveBeenWarned() }).then(done) }) - }) - ;[ - ['userWatcherCallback', 'watcher'], - ['userImmediateWatcherCallback', 'immediate watcher'] - ].forEach(([type, description]) => { it(`should recover from promise errors in user ${description} callback`, done => { const vm = createTestInstance(components[`${type}Async`]) assertBothInstancesActive(vm).then(() => { From d3285e791809d44c9304db3f0cfbf10b39f8e6a7 Mon Sep 17 00:00:00 2001 From: "boros.gergely" Date: Mon, 8 Jul 2019 23:22:33 +0200 Subject: [PATCH 3/4] test(errorHandler): add more errorHandler tests for watchers --- .../features/options/errorCaptured.spec.js | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/test/unit/features/options/errorCaptured.spec.js b/test/unit/features/options/errorCaptured.spec.js index d16c057da6f..16d081044b9 100644 --- a/test/unit/features/options/errorCaptured.spec.js +++ b/test/unit/features/options/errorCaptured.spec.js @@ -209,4 +209,148 @@ describe('Options errorCaptured', () => { expect(calls).toEqual([1, 2, 3]) }) + + it('should capture error from watcher', done => { + const spy = jasmine.createSpy() + + let child + let err + const Child = { + data () { + return { + foo: null + } + }, + watch: { + foo () { + err = new Error('userWatcherCallback error') + throw err + } + }, + created () { + child = this + this.foo = 'bar' + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + render: h => h(Child), + }).$mount() + + waitForUpdate(() => { + expect(spy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo"') + expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo"') + }).then(Vue.nextTick(done)) + }) + + it('should capture promise error from watcher', done => { + const spy = jasmine.createSpy() + + let child + let err + const Child = { + data () { + return { + foo: null + } + }, + watch: { + foo () { + err = new Error('userWatcherCallback error') + return Promise.reject(err) + } + }, + created () { + child = this + this.foo = 'bar' + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + render: h => h(Child), + }).$mount() + + waitForUpdate(() => { + expect(spy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') + expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') + }).then(Vue.nextTick(done)) + }) + + it('should capture error from immediate watcher', done => { + const spy = jasmine.createSpy() + + let child + let err + const Child = { + data () { + return { + foo: 'foo' + } + }, + watch: { + foo: { + immediate: true, + handler () { + err = new Error('userImmediateWatcherCallback error') + throw err + } + } + }, + created () { + child = this + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + render: h => h(Child) + }).$mount() + + waitForUpdate(() => { + expect(spy).toHaveBeenCalledWith(err, child, 'callback for immediate watcher "foo"') + expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for immediate watcher "foo"') + }).then(done) + }) + + it('should capture promise error from immediate watcher', done => { + const spy = jasmine.createSpy() + + let child + let err + const Child = { + data () { + return { + foo: 'foo' + } + }, + watch: { + foo: { + immediate: true, + handler () { + err = new Error('userImmediateWatcherCallback error') + return Promise.reject(err) + } + } + }, + created () { + child = this + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + render: h => h(Child) + }).$mount() + + waitForUpdate(() => { + expect(spy).toHaveBeenCalledWith(err, child, 'callback for immediate watcher "foo" (Promise/async)') + expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for immediate watcher "foo" (Promise/async)') + }).then(done) + }) }) From 3491ca9abd51144ce23aeeb1277b7e42831e8de7 Mon Sep 17 00:00:00 2001 From: "boros.gergely" Date: Tue, 9 Jul 2019 21:37:04 +0200 Subject: [PATCH 4/4] test(errorHandler): fix errorCaptured tests for watchers --- .../features/options/errorCaptured.spec.js | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/unit/features/options/errorCaptured.spec.js b/test/unit/features/options/errorCaptured.spec.js index 37dcfcb6449..0c1aba35b79 100644 --- a/test/unit/features/options/errorCaptured.spec.js +++ b/test/unit/features/options/errorCaptured.spec.js @@ -267,20 +267,21 @@ describe('Options errorCaptured', () => { }, created () { child = this - this.foo = 'bar' }, render () {} } new Vue({ errorCaptured: spy, - render: h => h(Child), + render: h => h(Child) }).$mount() + child.foo = 'bar' + waitForUpdate(() => { expect(spy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo"') expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo"') - }).then(Vue.nextTick(done)) + }).then(done) }) it('should capture promise error from watcher', done => { @@ -302,20 +303,23 @@ describe('Options errorCaptured', () => { }, created () { child = this - this.foo = 'bar' }, render () {} } new Vue({ errorCaptured: spy, - render: h => h(Child), + render: h => h(Child) }).$mount() - waitForUpdate(() => { - expect(spy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') - expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') - }).then(Vue.nextTick(done)) + child.foo = 'bar' + + child.$nextTick(() => { + waitForUpdate(() => { + expect(spy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') + expect(globalSpy).toHaveBeenCalledWith(err, child, 'callback for watcher "foo" (Promise/async)') + }).then(done) + }) }) it('should capture error from immediate watcher', done => {