fix: IsCustomError now works as intended; IsError is now simpler. #309
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed in the wolkenkit that
isCustomError
would think that basicError
s wereCustomError
s and realized that the implementation did not check enough properties to avoid false positives.I had two approaches in mind to make
isCustomError
precise: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 ourCustomError
s, but it is rather complicated and error-prone.CustomError
interface to a class that inherits fromError
and use it as the base class for any further custom errors. This allows us to check forex instanceof CustomError
inisCustomError
. 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 ofdefekt
is that any project will use onlydefekt
to create its custom errors and will only useisCustomError
to recognize them. In that case structural type-checking and compatibility are not relevant and any error comparison logic goes throughdefekt
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 oninstanceof Error
. This fulfills all of our tests and has less surface for bugs.