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

Proposal: Consider removing casts to Error when catching #87

Open
PatrickShaw opened this issue Feb 9, 2024 · 0 comments · May be fixed by #97
Open

Proposal: Consider removing casts to Error when catching #87

PatrickShaw opened this issue Feb 9, 2024 · 0 comments · May be fixed by #97

Comments

@PatrickShaw
Copy link

Was investigating this library and found that orWhen accepts an Error. I took a look at the internals of this library and found that there's a cast to Error in

const handled = this.errorFilter(error as Error);

From what I can see there's technically no guarantee that what's being throw is an Error. Someone could, for instance, if they really wanted do, say throw 'some error message' - Not ideal but sometimes we have no control over what's being thrown as the thrown error might live in, say, a third party library.

As such, I propose that we replace the use of Error + the cast to unknown

ghost91- added a commit to ghost91-/cockatiel that referenced this issue Jul 20, 2024
Instead, errors are now consistently typed as `unknown`.

BREAKING CHANGE: The types of errors have been changed from `Error` to unknown because they are not statically known (in JavaScript, _anything_ can be thrown).
If you know that in a particular situation, errors can only be instances of
`Error`, you can do the type assertion yourself (`error as Error`). Otherwise,
you should check whether an error is actually an `Error`, for example with
`error instanceof Error`.

Closes connor4312#87
@ghost91- ghost91- linked a pull request Jul 20, 2024 that will close this issue
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 a pull request may close this issue.

1 participant