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

Improve Informativeness of Error Messages in provider.query() #1000

Closed
idea404 opened this issue Sep 28, 2022 · 3 comments
Closed

Improve Informativeness of Error Messages in provider.query() #1000

idea404 opened this issue Sep 28, 2022 · 3 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@idea404
Copy link
Contributor

idea404 commented Sep 28, 2022

Is your feature request related to a problem? Please describe.
Uninformative error messages from this repository propagate into the JS workspaces testing suite. As such, the workspaces-js testing suite outputs an unreadable large blob of text which developers must search through until they find a small informative string such as FunctionCallError(MethodResolveError(MethodNotFound)) which tells them what actually is going wrong.

Describe the solution you'd like
I propose to implement error types in json-rpc-providers.ts > JsonRpcProvider.query(), which means expanding error types in errors.ts. This also involves implementing an JsonRpcErrorHandler class that would process errors returned by the query() and return their corresponding error class (with informative messages and info field for suggested action).

Describe alternatives you've considered
I've considered implementing string matching in errors.ts > TypedError as to provide other error types within the class, however this solution will look convoluted and goes against the principle of single concern.

Additional context
I first thought this was an issue with workspaces-js but was pointed here while discussing the issue. We also receive feedback from developers in TG and Discord regarding the confusing nature of the message outputs.

@andy-haynes
Copy link
Collaborator

@idea404 are you thinking different kinds of errors based on the RPC response? E.g. your original examples in near/near-workspaces-js#185 would throw something like a MethodNotFoundError ("Could not find method X on contract Y") or ContractNotDeployed ("No contract deployed to Y").

This looks pretty straightforward, though it's looking like we'll need to extend string matching for unstructured errors. The example you cite in particular does not currently get matched, but even if it did it would still be a little buried. Once we have the types set out we'll just need a little more sophisticated error handling (e.g. checking whether code is deployed at all when wasm execution failed with error: FunctionCallError(MethodResolveError(MethodNotFound)) is returned from RPC.

@idea404
Copy link
Contributor Author

idea404 commented Nov 9, 2022

Good point @andy-haynes. I'll start with some string-matching and we can expand it from there!

@andy-haynes
Copy link
Collaborator

@idea404 I dug in a little more and found that these RPC responses with string error messages don't go through the same error parsing logic as responses with structured errors. I put together a PR to address it as it handles the issue of the MethodNotFound error specifically. It doesn't address the bigger picture you've outlined but it's a good start 🙂

#1028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants