Skip to content
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

Error Capture Issue #5204

Open
1 task done
gaokun opened this issue Jan 24, 2022 · 10 comments
Open
1 task done

Error Capture Issue #5204

gaokun opened this issue Jan 24, 2022 · 10 comments
Assignees

Comments

@gaokun
Copy link
Contributor

gaokun commented Jan 24, 2022

  • I have searched the issues of this repository and believe that this is not a duplicate.

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
Copy link
Member

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

@gaokun
Copy link
Contributor Author

gaokun commented Jan 25, 2022

@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
Copy link
Member

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
Copy link
Contributor Author

gaokun commented Jan 27, 2022

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
Copy link
Contributor Author

gaokun commented Jan 27, 2022

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
Copy link
Contributor Author

gaokun commented Jan 27, 2022

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
Copy link
Member

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
Copy link
Contributor Author

gaokun commented Jan 28, 2022

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
Copy link
Contributor Author

gaokun commented Feb 8, 2022

@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
Copy link
Contributor Author

gaokun commented May 19, 2022

@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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants