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

Improve error logging in toCeloFn #2157

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Improve error logging in toCeloFn #2157

merged 1 commit into from
Jul 12, 2023

Conversation

karlb
Copy link
Contributor

@karlb karlb commented Jul 11, 2023

This might help us debug
#2134 if it happens again.

Comment on lines -390 to +393
curr, _ := currencyManager.GetCurrency(feeCurrency)
curr, err := currencyManager.GetCurrency(feeCurrency)
if err != nil {
log.Error("toCeloFn: could not get currency", "err", err)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should help a lot, since we just discarded all the useful error information before.

@karlb karlb marked this pull request as ready for review July 11, 2023 13:32
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit b1bba65

coverage: 48.9% of statements across all listed packages
coverage:  63.0% of statements in consensus/istanbul
coverage:  40.5% of statements in consensus/istanbul/announce
coverage:  54.5% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  61.0% of statements in consensus/istanbul/core
coverage:  45.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.4% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

@karlb karlb enabled auto-merge (squash) July 11, 2023 13:38
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

5855 passed, 1 failed, 45 skipped

Test failures:
  TestBlockTracingConcurrentMapAccess: e2e_test
Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028]
Checking medianRate. numerator = 1000000000000000000000000 denominator = 1000000000000000000000000
Checking gas price minimum. cusdValue = 100000000
e2e_test.go:396:
This test report was produced by the test-summary action.  Made with ❤️ in Cambridge.

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: +0.01 🎉

Comparison is base (4860e01) 55.17% compared to head (a10ae25) 55.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2157      +/-   ##
==========================================
+ Coverage   55.17%   55.18%   +0.01%     
==========================================
  Files         673      676       +3     
  Lines      113208   113888     +680     
==========================================
+ Hits        62458    62849     +391     
- Misses      46915    47175     +260     
- Partials     3835     3864      +29     
Impacted Files Coverage Δ
miner/block.go 52.67% <25.00%> (ø)
contracts/currency/currency.go 55.00% <100.00%> (ø)

... and 28 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

This might help us debug
#2134 if it happens
again.
@karlb karlb force-pushed the karlb/tocelo-error-logging branch from 652df2a to a10ae25 Compare July 12, 2023 06:12
@karlb karlb merged commit b1bba65 into master Jul 12, 2023
@karlb karlb deleted the karlb/tocelo-error-logging branch July 12, 2023 06:32
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