vueComponent / ant-design-vue

🌈 An enterprise-class UI components based on Ant Design and Vue. 🐜
https://antdv.com/
Other
20.26k stars 3.79k forks source link

Error Capture Issue #5204

Open gaokun opened 2 years ago

gaokun commented 2 years ago

Version

3.0.0-beta.8

Environment

MacOS 11.4, Chrome 97.0.4692.71, Vue 3.2.28

Reproduction link

https://github.com/gaokun/antdv-error-tracker

Steps to reproduce

  1. select an option [trigger 'onSelect']
  2. then you can find 'Uncaught (in promise) test error' in console

see detail in reproduction demo

What is expected?

'onSelect' of Select should fire onErrorCaptured

What is actually happening?

it dosen't

image


I am not sure this is a bug or wrong usage.

If it is indeed a bug, it exsits in everywhere we invoke callbacks like this way 'props.onSelect()', it breaks the promise chain.

I am free to any feedback. Thx Ken

tangjinzhou commented 2 years ago

Maybe this is vue3 bug. But return promise doesn't make any sense for onSelect

gaokun commented 2 years ago

@tangjinzhou In my scenario, we may send ajax in event callbacks(onSelect is just an example), if response error, errorHandler catch it. But it is uncaught error for now. Unified error handler is very important in all web project, like Exception in Java.

I updated above repo, added a custom component with custom event. It works as expected.

Please help to check

Page

image

Usage

    <CustomComp @custom-event="onTest('custom')" />

CustomComp.vue

<template>
  <div class="custom-comp" @click="onClick">Click Me</div>
</template>

<script setup>
const props = defineProps({
  onCustomEvent: Function,
});

// Style 1:  return result, whatever it is sync or async function.
function onClick() {
  return props.onCustomEvent();
}

// Style 2:  use async/await, good for complex biz
// async function onClick() {
//   await props.onCustomEvent();
// }
</script>

<style scoped>
.custom-comp {
  width: 100px;
  height: 50px;
  background-color: green;
}
</style>

Thx Ken

tangjinzhou commented 2 years ago

if change

function onClick() {
  return props.onCustomEvent();
}

to

function onClick() {
    props.onCustomEvent();
}

we can not get onErrorCaptured.

I donot know why. and recommended to handle errors in the ajax library instead of compoents

gaokun commented 2 years ago

I believe there is code in vue core like this:

try {
   await onClick();
} catch (err) {
  onErrorCaptured(err);
}

so we need to keep the promise chain, return promise or use async/await. Otherwise, it would be like this:

try {
   setTimeout(() => {
      throw "this error can't be catched";
   });
} catch (err) {
  console.error(err);
}
gaokun commented 2 years ago

We do handle errors in ajax, but just Triage, means transferring them into specific errors, like ValidationError, MessageError. Then the errors flow to components, components determine handle it by themselves, or throw them.

Error stream like this:

Ajax ---(triage)---> Biz Component ------> errorHandler

if I do Message.error(xxx) in ajax onReject, Biz Component lose opportunity to determine to display it or not.

Kind of like middleware concept, vue errorHandler is the last node, we can send errors which we want to Sentry.

gaokun commented 2 years ago

I believe there is code in vue core like this:

try {
   await onClick();
} catch (err) {
  onErrorCaptured(err);
}

so we need to keep the promise chain, return promise or use async/await. Otherwise, it would be like this:

try {
   setTimeout(() => {
      throw "this error can't be catched";
   });
} catch (err) {
  console.error(err);
}

@tangjinzhou Found it, please refer callWithAsyncErrorHandling

tangjinzhou commented 2 years ago

Maybe we should use emit instead of props. But it involves too many places and cannot be completed in a short time.

If you can, you can try pr

gaokun commented 2 years ago

emit is asynchronous, wouldn't break main process. Example:

function onClick() {
  emit('custom-event', 'reject error');
  console.log('this message will show before onErrorCaptured, and it always show up no matter emit fires error or not');
}

"But it involves too many places"

Yes, it is. So If I done the pr in weeks/months, there must be lots of conflicts with main branch. I would like to make a pr just for 'onSelect' of Select, and let's talk about how to deal with this enhancement after that.

gaokun commented 2 years ago

@tangjinzhou
demo PR #5229 submitted, please help to check.

The PR is just for onSelect by mouse click, doesn't consider key down or fired by search event(as mentioned it is demo PR).

gaokun commented 2 years ago

@tangjinzhou just FYI, after you refactor Uploader to setup, I faced same issue in beforeUpload

image

onChange Link

If I throw error in beforeUpload, the error won't be caught in errorHandler, coz promise chain doesn't connected,

beforeUpload should support throw error or promise as processFile said. "Rejection will also trade as false"