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

No pending request to flush ! errors when moving from 1.1.3 to 1.1.4 #2431

Closed
christianvuerings opened this issue Apr 17, 2013 · 29 comments
Closed
Assignees

Comments

@christianvuerings
Copy link
Contributor

We're currently trying to migrate from 1.1.3 to 1.1.4.
While doing so, many of our Ajax logic tests are failing with the No pending request to flush ! error.

When debugging our own code, it looks like the success method isn't being called anymore when doing a request:

$http.post('/api/' + authorizationService + '/remove_authorization').success(function(){
  $scope.user.profile['has_' + authorizationService + '_access_token'] = false;
});

(also tried then instead of success, but no luck)

The cause of this error is probably 4ae4681 where there are a lot of changes to the $http service.

Debugging the angular source code a bit more, it seems like the serverRequest function in http.js doesn't get executed in 1.1.4, which means sendReq never gets fired.

In 1.1.3 does get fired so the success call actually did happen then.

Has anyone seen this as well and is there a way to fix this?

Example code: https://github.com/ets-berkeley-edu/calcentral/blob/v7/app/assets/javascripts/angular/controllers/calcentralController.js#L90
Example test: https://github.com/ets-berkeley-edu/calcentral/blob/v7/spec/javascripts/calcentral/calcentralControllerSpec.js#L47

@bennick
Copy link

bennick commented May 6, 2013

I can confirm this bug. Here is a spec file:

'use strict';

describe("HomeController", function() {
  // Set Up App and Controller
  var $httpBackend, $rootScope, $controller, $timeout, $scope;

  beforeEach(module('app'));

  beforeEach(inject(function($injector) {
    $httpBackend = $injector.get('$httpBackend');
    $controller = $injector.get('$controller');
    $rootScope = $injector.get('$rootScope')
    $timeout = $injector.get('$timeout');

    $scope = $rootScope.$new();

    // mock the GET quotes.json request
    $httpBackend.when('GET', '/quotes.json').respond([
      {text: "text", author: "The Ward", author_description: "a dude"}, 
      {text: "text", author: "The Brian", author_description: "a dude"}, 
      {text: "text", author: "The Ryan", author_description: "a dude"}]);
  }));

  afterEach(function() {
    $httpBackend.verifyNoOutstandingExpectation();
    $httpBackend.verifyNoOutstandingRequest();
  });

  // Tests
  it("retrives all of the quotes from Rails", function() {
    $controller('HomeController', {$scope: $scope});
    expect($scope.quotes.list.length).toEqual(0);
    $httpBackend.flush();
    expect($scope.quotes.list.length).toEqual(3);
  });
});

With angular 1.1.3 this passes fine. If I then change to 1.1.4 I recieve the following error:

Chrome 26.0 (Mac) HomeController retrives all of the quotes from Rails FAILED
    Error: No pending request to flush !
        at Error (<anonymous>)
        at Function.$httpBackend.flush (/Users/rbennick/Projects/sellrest/spec/javascripts/lib/angular/angular-mocks.js:1171:34)
        at null.<anonymous> (/Users/rbennick/Projects/sellrest/spec/javascripts/controllers/home_controller_spec.js:33:18)

@christianvuerings
Copy link
Contributor Author

@bennick thanks for adding your test!

@mokesmokes
Copy link

Here are more reports of this issue: #2371 and #2557

@c0bra
Copy link

c0bra commented May 16, 2013

Yet another report: #2438

@aflock aflock mentioned this issue May 16, 2013
@christianvuerings
Copy link
Contributor Author

Talking to the AngularJS team, gave me the following information. The reason for this change was that you're now able the chain Promises. Before they were resolved automatically but not chainable.

The way we should be able to fix this is by adding a $scope.apply() after the $httpBackend.when. Haven't tested this yet but will definitely try this out.

@mokesmokes
Copy link

@ChristianV, this is quite strange.... so $http and $resource behavior is changed, I would expect some code samples at the very least..... this is a major breaking change.

@aflock
Copy link

aflock commented May 17, 2013

@ChristianV I also find it strange, since the chainable interceptors change could have been made while preserving current behavior (see @c0bra 's gist here https://gist.github.com/c0bra/5594851 and corresponding discussion in issue #2438). Perhaps the angularjs team could provide some guidance here?

@c0bra
Copy link

c0bra commented May 19, 2013

What I think is reasonable and "Do What I Mean" is that a straight
$http.get runs immediately. If instead I want to queue things up there
could be an $http.enqueue('GET', ...) method, and any enqueued requests
could be flushed on the next tick.

@lukemadera
Copy link

+1 for this issue. @ChristianV Thanks for the idea on $scope.$apply(). It did NOT work for me in the test (after the .when() ) but if I put it in my code after the $http() call the test passed again. Similar to: #2371

$http(...)
.success(function(response) {..})
.error(function(response) {..})
;
if(!$rootScope.$$phase) {
  $rootScope.$apply();    //AngularJS 1.1.4 fix
}

Then my tests passed as before the 1.1.4 upgrade (with no changes to my actual test files).

I wrap all my AJAX api calls in a function so I only have 1 instance of $http in my app so it was easy to make the change of adding in $rootScope.$apply(); in one place for me but obviously would be super annoying if I had to change it in many places throughout the app..

@christianvuerings
Copy link
Contributor Author

@lukemadera thanks for testing it out. Just as you've found, we would have to add

if(!$rootScope.$$phase) {
  $rootScope.$apply();
}

after every $http request and it doesn't work if we add this in the tests themselves.
This definitely feels like a hack and it would be good to have a better solution soon.
@IgorMinar @btford @mhevery

@mhevery
Copy link
Contributor

mhevery commented May 28, 2013

Interceptors used to be synchronous, and did not take advantage of Promises. To make them more flexible we have changed them to use promises. Since promises are async you need to do $rootScope.$digest() in your tests to get them going. This is a test change only, not a production code change. Since production code should always execute insid an $apply().

Yes, this is a breaking change, but the 1.1.x branch is the development branch, where breaking changes are to be expected. The change should only require a change to your tests, and not to your production code.

Doing this is wrong since it puts it in the production code not in tests.

if(!$rootScope.$$phase) {
  $rootScope.$apply();    //AngularJS 1.1.4 fix
}

PS: I don't see how you can have promis based interceptors and not have to do extra $digest in your test.

@christianvuerings
Copy link
Contributor Author

@mhevery Makes much more sense, adding a $rootScope.$digest() in our tests did the trick.
Thanks for taking the time and responding to this.

Closing this issue since there is a good solution for it.

@bclinkinbeard
Copy link
Contributor

production code should always execute insid an $apply().

That isn't always the case though is it? Hence #2438, #2794 and others. @mhevery I can't really tell from your post whether you are saying there are remaining bugs causing those other issues or not.

@mokesmokes
Copy link

@mhevery, if an $http, $resource call needs to be wrapped in $apply, I suggest this be specified in the docs, especially given that this breaking change was not required previously. It's entirely unintuitive that we need to be in Angular context to send an XHR request (e.g. compare $http to $timeout). There should be an effort to make the APIs behave in consistent fashion when possible.... or at least clearly document the use cases, highlighting changes and differences. This issue is a major headache and source of confusion - see all the similar bug reports.....

@mhevery
Copy link
Contributor

mhevery commented May 30, 2013

I am sorry for the confusion. Let me better explain.

There should be no need to call $apply in the production code to get the $http to work, provided that you are calling $http from within angular context. The way you can tell is to put a break point to where the call to $http is made and look at the stack trace. You should already be in the $apply function.

If you see $apply in the stack trace, then everything should just work.
If you don't see $apply then you managed to get out of the angular execution context somehow. You are probably getting called from setTimeout (instead from $timeout) or some other third party library. In wich case the solution is to wrap the callback in the $apply not to do $apply after call to $http.

@aflock
Copy link

aflock commented May 30, 2013

@mhevery thanks for the clarification

@mokesmokes
Copy link

@mhevery - if we want the success/error callbacks from $http/$resource, but needed $apply to fire the request (e.g. in 3rd party library callback) - how should this be done? Aren't the callbacks already in Angular context, thus being in the $apply wrap may be a bug - no? Also, please note this code has changed quite a bit, yet the 1.1.5 $resource docs (for example) are very much lacking..... new APIs with no examples, etc.

@IgorMinar
Copy link
Contributor

@bennick what does your controller look like?

I can't seem to be able to reproduce your issue with this:

'use strict';

angular.module('app', []).controller('HomeController', function($http, $scope) {
  $scope.quotes = {list: []};

  $http.get('/quotes.json').then(function(response) {
    $scope.quotes.list = response.data;
  })
});


/////////////


describe("HomeController", function() {
  // Set Up App and Controller
  var $httpBackend, $rootScope, $controller, $timeout, $scope;

  beforeEach(module('app'));

  beforeEach(inject(function($injector) {
    $httpBackend = $injector.get('$httpBackend');
    $controller = $injector.get('$controller');
    $rootScope = $injector.get('$rootScope')
    $timeout = $injector.get('$timeout');

    $scope = $rootScope.$new();

    // mock the GET quotes.json request
    $httpBackend.when('GET', '/quotes.json').respond([
      {text: "text", author: "The Ward", author_description: "a dude"},
      {text: "text", author: "The Brian", author_description: "a dude"},
      {text: "text", author: "The Ryan", author_description: "a dude"}]);
  }));

  afterEach(function() {
    $httpBackend.verifyNoOutstandingExpectation();
    $httpBackend.verifyNoOutstandingRequest();
  });

  // Tests
  it("retrives all of the quotes from Rails", function() {
    $controller('HomeController', {$scope: $scope});
    expect($scope.quotes.list.length).toEqual(0);
    $httpBackend.flush();
    expect($scope.quotes.list.length).toEqual(3);
  });
});

@bennick
Copy link

bennick commented Jun 5, 2013

@IgorMinar Here is my HomeController

'use strict';

App.controller('HomeController', ['$scope', '$timeout', '$http', function($scope, $timeout, $http) {
  $scope.quotes = {}
  $scope.quotes.index = 0;
  $scope.quotes.list = [];
  $scope.quotes.id = "quotes_current_element"
  // Load the Quotes
  $http({method: 'GET', url: '/quotes.json'}).
    success(function(data, status, headers, config) {
      $scope.quotes.list = data;
      $scope.quotes.current = $scope.quotes.list[0];
      setTimeout($scope.changeQuote, 5000);
    })


  // FUNCTIONS
  $scope.changeQuote = function() {
    var next = $scope.quotes.index + 1;

    // For testing
    $scope._next = next;

    if (next >= $scope.quotes.list.length) {
        next = 0;
    }

    $('#' + $scope.quotes.id).fadeOut(1000, 'swing', function() {
        $scope.$apply($scope.quotes.current = $scope.quotes.list[next]);
        $scope.$apply($scope.quotes.index = next);

        $('#' + $scope.quotes.id).fadeIn(1000, function() {
          setTimeout($scope.changeQuote, 5000);
        });


    });
  }

}]);

This was the first angular controller I ever wrote so please excuse the dom manipulation code. I have since improved and know better now.

@IgorMinar
Copy link
Contributor

@bennick I still can't reproduce the error.

Please check out this plunk: http://plnkr.co/edit/8l6zczMzJ69D24vhwblq?p=preview

Can you fork it and get it to fail in the way you described in this ticket?

@IgorMinar
Copy link
Contributor

I'm starting to wonder if you didn't use older version of angular-mocks.js with the latest angular.js file. in that case you could end up seeing this issue.

That one is easy to reproduce: http://plnkr.co/edit/vKPfjoAjCl76hOvsXE5N?p=preview

But that's not a bug.

@bunions1
Copy link

One easy way to get outside of an angular context and run into this issue in non-test/production code is to try and do
an $http.get() within an underscore.js _.debounced function. Tripped me up for a while.

ie:
var debouncedGet = _.debounce(function(){
$http.get("/something").success(function(){alert('this doesn't happen'});
})
debouncedGet();

@lambdahands
Copy link

+1 -- Good God this gave me a massive headache. Was using a raw xhr request to perform custom file uploads, and 1.1.4+ broke my callback behavior. Wrapping my $http callback into $scope.$apply() did the trick.

Thanks so much for this solution!

@toxaq
Copy link

toxaq commented Jul 17, 2013

Wow, totally unexpected and very breaking change IMO. Definitely should go in the "breaking changes" section that XHR requests are now part of the angular lifecycle. I don't think "support request/response promise chaining" really covers how significant this is. I had a custom directive that made $http requests and this just stopped them all dead in their tracks.

@petebacondarwin
Copy link
Member

@IgorMinar - I think this change has caused enough confusion that we need to highlight it in the documentation. Also what about the idea of only requiring $q resolves (i.e. $digest) if there are request interceptors in place?

@mokesmokes
Copy link

@IgorMinar
Copy link
Contributor

@petebacondarwin I'd rather think of fixing this for all promises rather than special-casing one scenario in $http

@petebacondarwin
Copy link
Member

@IgorMinar - I think this is a special case. What is the general issue with $q? Or are you saying that we should make all resolve\reject calls synchronous and not go through the evalAsync queue at all?

jamestalmage added a commit to promise-testing/angular.js that referenced this issue Aug 10, 2013
…phase

Add $rootScope.$apply() call upon resolve/reject if outside a digest phase

Closes angular#2431
@jamesguitar3
Copy link

Hi guys, I am new to AngularJS and was working on a code using $http with v1.0.7, but found it quit working after upgraded to v1.1.5. Below is the code I had it working in v1.0.7 and I can call this hellorworld() from outside the Angular controller. Can someone please let me know how can I make it work in v1.1.5 and to call this function from outside? Thank you.


//Angular Controller
function UserListController($scope, $http) {
$scope.users = [];
$scope.test = function (){
alert("test");
}
//Test the exception threw by the WCF
$scope.helloworld = function () {
$http.post('/_vti_bin/Project/Service.svc/HelloWorld', { 'Content-type': 'application/json' })
.success(function (data) {
//do something after received the data
})
.error(function (XMLHttpRequest, StatusCode, headers, config) {
});
}
});


//Call the function from outside of the Angular controller
angular.element($("#userListDiv")).scope().helloworld();

jotbe added a commit to jotbe/angular-stockwatch that referenced this issue Nov 3, 2013
- Improved the controller tests by using mockup JSON data, registered in Angular.
- Created (only) empty basic code to test the watchlist service. Due to some changes in Angular 1.1.4 the httpBackend approach failed with "Error: No pending request to flush" and so far I could not find a way to deal with it. See: angular/angular.js#2431
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet