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

Don't rethrow string errors in handleTransitionReject #20529

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

gitKrystan
Copy link
Contributor

@gitKrystan gitKrystan commented Aug 22, 2023

handleTransitionReject blindly rethrows error.error regardless of its type. While the code appears to assume that the error will be of type TransitionError, handleTransitionReject might be called with another error type.

Newer versions of EmberData will throw an error that has error.error (https://github.com/emberjs/data/blob/f0dd0d429052abe3ce49e4e87f3492691e62d523/ember-data-types/cache/document.ts#L59) and when this happens, this code is rethrowing the string/object error unexpectedly.

Based on https://github.com/tildeio/router.js/blob/9320c7b1c1e887e73afd3700cd0093b9f34c9e6b/lib/router/transition-state.ts#L124 it looks like the type for TransitionError should be Error, and generally it sames safe enough to rethrow as long as the error.error is itself an Error. :)

`handleTransitionReject` blindly rethrows `error.error` regardless of its type. While the code appears to assume that the error will be of type `TransitionError`, `handleTransitionReject` might be called with another error type.

Newer versions of EmberData will throw an error that has `error.error` (https://github.com/emberjs/data/blob/f0dd0d429052abe3ce49e4e87f3492691e62d523/ember-data-types/cache/document.ts#L59) and when this happens, this code is rethrowing the string/object error unexpectedly.

Based on https://github.com/tildeio/router.js/blob/9320c7b1c1e887e73afd3700cd0093b9f34c9e6b/lib/router/transition-state.ts#L124 it looks like the type for `TransitionError` should be `Error`, and generally it sames safe enough to rethrow as long as the `error.error` is itself an `Error`. :)

Newer versions of EmberData will throw an error that has `error.error`
@wagenet wagenet merged commit 40ba038 into emberjs:main Aug 23, 2023
17 of 18 checks passed
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.

3 participants