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

fix: surface original stack trace and message with errors #651

Merged
merged 2 commits into from
Mar 27, 2019

Conversation

JustinBeckwith
Copy link
Contributor

@JustinBeckwith JustinBeckwith commented Mar 26, 2019

Fixes #643. This PR looks for multiple instances where context for exceptions could be lost. The two primary types of cases here are:

  1. Completely overwriting the error message but retaining the stack trace
  2. Catching, and throwing an entirely new error with the original context lost

Instead, we always make sure to re-throw the original error, and consistently pin the original error message to the tail of the new message.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 26, 2019
@JustinBeckwith JustinBeckwith requested review from ofrobots and bcoe March 26, 2019 04:37
@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #651 into master will decrease coverage by 0.14%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   88.39%   88.25%   -0.15%     
==========================================
  Files          18       18              
  Lines         784      783       -1     
  Branches       86       84       -2     
==========================================
- Hits          693      691       -2     
- Misses         79       81       +2     
+ Partials       12       11       -1
Impacted Files Coverage Δ
src/auth/computeclient.ts 93.93% <100%> (ø) ⬆️
src/auth/oauth2client.ts 86.45% <55.55%> (-0.24%) ⬇️
src/auth/googleauth.ts 91.28% <85.71%> (-0.18%) ⬇️

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 daab404...b20ba91. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #651 into master will decrease coverage by 0.14%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   88.26%   88.12%   -0.15%     
==========================================
  Files          18       18              
  Lines         784      783       -1     
  Branches       86       84       -2     
==========================================
- Hits          692      690       -2     
- Misses         80       82       +2     
+ Partials       12       11       -1
Impacted Files Coverage Δ
src/auth/computeclient.ts 93.93% <100%> (ø) ⬆️
src/auth/oauth2client.ts 86.45% <71.42%> (-0.24%) ⬇️
src/auth/googleauth.ts 90.76% <75%> (-0.19%) ⬇️

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 5b74985...6cb1092. Read the comment docs.

@JustinBeckwith JustinBeckwith added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 27, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 27, 2019
@JustinBeckwith JustinBeckwith merged commit 8fb65eb into master Mar 27, 2019
@JustinBeckwith JustinBeckwith deleted the errory branch April 13, 2019 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants