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

Use thiserror for validator-client #658

Merged
merged 4 commits into from
Jun 29, 2021
Merged

Use thiserror for validator-client #658

merged 4 commits into from
Jun 29, 2021

Conversation

durch
Copy link
Contributor

@durch durch commented Jun 28, 2021

No description provided.

@jstuczyn
Copy link
Contributor

You will probably either want to either remove the comment I made regarding CodedError or move it to the new definition. Or ideally find out what's the proper type for that. The comment in question is:

// this is the case of message like
/*
{
  "code": 12,
  "message": "Not Implemented",
  "details": [
  ]
}
 */
// I didn't manage to find where it exactly originates, nor what the correct types should be
// so all of those are some educated guesses

@durch
Copy link
Contributor Author

durch commented Jun 28, 2021

> {
>   "code": 12,
>   "message": "Not Implemented",
>   "details": [
>   ]
> }

I've actually seen this today, it seems to pop if you hit an unimplemented route, for example if you just hit /smart you end up with 501 - Not implemented. I'll take a look at validator code

@jstuczyn
Copy link
Contributor

Yep, that's exactly how I encountered it. I tried to find where it originated in the validator codebase, but I could not find it anywhere

@durch durch merged commit 31f567f into develop Jun 29, 2021
@durch durch deleted the validator-thiserror branch June 29, 2021 08:01
@neacsu neacsu restored the validator-thiserror branch August 12, 2021 12:19
@durch durch deleted the validator-thiserror branch September 15, 2021 15:31
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.

None yet

3 participants