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.

Sign up for GitHub

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

Jump to bottom

Create actor error type #66

Merged
merged 11 commits into from
Jul 23, 2019
Merged

Create actor error type #66

merged 11 commits into from
Jul 23, 2019

Conversation

Kubuxu
Copy link
Contributor

@Kubuxu Kubuxu commented Jul 19, 2019

Resolves #59

@Kubuxu
Create actor error type
02dab3e 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Mid rework
020fb6f 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
make invoker work again
877ccb3 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Complete the refactor
537ff23 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Cleanup lint
e513884 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
go mod tidy
52f9060 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu Kubuxu requested review from whyrusleeping and magik6k and removed request for whyrusleeping July 23, 2019 13:05
@Kubuxu
Copy link
Contributor Author

Kubuxu commented Jul 23, 2019

Ready for CR.

@Kubuxu
Fix new
fd374fa 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Make lint even happier
1ec2746 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
go mod tidy
681f40a 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Copy link
Contributor Author

Kubuxu commented Jul 23, 2019

Given that we are using xerrors frequently, I would recommend using xerrrors.Is(err, target) instead of err == target. This will also work with wrapped errors.

whyrusleeping
whyrusleeping reviewed Jul 23, 2019
View reviewed changes
chain/actors/actor_storagemarket.go Outdated
return types.InvokeRet{}, err
naddr, nerr := address.NewFromBytes(ret)
if nerr != nil {
return nil, err
Copy link
Member

Choose a reason for hiding this comment

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

returning the wrong error here

whyrusleeping
whyrusleeping approved these changes Jul 23, 2019
View reviewed changes
Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

This looks good! Much cleaner, and makes me feel more confident in writing actor methods. Let's make a nicer hamt interface in a followup.

whyrusleeping
whyrusleeping reviewed Jul 23, 2019
View reviewed changes
chain/actors/aerrors/wrap.go
fatal: true,
retCode: 0,

msg: "tried creating an error and setting RetCode to 0",
Copy link
Member

Choose a reason for hiding this comment

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

lol

@Kubuxu
Make invoker error non fatal
68cfff1 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Wrap and return correct error
2e6dd1b 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu Kubuxu merged commit 38102b2 into master Jul 23, 2019
@whyrusleeping whyrusleeping deleted the errors/oh-errors branch July 23, 2019 18:26
magik6k added a commit that referenced this pull request Aug 10, 2020
@magik6k
Merge pull request #66 from filecoin-project/fix/es
e3b9ba0 
Fixes / changes for fast-retrieval support
nonsense pushed a commit that referenced this pull request Nov 6, 2020
@vyzo
Merge pull request #66 from filecoin-project/plan/common-roles
1230430 
extract common roles for bootstrapper and miner
dumikau pushed a commit to protofire/lotus that referenced this pull request Jun 8, 2023
@dnkolegov
Fix nil params (filecoin-project#66)
b2e30df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@magik6k magik6k Awaiting requested review from magik6k

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Less error prone errors in actor invokation interface.
2 participants
@Kubuxu @whyrusleeping