Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat!: support debug trace QueryResult #9576

Merged
merged 2 commits into from
Jul 8, 2021

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Jun 24, 2021

Description

To let abci query response include more detailed error message with node started with --trace.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@yihuang yihuang force-pushed the debug_query_result branch from 47ada97 to fc4354e Compare June 24, 2021 08:34
@yihuang yihuang changed the title support debug trace QueryResult feat: support debug trace QueryResult Jun 24, 2021
@yihuang yihuang force-pushed the debug_query_result branch from fc4354e to 940a1f1 Compare June 24, 2021 08:36
@codecov
Copy link

codecov bot commented Jun 24, 2021

Codecov Report

Merging #9576 (d13574c) into master (0027111) will increase coverage by 27.98%.
The diff coverage is 63.61%.

❗ Current head d13574c differs from pull request most recent head fabbcc7. Consider uploading reports for the commit fabbcc7 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #9576       +/-   ##
===========================================
+ Coverage   35.48%   63.47%   +27.98%     
===========================================
  Files         332      571      +239     
  Lines       32620    37521     +4901     
===========================================
+ Hits        11575    23815    +12240     
+ Misses      19819    11848     -7971     
- Partials     1226     1858      +632     
Impacted Files Coverage Δ
client/keys/types.go 100.00% <ø> (+100.00%) ⬆️
client/query.go 16.98% <ø> (ø)
client/rpc/block.go 0.00% <ø> (ø)
client/rpc/status.go 67.74% <ø> (ø)
client/rpc/validators.go 0.00% <ø> (ø)
client/test_helpers.go 0.00% <ø> (ø)
client/tx/factory.go 27.00% <ø> (ø)
client/tx/legacy.go 68.42% <ø> (ø)
client/tx/tx.go 40.83% <ø> (ø)
client/utils.go 41.93% <ø> (-41.40%) ⬇️
... and 681 more

Copy link
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good
can you update this branch with master @yihuang ?

CHANGELOG.md Outdated
@@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#9231](https://github.com/cosmos/cosmos-sdk/pull/9231) Remove redundant staking errors.
* [\#9205](https://github.com/cosmos/cosmos-sdk/pull/9205) Improve readability in `abci` handleQueryP2P
* [\#9314](https://github.com/cosmos/cosmos-sdk/pull/9314) Update Rosetta SDK to upstream's latest release.
* [\#9576](https://github.com/cosmos/cosmos-sdk/pull/9576) Add debug error message to query result when enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this to a Features subsection in the Unreleased section?

@amaury1093
Copy link
Contributor

@yihuang any updates on this?

@yihuang yihuang force-pushed the debug_query_result branch from 940a1f1 to d13574c Compare July 8, 2021 06:54
@yihuang
Copy link
Collaborator Author

yihuang commented Jul 8, 2021

@yihuang any updates on this?

sorry for the delay, done now.

@yihuang
Copy link
Collaborator Author

yihuang commented Jul 8, 2021

Everything looks good
can you update this branch with master @yihuang ?

done

@amaury1093 amaury1093 changed the title feat: support debug trace QueryResult feat!: support debug trace QueryResult Jul 8, 2021
Copy link
Contributor

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@amaury1093 amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Jul 8, 2021
@mergify mergify bot merged commit e4f2fa0 into cosmos:master Jul 8, 2021
amaury1093 added a commit that referenced this pull request Jul 8, 2021
amaury1093 added a commit that referenced this pull request Jul 8, 2021
* add changelog entry fro 9432

* Move #9576 to api-breaking

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
@yihuang yihuang deleted the debug_query_result branch July 8, 2021 10:39
@yihuang
Copy link
Collaborator Author

yihuang commented Feb 9, 2022

can we backport this to 0.45? it's really useful for debugging because the stack trace is not outputted anywhere else.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants