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

Error thrown in xhr.onreadystatechange when javascript execution is suspended/resumed on iOS #5426

Closed
ilplotkin opened this issue Dec 16, 2013 · 9 comments · Fixed by #5603
Closed

Error thrown in xhr.onreadystatechange when javascript execution is suspended/resumed on iOS #5426

ilplotkin opened this issue Dec 16, 2013 · 9 comments · Fixed by #5603
Assignees
@tbosch
Labels
component: $http frequency: moderate severity: regression type: bug
Milestone
1.3.x - superlumi...

Comments

@ilplotkin
Copy link

On iOS, javascript execution is suspended when user switches to another tab in browser, closes browser, performs back/forward using swipe gestions.
In our application, if http request is occurred when javascript execution is suspended/resumed, sometimes (apprx one time per dozen tries) the following error is thrown:

TypeError: 'null' is not an object (evaluating 'xhr.readyState')

  src/ng/httpBackend.js
  xhr.onreadystatechange = function() {
    if (xhr.readyState == 4) {  // Error is thrown in this line

Debugging showed that xhr.onreadystatechange was invoked several times with readyState=4. On the first invocation xhr was nullified in completeRequest function, and then, on subsequent invocations, error was thrown.

Unfortunately, I could not reproduce the problem in small example. But I found that jQuery and Zepto have appropriate guards. Please see, for example, madrobby/zepto#633

The following change is proposed:
ilplotkin@a79004c
@IgorMinar
Copy link
Contributor

very odd corner case

I'm triaging the issue for the next release. thanks for all the info and suggested fix.

@kole
Copy link

kole commented Dec 20, 2013

I can confirm this bug using 1.1.5. Attaching screenshot from sentry for reference.
readystate

@ghost ghost assigned tbosch Dec 30, 2013
@tbosch tbosch mentioned this issue Jan 2, 2014
Merged
@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

I could reproduce this bug with:

  • iOS Simulator
  • Backend that takes a long time to resolve an xhr

Procedure:

  1. Create a test page that does an xhr
  2. Send the browser into the background by going to the iOS start screen
  3. Wait until the request is resolved
  4. Open up the browser again.

Note: The iOS remote debugger somehow didn't show the error. Had to add manual try/catch inside the onreadystatechange function to see it...

@tbosch tbosch closed this as completed in 4f57236 Jan 2, 2014
@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Thanks for submitting this issue!

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@tbosch @jamesdaily
fix($httpBackend): Ignore multiple calls to onreadystatechange with r…
d5374bd 
…eadyState=4

On mobile webkit `onreadystatechange` might by called multiple times
with `readyState===4`  caused by xhrs that are resolved while the app is
in the background.

 Fixes angular#5426.
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@tbosch @jamesdaily
fix($httpBackend): Ignore multiple calls to onreadystatechange with r…
21bcf2a 
…eadyState=4

On mobile webkit `onreadystatechange` might by called multiple times
with `readyState===4`  caused by xhrs that are resolved while the app is
in the background.

 Fixes angular#5426.
@peleteiro peleteiro mentioned this issue Mar 3, 2014
Closed
@davidjnelson
Copy link
Contributor

Reported as still occuring: #6530

@davidjnelson davidjnelson reopened this Mar 4, 2014
@davidjnelson davidjnelson modified the milestones: Backlog, 1.2.7 Mar 4, 2014
@peleteiro peleteiro mentioned this issue Mar 6, 2014
Closed
@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.3, Backlog Mar 17, 2014
@btford btford modified the milestones: 1.3.0-beta.4, 1.3.0-beta.3 Mar 24, 2014
@jeffbcross jeffbcross modified the milestones: 1.3.0-beta.6, 1.3.0-beta.5 Apr 3, 2014
@tbosch
Copy link
Contributor

tbosch commented Apr 14, 2014

Hi,
I can't reproduce this problem any more.

  1. We have a test that does exactly this: trigger onreadystatechange multiple times, and it works correctly (see https://github.com/angular/angular.js/blob/master/test/ng/httpBackendSpec.js#L123)
  2. I tried to reproduce this using the procedure described above and it did not show the error, as it did before we made the changes.

Here is the sample code that can be used:

Put this into test.html in the angular.js root directory

<!doctype html>
<html ng-app ng-controller="TestCtrl">
<head>
    <meta name="viewport" content="width=device-width, user-scalable=no">
</head>
<body>
<button ng-click="start()">Start</button>
<div>{{state}}</div>

<script src="build/angular.js"></script>
<script>
window.onerror = function() {
    alert('error');
}
function TestCtrl($scope, $http) {
    $scope.start = function() {
        $scope.state = 'started'
        $http.get('/wait').success(function() {
            $scope.state = 'done'
        });
    }
}
</script>

</body>
</html>

Change the Gruntfile.js in angular.js root directory:

   ....
     connect: {
      devserver: {
          ....
          middleware: function(connect, options){
            return [
                ....
              function(req, resp, next) {
                if (req.url === '/wait') {
                  global.setTimeout(function() {
                    resp.end('OK', 200);
                  }, 5000);
                } else {
                  next();
                }
              },
              connect.static(options.base),
              connect.directory(options.base)
            ];
          }

@peleteiro Could you provide more information how you reproduced this?

@peleteiro
Copy link

It's hard to reproduce. It happens when you close IOS Safari in the middle of the request. You need help from a getsentry.com kind of tool to get it.

But it is well known. If fact this fix was taken from Zepto.js.

@tbosch
Copy link
Contributor

tbosch commented Apr 14, 2014

@peleteiro I can reproduce this error without the corresponding fixes that we added to Angular.js (see https://github.com/angular/angular.js/blob/master/src/ng/httpBackend.js#L82). That's why we added them in the first place. However, I can't reproduce the error now any more, with the fixes applied. Are you sure that the problem is still there in current Angular?

My testcase was as describe above (in my comment from 3 months ago).

@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.7, 1.3.0-beta.6 Apr 22, 2014
@tbosch
Copy link
Contributor

tbosch commented Apr 28, 2014

@peleteiro Waiting for your reply... Otherwise closing this again...

@caitp caitp modified the milestones: 1.3.0-beta.9, 1.3.0-beta.8 May 9, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0, 1.3.0-beta.9 May 12, 2014
@btford btford closed this as completed Jun 30, 2014
@realityking realityking mentioned this issue Sep 29, 2014
Closed
@YvonneYu YvonneYu mentioned this issue Nov 20, 2014
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tbosch tbosch

Labels
component: $http frequency: moderate severity: regression type: bug
Projects
None yet
Milestone
1.3.x - superluminal-nudge
Development

Successfully merging a pull request may close this issue.

fix($httpBackend): Ignore multiple calls to onreadystatechange with read... tbosch/angular.js
10 participants
@peleteiro @petebacondarwin @IgorMinar @davidjnelson @kole @jeffbcross @btford @tbosch @caitp @ilplotkin