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

Improved error messages for client state/consensus queries #1795

Merged
merged 2 commits into from
Jan 21, 2022

Conversation

adizere
Copy link
Member

@adizere adizere commented Jan 20, 2022

Closes: #XXX

Description

While debugging with a relayer operator this log message:

Jan 20 02:43:40 Imperator-Relayer hermes[751041]: 2022-01-20T02:43:40.107723Z WARN ThreadId(73) task RefreshClientWorker(sentinelhub-2 -> osmosis-1:07-tendermint-645) encountered ignorable error: failed while querying Tx for client 07-tendermint-645 on chain id: osmosis-1: error decoding protobuf: error converting message type into domain type: the client state was not found

We found out that Hermes logs generic messages and we cannot debug without additional refinement on the error types of ForeignClient. This PR addresses that.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@adizere adizere marked this pull request as ready for review January 21, 2022 09:03
@adizere
Copy link
Member Author

adizere commented Jan 21, 2022

Thanks for the review & approval Mikhail. I'll go ahead and merge this, since it's a simple one that improves debugging & observability.

For reference, the problem the operator was having was that he had filter = false, so Hermes spawned client refresh tasks for many (even expired) clients. In particular, the refresh task for client 07-tendermint-645 on osmosis-1 was failing because the latest_height for this client was 2-2551351. Sentinel produced that block 4 months ago , and probably Osmosis pruned the client's state, hence Hermes was failing the query for the client consensus state at that height.

@adizere adizere merged commit 4310f92 into master Jan 21, 2022
@adizere adizere deleted the adi/error_types branch January 21, 2022 09:39
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…ystems#1795)

* Improved error messages for client state/consensus queries

* FMT
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