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

Redact query errors #775

Merged
merged 1 commit into from
Mar 9, 2022
Merged

Redact query errors #775

merged 1 commit into from
Mar 9, 2022

Conversation

ethanfrey
Copy link
Member

Follow up from #765

Closes #759

@ethanfrey ethanfrey requested a review from alpe as a code owner March 7, 2022 21:27
@codecov
Copy link

codecov bot commented Mar 7, 2022

Codecov Report

Merging #775 (5757449) into master (f35a13f) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #775      +/-   ##
==========================================
+ Coverage   58.61%   58.66%   +0.04%     
==========================================
  Files          49       49              
  Lines        5835     5837       +2     
==========================================
+ Hits         3420     3424       +4     
+ Misses       2165     2164       -1     
+ Partials      250      249       -1     
Impacted Files Coverage Δ
x/wasm/keeper/query_plugins.go 79.87% <100.00%> (+0.13%) ⬆️
x/wasm/keeper/keeper.go 87.90% <0.00%> (+0.35%) ⬆️

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

LGTM 👍
The error path was covered by a test already

@ethanfrey ethanfrey merged commit b7dd4ce into master Mar 9, 2022
@ethanfrey ethanfrey deleted the redact-query-errors branch March 9, 2022 10:57
// Otherwise redact all (we can theoretically redact less in the future)
if err != nil {
// Issue #759 - we don't return error string for worries of non-determinism
// moduleLogger(ctx).Info("Redacting submessage error", "cause", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

@alpe @webmaster128 Is it possible to have this error being logged?

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem. I have opened an issue to track the work

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return minimal info on errors returned to contracts
4 participants