refactor(jsonrpc): Initial refactoring for Structured Error handling for JSON RPC and ViewClient #3805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
khorolets
force-pushed
the
refactor/structured-errors-view-client
branch
from
d581fd9 to
661fb67
Compare
frol
reviewed
Jan 15, 2021
khorolets
force-pushed
the
refactor/structured-errors-view-client
branch
2 times, most recently
from
af4c7a9 to
f1a630d
Compare
frol
reviewed
Jan 18, 2021
khorolets
force-pushed
the
refactor/structured-errors-view-client
branch
from
f1a630d to
00e2633
Compare
frol
requested changes
Jan 21, 2021
frol
requested changes
Jan 21, 2021
khorolets
requested review from
bowenwang1996,
evgenykuzyakov,
maxzaver and
SkidanovAlex
as code owners
frol
approved these changes
Jan 21, 2021
There was a problem hiding this comment.
I unblock this PR. The minor changes are agreed to be addressed, so other code owners need to review.
Review hits:
- The changes mostly target
chain/jsonrpc(@frol and @khorolets are code-owners, so we review that) - The rest of the changes are about extracting modules into separate crates to avoid fat dependencies, fat primitives, and circular dependencies
frol
changed the title
khorolets
force-pushed
the
refactor/structured-errors-view-client
branch
from
9cbda12 to
5a70bf7
Compare
This was referenced Jan 25, 2021
frol
approved these changes
Jan 25, 2021
frol
approved these changes
Jan 26, 2021
|
@bowenwang1996 could you please take a look at this PR? Thank you. |
bowenwang1996
approved these changes
Jan 27, 2021
…te crate client-primitives. Extract messages from jsonrpc-client to separate jsonrpc-client-primitives crate. Refactor GetBlock handler to have GetBlockError. Work in progress.
…BlockError possible to be casted from chain-primitives Error
…ockRequest to near-primitives::rpc. Add metrics to watch for unexpected errors
…dress naming review suggestions
Co-authored-by: Vlad Frolov <[email protected]>
…rdering in core/primitives
khorolets
force-pushed
the
refactor/structured-errors-view-client
branch
from
71015fa to
deef168
Compare
This was referenced Jan 27, 2021
2 tasks
miraclx
reviewed
Aug 30, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Start of #3788
It's just a beginning of refactoring. @frol and I decided that having error structures for each handler would be better solution. This is a first try with
GetBlockhandler, which doesn't change the response from jsonrpc.To start it, I've done:
chain/clientto separate crateclient-primitivesmessages.rsfromchain/jsonrpc/clientto separatejsonrpc-client-primitivescrateGetBlockhandler to haveGetBlockErrorWork in progress.
@frol I'd like to know what should be changed in
GetBlockErrorto make it betterP.S. Commented code in the commit will be cleared while I move further.