Skip to content

fix(zone.js): fix zone for Jasmine 3.3.#31497

Closed
vikerman wants to merge 1 commit intoangular:masterfrom
vikerman:zone_jasmine
Closed

fix(zone.js): fix zone for Jasmine 3.3.#31497
vikerman wants to merge 1 commit intoangular:masterfrom
vikerman:zone_jasmine

Conversation

@vikerman
Copy link
Copy Markdown
Contributor

If zonejs is sent undefined callbacks it proceeds to attempt to call them, then fails, catches it own fail, rewrites the stack to hide the mistake, and reports a TypeError with a callstack unrelated to inputs.

Throw early if the callback is undefined (as can happen if JS or any-ified TS calls zone invokeTask).

Check for undefined onComplete callback to zonejs jasmine wrapper.

If zonejs is sent undefined callbacks it proceeds to attempt to call them, then fails, catches it own fail, rewrites the stack to hide the mistake, and reports a TypeError with a callstack unrelated to inputs.

Throw early if the callback is undefined (as can happen if JS or any-ified TS calls zone invokeTask).

Check for undefined onCommplete callback to zonejs jasmine wrapper.
@vikerman vikerman requested review from a team, JiaLiPassion and alexeagle July 10, 2019 22:42
Copy link
Copy Markdown
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are LGTM.
And is there any issue to describe how to reproduce this undefined callback?

@JiaLiPassion JiaLiPassion added the area: zones Issues related to zone.js label Jul 11, 2019
@ngbot ngbot Bot added this to the needsTriage milestone Jul 11, 2019
@vikerman
Copy link
Copy Markdown
Contributor Author

This breaks tests internal to Google when upgraded to Jasmine 3.4 and jasmine.ts was breaking by passing an undefined callback and this issue was discovered.

@vikerman vikerman added target: major This PR is targeted for the next major release action: merge The PR is ready for merge by the caretaker labels Jul 12, 2019
@vikerman vikerman removed the request for review from alexeagle July 12, 2019 17:26
@matsko matsko closed this in c4c340a Jul 12, 2019
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
If zonejs is sent undefined callbacks it proceeds to attempt to call them, then fails, catches it own fail, rewrites the stack to hide the mistake, and reports a TypeError with a callstack unrelated to inputs.

Throw early if the callback is undefined (as can happen if JS or any-ified TS calls zone invokeTask).

Check for undefined onCommplete callback to zonejs jasmine wrapper.

PR Close angular#31497
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: zones Issues related to zone.js cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants