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

GetCurrentDealInfo err: handle correctly err case #7346

Merged

Conversation

nonsense
Copy link
Member

Fixes: #7345


When GetCurrentDealInfo returns an error, such as:

		return nil, xerrors.Errorf(
			"deal %d not found "+
				"- deal may not have completed sealing before deal proposal "+
				"start epoch, or deal may have been slashed",
			dealID)

we should just note that we can't recover that deal, mark it as failed, and later remove the sector.

At the moment when GetCurrentDealInfo returns an error, we continue below the if-condition check for err and we throw a nil pointer exception and crash.

@codecov
Copy link

codecov bot commented Sep 17, 2021

Codecov Report

Merging #7346 (6059535) into master (6df17bc) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7346      +/-   ##
==========================================
+ Coverage   39.09%   39.14%   +0.04%     
==========================================
  Files         614      614              
  Lines       64995    64996       +1     
==========================================
+ Hits        25410    25440      +30     
+ Misses      35172    35147      -25     
+ Partials     4413     4409       -4     
Impacted Files Coverage Δ
extern/storage-sealing/states_failed.go 12.60% <100.00%> (+1.25%) ⬆️
storage/wdpost_sched.go 77.22% <0.00%> (-1.99%) ⬇️
extern/sector-storage/sched_worker.go 78.38% <0.00%> (ø)
chain/messagepool/messagepool.go 57.17% <0.00%> (+0.24%) ⬆️
miner/miner.go 55.62% <0.00%> (+0.33%) ⬆️
chain/sync.go 65.27% <0.00%> (+0.33%) ⬆️
storage/wdpost_changehandler.go 98.58% <0.00%> (+0.47%) ⬆️
chain/actors/builtin/miner/v5.go 54.46% <0.00%> (+0.61%) ⬆️
chain/sync_manager.go 67.39% <0.00%> (+0.62%) ⬆️
chain/store/store.go 64.37% <0.00%> (+0.70%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6df17bc...6059535. Read the comment docs.

@nonsense nonsense marked this pull request as ready for review September 17, 2021 13:04
@nonsense nonsense requested a review from a team as a code owner September 17, 2021 13:04
@jennijuju jennijuju merged commit 38f7cbc into master Sep 18, 2021
@jennijuju jennijuju deleted the nonsense/add-missing-continue-to-err-GetCurrentDealInfo branch September 18, 2021 12:50
@jennijuju
Copy link
Member

backported in v1.11.3-rc2

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.

1.11.3-rc1: Deal problem caused mine crashed and not able to get restarted
3 participants