This repository was archived by the owner on Apr 12, 2024. It is now read-only.
fix($compile): do not swallow thrown errors in test#15631
Closed
gkalpak wants to merge 1 commit into
Closed
Conversation
In e13eeab, errors/rejections produces during fetching the template or compiling an asynchronous directive, where overzealously silenced. This doesn't make any difference in (most) production apps, where `$exceptionHandler` does not rethrow the errors. In tests though (where `$exceptionHandler` rethrows by default), it can unexpectedly "swallow" thrown errors. This commit fixes it by removing the extraneous `.catch(noop)`, thus letting errors thrown by `$exceptionHandler` to surface. The changes in 'compileSpec.js' essentially revert the modifications that were unnecessarily (and incorrectly) done in e13eeab (and also one incorrect modification from [c22615c][1]). [1]: angular@c22615c#diff-348c2f3781ed66a24894c2046a52c628L2084 Fixes angular#15629
Contributor
|
Is this a breaking change? Even when this is only for tests (and I am ok in
having it in the current stable branch).
|
Member
Author
It depends how you look at it 😁 First of all, this will have no effect in cases where In cases where tl;dr |
Contributor
|
I am fine with the explanation for not calling this a BR. LGTM |
Contributor
|
LGTM |
gkalpak
added a commit
that referenced
this pull request
Jan 30, 2017
In e13eeab, errors/rejections produced during fetching the template or compiling an asynchronous directive, where overzealously silenced. This doesn't make any difference in (most) production apps, where `$exceptionHandler` does not rethrow the errors. In tests though (where `$exceptionHandler` rethrows by default), it can unexpectedly "swallow" thrown errors. This commit fixes it by removing the extraneous `.catch(noop)`, thus letting errors thrown by `$exceptionHandler` to surface. The changes in 'compileSpec.js' essentially revert the modifications that were unnecessarily (and incorrectly) done in e13eeab (and also one incorrect modification from [c22615c][1]). [1]: c22615c#diff-348c2f3781ed66a24894c2046a52c628L2084 Fixes #15629 Closes #15631
ellimist
pushed a commit
to ellimist/angular.js
that referenced
this pull request
Mar 15, 2017
In e13eeab, errors/rejections produced during fetching the template or compiling an asynchronous directive, where overzealously silenced. This doesn't make any difference in (most) production apps, where `$exceptionHandler` does not rethrow the errors. In tests though (where `$exceptionHandler` rethrows by default), it can unexpectedly "swallow" thrown errors. This commit fixes it by removing the extraneous `.catch(noop)`, thus letting errors thrown by `$exceptionHandler` to surface. The changes in 'compileSpec.js' essentially revert the modifications that were unnecessarily (and incorrectly) done in e13eeab (and also one incorrect modification from [c22615c][1]). [1]: angular@c22615c#diff-348c2f3781ed66a24894c2046a52c628L2084 Fixes angular#15629 Closes angular#15631
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.
What is the current behavior? (You can also link to an open issue here)
Since e13eeab, errors/rejections produces during fetching the template or compiling an asynchronous directive, where overzealously silenced. This doesn't make any difference in (most) production apps, where
$exceptionHandlerdoes not rethrow the errors. In tests though (where$exceptionHandlerrethrows by default), it can unexpectedly "swallow" thrown errors.See also #15629.
What is the new behavior (if this is a feature change)?
The extraneous
.catch(noop)has been removed, thus letting errors thrown by$exceptionHandlerto surface.Does this PR introduce a breaking change?
No
(It is possible that exceptions might pop up in tests, but they where previously incorrectly hidden. Also note that the only modification in tests were to restore them to their pre-1.6.0-rc.0 state.)
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information:
The changes in 'compileSpec.js' essentially revert the modifications that were unnecessarily (and incorrectly) done in e13eeab (and also one incorrect modification from c22615c).
Fixes #15629