Skip to content

Added additional check before adding the ._error property #1657

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

Merged
merged 2 commits into from
Aug 21, 2020

Conversation

Alex-Sokolov
Copy link
Contributor

@Alex-Sokolov Alex-Sokolov commented Aug 19, 2020

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

Other information:

Sometimes an error may occur in nextTick. In this case an error message will be displayed in the console:

  ●  Cannot log after tests are done. Did you forget to wait for something async in your test?
    Attempted to log "[Vue warn]: Error in config.errorHandler: "TypeError: Cannot set property '_error' of undefined"".

      at BufferedConsole.error (node_modules/@jest/console/build/BufferedConsole.js:165:10)
      at warn (node_modules/vue/dist/vue.runtime.common.dev.js:621:15)
      at logError (node_modules/vue/dist/vue.runtime.common.dev.js:1880:5)
      at globalHandleError (node_modules/vue/dist/vue.runtime.common.dev.js:1871:9)
      at handleError (node_modules/vue/dist/vue.runtime.common.dev.js:1835:5)
      at Array.<anonymous> (node_modules/vue/dist/vue.runtime.common.dev.js:1978:9)
      at flushCallbacks (node_modules/vue/dist/vue.runtime.common.dev.js:1902:14)

In Vue in the nextTick handler, the context is an optional parameter (https://github.com/vuejs/vue/blob/dev/src/core/util/next-tick.js#L87-L95):

export function nextTick (cb?: Function, ctx?: Object) {
  let _resolve
  callbacks.push(() => {
    if (cb) {
      try {
        cb.call(ctx)
      } catch (e) {
        handleError(e, ctx, 'nextTick')
      }
// ...

In the handleError handler, the presence of vm is checked first, but later, if not, it is still called (https://github.com/vuejs/vue/blob/dev/src/core/util/error.js#L9-L30):

export function handleError (err: Error, vm: any, info: string) {
  // Deactivate deps tracking while processing error handler to avoid possible infinite rendering.
  // See: https://github.com/vuejs/vuex/issues/1505
  pushTarget()
  try {
    if (vm) {
      let cur = vm
      while ((cur = cur.$parent)) {
        const hooks = cur.$options.errorCaptured
        if (hooks) {
          for (let i = 0; i < hooks.length; i++) {
            try {
              const capture = hooks[i].call(cur, err, vm, info) === false
              if (capture) return
            } catch (e) {
              globalHandleError(e, cur, 'errorCaptured hook')
            }
          }
        }
      }
    }
    globalHandleError(err, vm, info)
//...

Finally, vue-test-utils register global error handler and try to add the new ._error property to the vm parameter which can be undefined (https://github.com/vuejs/vue-test-utils/blob/dev/packages/test-utils/src/error.js#L4-L10):

function errorHandler(errorOrString, vm) {
  const error =
    typeof errorOrString === 'object' ? errorOrString : new Error(errorOrString)

  vm._error = error
  throw error
}

This PR adds extra checking in case.

@lmiller1990
Copy link
Member

lmiller1990 commented Aug 21, 2020

Wow great job figuring this out. Thanks a lot.

Can you check what's going on with the CI?

Edit: just need to run the linter :)

@Alex-Sokolov
Copy link
Contributor Author

@lmiller1990 done 😄

@lmiller1990 lmiller1990 merged commit 8e9dda3 into vuejs:dev Aug 21, 2020
@Alex-Sokolov Alex-Sokolov deleted the patch-1 branch August 22, 2020 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants