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: remove extra stackframe #391

Merged
merged 1 commit into from
Jul 24, 2020
Merged

fix: remove extra stackframe #391

merged 1 commit into from
Jul 24, 2020

Conversation

erights
Copy link
Contributor

@erights erights commented Jul 24, 2020

Fixes #370

When we make an error by calling new on either of our replacement Error constructors, it calls the original Error constructor to make the error. Thus, the replacement Error constructor is on the stack above the creation of the new error object. This is exactly the problem that v8 designed Error.captureStackTrace to solve, and we solve it using their suggested pattern. The price of this pattern is that the stack trace is reified into a structured-stack-trace (sst) immediately rather than on demand. However, rendering the sst as a string is still on demand as intended.

@erights erights requested review from kriskowal and warner July 24, 2020 05:56
@erights erights self-assigned this Jul 24, 2020
@erights erights merged commit 9ba5ecf into master Jul 24, 2020
@erights erights deleted the 370-remove-extra-stackframe branch July 24, 2020 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error taming: consider removing the bogus extra frame
2 participants