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.

Sign up for GitHub

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

Jump to bottom

spec: Improve ABCI spec to clearly explain error returns #2907

Closed
2 tasks done
Tracked by #2317
andynog opened this issue Apr 26, 2024 · 4 comments · Fixed by #3072
Closed
2 tasks done
Tracked by #2317

spec: Improve ABCI spec to clearly explain error returns #2907

andynog opened this issue Apr 26, 2024 · 4 comments · Fixed by #3072
Assignees
@andynog
Labels
abci Application blockchain interface spec Specification-related
Milestone
2024-Q2

Comments

@andynog
Copy link
Contributor

andynog commented Apr 26, 2024 •
edited

  • Improve ABCI spec to clearly explain the difference between error codes (inside ) and errors returned by the ABCI calls themselves
  • Fix some error conditions on abci/example/kvstore/kvstore.go
@andynog andynog mentioned this issue Apr 26, 2024
Closed
6 tasks
@andynog andynog changed the title Improve ABCI spec to clearly explain the difference between error codes (inside ) and errors returned by the ABCI calls themselves spec: Improve ABCI spec to clearly explain error returns Apr 26, 2024
@andynog andynog self-assigned this Apr 26, 2024
@andynog andynog added the spec Specification-related label Apr 26, 2024
@andynog andynog added this to the 2024-Q2 milestone Apr 26, 2024
@andynog andynog added the abci Application blockchain interface label Apr 26, 2024
@hvanz
Copy link
Member

hvanz commented May 1, 2024

I just noticed this comment explaining the difference between error codes and errors in methods. I think that for app developers it's quite difficult to find, so better to put it clearly in the docs or spec. https://github.com/cometbft/cometbft/blob/main/abci/client/client.go#L21-L23

@andynog
Copy link
Contributor Author

andynog commented May 1, 2024

Thanks for the pointer @hvanz, I'll take a look at that.

andynog added a commit that referenced this issue May 13, 2024
@andynog
Adding ABCI error information for tutorials (#2907)
0eef641
@andynog andynog mentioned this issue May 13, 2024
Merged
4 tasks
andynog added a commit that referenced this issue May 13, 2024
@andynog
adding kvstore error handling logic (#2907)
7b4a828
@andynog
Copy link
Contributor Author

andynog commented May 13, 2024

I've added the additional information about ABCI error to the tutorial in docs since it might be easier to find than the spec and we have also code in the kvstore example so it made more sense to add to the docs.

@sergio-mena
Copy link
Contributor

I added a comment to the PR. We still need to fix the "Errors" paragraph we have in the spec, as it's explaining errors, but not mentioning the non-recoverable ones.

andynog added a commit that referenced this issue May 14, 2024
@andynog
added additional information to the spec (#2907)
b362680
github-merge-queue bot pushed a commit that referenced this issue May 22, 2024
@andynog @hvanz @sergio-mena
docs: adding ABCI error information (#3072)
0b4cf9b 
close: #2907 

adding some additional information about ABCI error in the docs and
example in kvstore

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Hernán Vanzetto <[email protected]>
Co-authored-by: Sergio Mena <[email protected]>
mergify bot pushed a commit that referenced this issue May 23, 2024
@andynog @mergify
docs: adding ABCI error information (#3072)
6e04ec9 
close: #2907

adding some additional information about ABCI error in the docs and
example in kvstore

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Hernán Vanzetto <[email protected]>
Co-authored-by: Sergio Mena <[email protected]>
(cherry picked from commit 0b4cf9b)
@mergify mergify bot mentioned this issue May 23, 2024
Merged
4 tasks
hvanz pushed a commit that referenced this issue May 24, 2024
@mergify @andynog
docs: adding ABCI error information (backport #3072) (#3113)
bb59b7b 
close: #2907 

adding some additional information about ABCI error in the docs and
example in kvstore

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #3072 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Andy Nogueira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andynog andynog

Labels
abci Application blockchain interface spec Specification-related
Projects
CometBFT
Status: Done
Milestone
2024-Q2
Development

Successfully merging a pull request may close this issue.

docs: adding ABCI error information cometbft/cometbft
3 participants
@sergio-mena @hvanz @andynog