disttask: make task executor `onError` print error's stack.

[LindaSummer]

What problem does this PR solve?

Issue Number: close #56014

Problem Summary:

What changed and how does it work?

Error log in onError function's stack is always in the same function.

But the real stack should be the one invoking onError.

Check List

Tests

• [x] Unit test

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

[ti-chi-bot[bot]]

Hi @LindaSummer. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tiprow[bot]]

Hi @LindaSummer. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[LindaSummer]

Hi @lance6716 ,

Please take a look. 😊

Thanks very much.

Best Regards, Edward

[lance6716]

/ok-to-test

And please post the test result. Or if you can, add an unit test for it. For example, check the output of logger and verify it contents the function that throws the error

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 56.8286%. Comparing base (e13bfeb) to head (5420294). Report is 13 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #56618         +/-   ##
=================================================
- Coverage   73.3262%   56.8286%   -16.4977%     
=================================================
  Files          1630       1759        +129     
  Lines        450312     635618     +185306     
=================================================
+ Hits         330197     361213      +31016     
- Misses        99833     250289     +150456     
- Partials      20282      24116       +3834     

Flag	Coverage Δ
integration	37.0566% <20.0000%> (?)
unit	73.0720% <66.6666%> (+0.6119%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	52.9478% <ø> (ø)
parser	∅ <ø> (∅)
br	53.0248% <ø> (+7.4360%)	:arrow_up:

[LindaSummer]

/ok-to-test

And please post the test result. Or if you can, add an unit test for it. For example, check the output of logger and verify it contents the function that throws the error

Thanks very much for your suggestions!

I will try to add a test case for it. 😊

Best Regards, Edward

[LindaSummer]

Hi @lance6716 ,

I have corrected my mistake and add unit tests. 😊

Please take a look.

Best Regards, Edward

[LindaSummer]

Thanks very much for your warm suggestion and help!

I will update this PR later today.😄

Best Regards, Edward

[LindaSummer]

Hi @lance6716 ,

Thanks very much for your bazel patch!😊

I have updated related code and added stacktrace explicitly for all onError invokers.

Best Regards, Edward

[LindaSummer]

Hi @lance6716 ，

Thanks for your patience and warm help! 😊

I have added an error stack field for stack info inside error and skipped it for error without stack info.

Best Regards, Edward

[LindaSummer]

/retest

[LindaSummer]

rest lgtm

Thanks very much for your patience and help! 😊

I'll update it later today.

Best Regards, Edward

[LindaSummer]

/retest

[LindaSummer]

Hi @lance6716 ,

Thanks for your review and suggestions! 😊

I have updated the related code, please help take a look.

Best Regards, Edward

[lance6716]

/cc @GMHDBJD

[LindaSummer]

/retest

[LindaSummer]

Hi @GMHDBJD,

Sorry to bother you. 😊

Could help take a look?

Best Regards, Edward

[LindaSummer]

/retest

[LindaSummer]

/retest

[LindaSummer]

/retest

[ti-chi-bot[bot]]

[LGTM Timeline notifier]

Timeline:

• 2024-10-17 13:49:07.952007338 +0000 UTC m=+538145.100917175: :ballot_box_with_check: agreed by lance6716.
• 2024-10-22 02:37:36.911991612 +0000 UTC m=+318657.608782236: :ballot_box_with_check: agreed by Benjamin2037.

[LindaSummer]

/assign

[ti-chi-bot[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, CbcWestwolf, lance6716

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~pkg/disttask/OWNERS~~ [Benjamin2037,lance6716]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[D3Hunter]

/hold

[D3Hunter]

/unhold