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

dcrjson: update error types. #2271

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Jul 16, 2020

This updates the dcrjson error types to leverage go 1.13 errors.Is/As functionality as well as confirm to the error infrastructure best practices outlined in #2181.

@davecgh davecgh added this to the 1.7.0 milestone Jul 17, 2020
@dnldd dnldd force-pushed the update_dcrjson_error_types branch 3 times, most recently from f92695c to 4d5b9f1 Compare October 21, 2020 10:27
@dnldd dnldd force-pushed the update_dcrjson_error_types branch from 4d5b9f1 to 0834be5 Compare December 16, 2020 23:23
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

The removal of the jsonerr.go and associated errors isn't correct. These are error codes with very specific meaning to JSON-RPC that need to be retained.

Note that the old ErrorCode cases are for the package API and those should be switched over (as you've done here). However, the "Standard JSON-RPC 2.0 errors", "General application defined JSON errors", etc, need to remain because those are codes that are required by the JSON-RPC specification.

dcrjson/cmdinfo_test.go Outdated Show resolved Hide resolved
dcrjson/cmdinfo_test.go Outdated Show resolved Hide resolved
dcrjson/error_test.go Outdated Show resolved Hide resolved
@davecgh
Copy link
Member

davecgh commented Dec 17, 2020

To follow up, I believe jsonerr.go will want a new

type RPCError struct {
	Code int
	Message string
}

Then all of those errors would become:

ErrInvalidRequest = RPCError{
...
}

...

You can see from the removed comment that there was once a separation between the two for this purpose:

// Error identifies a general error.  This differs from an RPCError in that this
// error typically is used by the consumers of the package as opposed to
// RPCErrors which are intended to be returned to the client across the wire via
// a JSON-RPC Response.  The caller can use type assertions to determine the
// specific error and access the ErrorCode field.

I guess it was incorrectly changed at some point to shoehorn it into using the generic Error type when it shouldn't have been.

@dnldd dnldd force-pushed the update_dcrjson_error_types branch from 0834be5 to da9bef9 Compare December 17, 2020 19:12
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Just needs the copyright year updated on the modified files and then this is good to go.

This updates the dcrjson error types to leverage go
1.13 errors.Is/As functionality as well as confirm to
the error infrastructure best practices.
@dnldd dnldd force-pushed the update_dcrjson_error_types branch from da9bef9 to d80953e Compare December 17, 2020 19:32
@davecgh davecgh merged commit f20a715 into decred:master Dec 17, 2020
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.

2 participants