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: capture stack trace #536

Merged
merged 2 commits into from
Sep 23, 2021
Merged

fix: capture stack trace #536

merged 2 commits into from
Sep 23, 2021

Conversation

fireridlle
Copy link
Contributor

No description provided.

@fireridlle fireridlle linked an issue Sep 10, 2021 that may be closed by this pull request
Copy link

@milyord milyord left a comment

Choose a reason for hiding this comment

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

There's two problems with this fix. One captureStacktTrace is only available in chrome and will throw in other browsers. Two you'll get a stack trace that doesn't originate at the point of throwing the error. If you insist on using this approach you should wrap the call to captureStackTarce in a conditional statement. Also relying on this.constructor can lead to other issues as in some environments the constructor property is not set to configurable: false (granted it is a minor surface for possible future failure).

if (Error.captureStackTrace) {
  Error.captureStackTrace(this, this.constructor)
} 

@fireridlle fireridlle merged commit ee897e9 into main Sep 23, 2021
@fireridlle fireridlle deleted the fix/capture_stack_trace branch September 23, 2021 18:14
@elersong elersong mentioned this pull request Jan 27, 2022
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.

FaunaError and it's sub-types eat the stack trace.
3 participants