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

Add error codes and define errors with it #343

Closed
wants to merge 12 commits into from

Conversation

lpoli
Copy link
Contributor

@lpoli lpoli commented Dec 27, 2021

Our error structure in "github.com/0chain/errors" is;

type Error struct{
	code string
	msg string
}

In gosdk we have different error implementation. Sometimes above structure is considered; other time thrown.Throw is considered and
sometimes errors.New or fmt.Errorf is considered.
This makes error handling difficult and error response are inconsistent as well.
Below is just a structure considering error structure from "github.com/0chain/errors". We can work with other structure but it should be
consistently used inside gosdk.

One case:
Say I requested some file refs from 6 blobbers(with 4:2 ratio) and 3 of them gave too_many_requests error. If only two had given too_many_requests error then anyway we could take consensus from other 4 blobbers and request is successful. But with three blobbers giving those errors, consensus would not be reached so error would be consensus_failed. The application that made this request should have been let know that it was too_many_requests error that caused consensus failure, so that it would wait for some time interval and re-request again.

We can define codes as constant values so function would use it. Later some function can check which error it is with errors.Is function with the errors defined below in var.

@lpoli
Copy link
Contributor Author

lpoli commented Dec 27, 2021

@Sriep @dabasov @cnlangzi
Please comment on this.

@cnlangzi
Copy link
Contributor

@Sriep @dabasov @cnlangzi Please comment on this.

old errors.Wrap works with many code. many things might be broken. but I am not sure, you have to test all of them.

errors.Thrown is working with go errors v2 in blobber only.

@cnlangzi
Copy link
Contributor

please make all system_tests passed first. and talk to @moldis about zmobile

@lpoli
Copy link
Contributor Author

lpoli commented Dec 27, 2021

@cnlangzi
This is just an initial draft.
I will continue if you guys approve it.

@dabasov
Copy link
Member

dabasov commented Dec 29, 2021

Having distinct errors is a good idea. I like GOish style of managing it, just having constant errors and wrap them when needed. Using 0chain custom error is not a problem too, since it implements errors interface.

@dabasov dabasov closed this Jul 22, 2023
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.

4 participants