Skip to content

Conversation

yxxhero
Copy link
Member

@yxxhero yxxhero commented Apr 22, 2025

This pull request includes a minor update to the error handling in the storeTinyPeerTask method of the peerTaskConductor struct. The change improves error reporting by ensuring that the same error message is logged and passed to the cancel method.

Error handling improvement:

  • client/daemon/peer/peertask_conductor.go: Replaced the inline error message in pt.cancel with a formatted string stored in the resson variable, ensuring consistency between the logged error and the cancellation reason.
err is nil here. err.Error()? maybe it will panic? or empty?

@yxxhero yxxhero requested a review from a team as a code owner April 22, 2025 00:51
Copy link

codecov bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 34.42%. Comparing base (df69b4f) to head (d5ad78e).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
client/daemon/peer/peertask_conductor.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3980      +/-   ##
==========================================
- Coverage   34.42%   34.42%   -0.01%     
==========================================
  Files         342      342              
  Lines       40079    40080       +1     
==========================================
- Hits        13799    13798       -1     
- Misses      25372    25374       +2     
  Partials      908      908              
Flag Coverage Δ
unittests 34.42% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
client/daemon/peer/peertask_conductor.go 52.00% <0.00%> (-0.05%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

…Task

Signed-off-by: yxxhero <aiopsclub@163.com>
@chlins chlins force-pushed the fix(peer/peertask_conductor)--correct-error-handling-in-storeTinyPeerTask branch from e025909 to d5ad78e Compare April 22, 2025 04:29
@chlins chlins enabled auto-merge (squash) April 22, 2025 04:29
Copy link
Member

@gaius-qi gaius-qi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chlins chlins merged commit 32e353a into main Apr 22, 2025
27 checks passed
@chlins chlins deleted the fix(peer/peertask_conductor)--correct-error-handling-in-storeTinyPeerTask branch April 22, 2025 05:51
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.

3 participants