From e4dea59f84dfbf32cda1cdd832380dd90b1a6fd1 Mon Sep 17 00:00:00 2001 From: hello-brsd Date: Fri, 16 Apr 2021 17:57:25 +0200 Subject: [PATCH] fix(errorHandler): async error handling for watchers (#9484) Co-authored-by: Eduardo San Martin Morote --- src/core/instance/state.js | 10 +- src/core/observer/watcher.js | 8 +- test/unit/features/error-handling.spec.js | 63 +++++--- .../features/options/errorCaptured.spec.js | 148 ++++++++++++++++++ 4 files changed, 200 insertions(+), 29 deletions(-) diff --git a/src/core/instance/state.js b/src/core/instance/state.js index db0d8acd087..64d03649121 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 = { @@ -357,12 +358,9 @@ export function stateMixin (Vue: Class) { options.user = true const watcher = new Watcher(vm, expOrFn, cb, options) if (options.immediate) { + const info = `callback for immediate watcher "${watcher.expression}"` pushTarget() - try { - cb.call(vm, watcher.value) - } catch (error) { - handleError(error, vm, `callback for immediate watcher "${watcher.expression}"`) - } + invokeWithErrorHandling(cb, vm, [watcher.value], vm, info) popTarget() } return function unwatchFn () { 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..7dd36dc9f1f 100644 --- a/test/unit/features/error-handling.spec.js +++ b/test/unit/features/error-handling.spec.js @@ -127,25 +127,25 @@ 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) + 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 +359,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 () { diff --git a/test/unit/features/options/errorCaptured.spec.js b/test/unit/features/options/errorCaptured.spec.js index 3da13b9ed38..0c1aba35b79 100644 --- a/test/unit/features/options/errorCaptured.spec.js +++ b/test/unit/features/options/errorCaptured.spec.js @@ -247,4 +247,152 @@ describe('Options errorCaptured', () => { expect(store.errors[0]).toEqual(new Error('render error')) }).then(done) }) + + 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 + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + 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(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 + }, + render () {} + } + + new Vue({ + errorCaptured: spy, + render: h => h(Child) + }).$mount() + + 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 => { + 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) + }) })