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

Gracefully handle Error instances without a stack property #7

Merged
merged 1 commit into from
Feb 28, 2019
Merged

Gracefully handle Error instances without a stack property #7

merged 1 commit into from
Feb 28, 2019

Conversation

kmohrf
Copy link
Contributor

@kmohrf kmohrf commented Feb 18, 2019

This fixes #6. I’ve added a test and included some documentation why the stack property is checked.

I thought about simply casting error.stack to a string, but that didn’t seem like a good idea (who knows if there are more fun exceptions to the stack property like array values). So I ended up with validating it as a string or String object (both of which implement replace, so that should be fine).

Feel free to change anything to your liking!

Thanks!

index.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
@sindresorhus sindresorhus changed the title gracefully handle Error instances w/o stack property Gracefully handle Error instances without a stack property Feb 18, 2019
Error.stack is not standardized and while most instances have
the property there are some (like AbortError thrown by the
AbortController in Chromium/Chrome) that don’t.

As the clean-stack implementation requires the passed value to
be a string it’s necessary to validate stack to be that type.
@kmohrf
Copy link
Contributor Author

kmohrf commented Feb 18, 2019

typeof value === 'string' is enough. The stack can never be a Object-wrapped string.

With stack not being standardized it seems to me that it’s as likely a String object as it is undefined. I don’t see the point of omitting that specific case if support for it is already there.

Also, this way, it will lose information about error type. I would consider a better way to handle this.

I agree and have changed the implementation. If you still have something else in mind, please consider a more detailed expectation.

@kmohrf
Copy link
Contributor Author

kmohrf commented Feb 25, 2019

any further thoughts @sindresorhus?

@sindresorhus sindresorhus merged commit a0c122b into sindresorhus:master Feb 28, 2019
@sindresorhus
Copy link
Owner

Thanks :)

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.

message generation breaks when AbortError/Error without stack is encountered
2 participants