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

feat: improve error wrapping, stack trace management, and formatting #46

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

morningvera
Copy link
Member

This commit includes a change to error wrapping that enables wrap error stack frames to be inserted into the root error stack trace. It also changes error formatting to display root errors first followed by wrapped errors in the same order as the stack trace (same order as runtime.Callers).

@morningvera morningvera requested a review from sum2000 January 5, 2020 21:44
@morningvera morningvera added the enhancement New feature or request label Jan 5, 2020
@morningvera morningvera added this to the v0.2.0 milestone Jan 5, 2020
@codecov-io
Copy link

codecov-io commented Jan 7, 2020

Codecov Report

Merging #46 into master will decrease coverage by 1.27%.
The diff coverage is 87.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   95.57%   94.29%   -1.28%     
==========================================
  Files           3        3              
  Lines         226      228       +2     
==========================================
- Hits          216      215       -1     
- Misses          8        9       +1     
- Partials        2        4       +2
Impacted Files Coverage Δ
eris.go 100% <100%> (ø) ⬆️
format.go 100% <100%> (+5.35%) ⬆️
stack.go 72.91% <62.96%> (-15.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fe3c59...ab35abd. Read the comment docs.

Copy link
Member

@sum2000 sum2000 left a comment

Choose a reason for hiding this comment

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

I am yet to review the examples you added but I have reviewed the code. Overall I suggest that we add some comments, especially in format.go and stack.go. It is a pain but a few days from now if we look at our code we won't understand it easily without the comments.

@morningvera morningvera force-pushed the feat/improve-wrap-stack-trace branch from fe67a73 to 0273d62 Compare January 9, 2020 17:32
@morningvera morningvera requested a review from sum2000 January 9, 2020 17:32
Copy link
Member

@sum2000 sum2000 left a comment

Choose a reason for hiding this comment

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

format.go could be better with some comments but feel free to skip it. I am planning to add some in the formatter PR

@sum2000 sum2000 assigned sum2000 and unassigned sum2000 Jan 9, 2020
@sum2000 sum2000 self-requested a review January 9, 2020 21:11
@morningvera morningvera requested a review from sum2000 January 10, 2020 00:10
@morningvera morningvera force-pushed the feat/improve-wrap-stack-trace branch from 97e1e2c to 1784b3d Compare January 10, 2020 00:48
This commit includes a change to error wrapping that enables wrap error
stack frames to be inserted into the root error stack trace. It also
changes error formatting to display root errors first followed by wrapped
errors in the same order as the stack trace (same order as runtime.Callers).
@morningvera morningvera force-pushed the feat/improve-wrap-stack-trace branch from 1784b3d to ab35abd Compare January 10, 2020 00:56
Copy link
Member

@sum2000 sum2000 left a comment

Choose a reason for hiding this comment

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

👍 👍

@morningvera morningvera merged commit ebc3771 into master Jan 10, 2020
@morningvera morningvera deleted the feat/improve-wrap-stack-trace branch January 10, 2020 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants