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: IsCustomError now works as intended; IsError is now simpler. #309

Merged
merged 2 commits into from
May 18, 2021

Conversation

yeldiRium
Copy link
Contributor

I noticed in the wolkenkit that isCustomError would think that basic Errors were CustomErrors and realized that the implementation did not check enough properties to avoid false positives.

I had two approaches in mind to make isCustomError precise:

  1. As we did in earlier versions of defekt, check whether all expected properties exist and have the correct types. Basically manual type-checking. This works and keeps compatibility with other libraries that want to build error objects that are structurally compatible with our CustomErrors, but it is rather complicated and error-prone.
  2. Convert our CustomError interface to a class that inherits from Error and use it as the base class for any further custom errors. This allows us to check for ex instanceof CustomError in isCustomError. This is I think the slightly less elegant solution, since it relies on the prototype-chain, which is one of my least favorite features of JS. But in this case I think it works, since the assumption of defekt is that any project will use only defekt to create its custom errors and will only use isCustomError to recognize them. In that case structural type-checking and compatibility are not relevant and any error comparison logic goes through defekt anyway. Also, isCustomError is much simpler and less error-prone this way.

So this PR goes with option #2.

This PR also rewrites isError using the same approach - checking wether something is an object and then relying on instanceof Error. This fulfills all of our tests and has less surface for bugs.

@goloroden goloroden enabled auto-merge (squash) May 18, 2021 08:29
@goloroden goloroden merged commit db33b8b into main May 18, 2021
@goloroden goloroden deleted the fix/is-error branch May 18, 2021 08:30
goloroden pushed a commit that referenced this pull request May 18, 2021
## [7.1.2](7.1.1...7.1.2) (2021-05-18)

### Bug Fixes

* IsCustomError now works as intended; IsError is now simpler. ([#309](#309)) ([db33b8b](db33b8b))
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.

2 participants