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

flag to print out error stack #257

Merged
merged 2 commits into from
Nov 19, 2021
Merged

flag to print out error stack #257

merged 2 commits into from
Nov 19, 2021

Conversation

shrimalmadhur
Copy link
Contributor

Fixes # .

Motivation

When check data/construction fails, there's no way to see where the error came from in the code.

Solution

This PR adds a flag which when enabled will print the error stack.
Eventually we should also be able to print error stack from rosetta-sdk-go package for deeper debugging but that will be a different change.

Open questions

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7317

  • 1 of 11 (9.09%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 41.983%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/results/construction_results.go 0 4 0.0%
pkg/results/data_results.go 1 7 14.29%
Totals Coverage Status
Change from base Build 7289: -0.2%
Covered Lines: 707
Relevant Lines: 1684

💛 - Coveralls

@septerr
Copy link
Contributor

septerr commented Nov 18, 2021

Do we need a flag? Should we always print the stacktrace?

@shrimalmadhur
Copy link
Contributor Author

Do we need a flag? Should we always print the stacktrace?

Ya I thought about it before. but eventually I am also planning to bubble up errors from rosetta-sdk-go. So I thought of giving a configurable option to enable and disable it so that if someone doesn't want to see stack trace. I mean in some sense it's always helpful when there are errors but I also wanted to give an option to not log so much stack trace. we could also enable it by default rather than disabling it.

@shrimalmadhur shrimalmadhur merged commit 312b2de into master Nov 19, 2021
@shrimalmadhur shrimalmadhur deleted the madhur/log-error-stack branch November 19, 2021 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants