Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix($httpBackend): cancelled JSONP requests should not throw error #5616

Closed
wants to merge 1 commit into from

Conversation

arty-name
Copy link
Contributor

When you cancel a JSONP request, angular deletes the callback for it. However the script still executes, and since the callback is now deleted and undefined, the script throws an exception visible in the console. The quick fix for this is not to delete the callback, but replace it with angular.noop.

Closes #5615

…he console

When you cancel a JSONP request, angular deletes the callback for it. However the script still executes, and since the callback is now deleted and undefined, the script throws an exception visible in the console. The quick fix for this is not to delete the callback, but replace it with `angular.noop`.

Closes angular#5615
@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@arty-name
Copy link
Contributor Author

Hi Igor. I've signed Google CLA on 2008-02-29 as Artemy Tregubenko or Artemy Tregoubenko. I suppose the email was [email protected], but it could have been [email protected] as well. Should I change it to [email protected]?

@ghost ghost assigned IgorMinar Jan 4, 2014
@IgorMinar
Copy link
Contributor

you can just sign again with the same email address as you used in this commit

@IgorMinar
Copy link
Contributor

please change the first line of the commit message to:

fix($httpBackend): cancelled JSONP requests should not throw error

@IgorMinar
Copy link
Contributor

instead of using angular.noop, just set it to a simple fn that will remove the callback from the array when called. that way we are not leaving callbacks in that callbacks object.

@IgorMinar
Copy link
Contributor

otherwise LGTM

@ghost ghost assigned jeffbcross Jan 6, 2014
@arty-name
Copy link
Contributor Author

Regarding the usage of angular.noop, I believe we have choice of two options here. One is currently committed, and its downside is that the number of keys in the callbacks object grows, but all of them have the same object as value. Another option is to have self-removing functions as values, but each function would be different, because each of them needs to create a closure to store the key in. I believe the second options would be much more memory-inefficient because of all these closures and the fact that the callbacks will not necessarily be called. However I do not have a strong opinion here and just want to bring this up. What do you think?

@arty-name
Copy link
Contributor Author

I have signed the CLA with the same email address [email protected] which I have used for this commit.

Regarding the commit message, I honestly do not know how to update it after it have been pushed to GitHub. I can close this pull request and create a new one with correct data.

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@IgorMinar
Copy link
Contributor

alright. I'm fine with the change as is. thanks!

@ghost ghost assigned IgorMinar Jan 8, 2014
@IgorMinar IgorMinar closed this in 95e1b2d Jan 8, 2014
@arty-name arty-name deleted the abort-jsonp-error branch January 8, 2014 08:54
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…he console

When you cancel a JSONP request, angular deletes the callback for it. However the script still executes, and since the callback is now deleted and undefined, the script throws an exception visible in the console. The quick fix for this is not to delete the callback, but replace it with `angular.noop`.

Closes angular#5615
Closes angular#5616
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…he console

When you cancel a JSONP request, angular deletes the callback for it. However the script still executes, and since the callback is now deleted and undefined, the script throws an exception visible in the console. The quick fix for this is not to delete the callback, but replace it with `angular.noop`.

Closes angular#5615
Closes angular#5616
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aborted JSONP request prints an error in the console
3 participants