4

The following must be the most common error Angular.js developers have to troubleshoot at some point or another.

Error: $apply already in progress

Sometimes, a $scope.$apply call wasn't required after all, and removing it fixes the issue. Some other times, it might be needed, and so developers resort to the following pattern.

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

My question is, why doesn't Angular merely check for $scope.$$phase and return without doing anything when a $digest loop is in invoked, or when $scope.$apply is called, rather than complain and throw an error so insidiously hard to track down.

Couldn't Angular just check for $$phase itself at the entry points of any methods which trigger a $digest and save the consumers from having to go through the trouble?

Are there any implications in leaving misplaced $scope.$apply calls around, other than unwanted watches firing, which the check for $$phase would prevent?

Why isn't a$$phase check the default, rather than an oddity?

Background

I'm upgrading Angular from 1.2.1 to 1.2.6 and that exception is being thrown when the page loads, so I have no idea where it's originating from. Hence the question, why is it that we even have to track down this obscure sort of bug with no idea of where in our code to look for it?

bevacqua
  • 47,502
  • 56
  • 171
  • 285
  • You might want to check out $safeApply - https://github.com/yearofmoo/AngularJS-Scope.SafeApply as well as this issue: https://github.com/angular/angular.js/pull/5402 – KayakDave Dec 27 '13 at 18:11
  • See the background information I've just added. It's not about an specific piece of code, but rather about being able to trace it. – bevacqua Dec 27 '13 at 18:17
  • Also, perhaps their thinking is grounded in this [Angular anti-patterns](https://github.com/angular/angular.js/wiki/Anti-Patterns) comment: "Don't do `if (!$scope.$$phase) $scope.$apply()`, it means your `$scope.$apply()` isn't high enough in the call stack." – KayakDave Dec 27 '13 at 18:18

1 Answers1

8

If you ever get this error, then you're using $scope.$apply() in the wrong place. I think the reasoning behind this error is as follows: if you're calling $apply in the wrong place, you probably don't fully understand the reason it exists, and you may have missed places where it actually should be called.

If such lapses are allowed to go by silently (by always testing $scope.$$phase first, for example), it would encourage sloppy usage, and people might get holes in their "$apply coverage". That kind of bug (where a $scope change is not reflected in the view) is a lot harder to track down than this error message, which you should be able to debug by examining the stack-trace (see below).

Why does $apply exist?

The purpose of $apply is to enable automated two-way data binding between $scope and view, one of the main features of Angular. This feature requires that all code that may potentially contain a $scope modification (so, basically every piece of user-code) is run below an $apply call in the call-stack. Doing this properly is actually not that complicated, but I think it's not documented very well.

The main 'problem' is that Javascript can have any number of active call-stacks, and Angular is not automatically notified about new ones. A new call-stack is created whenever an asynchronous callback is triggered, e.g., from a click, a timeout, a file-access, a network response, and so on. It's very common to want to modify the $scope inside such a callback function. If the corresponding event was provided by Angular itself, everything will just work. But there are times when you'll want to subscribe to 'outside events'. Google Maps events, for example:

function Controller($scope) {
    $scope.coordinates = [];
    //...
    var map = new google.maps.Map(mapElement, mapOptions);
    google.maps.event.addDomListener(map, 'dblclick', function (mouseEvent) {
        $scope.coordinates.push(mouseEvent.latLng);
    });
}

http://jsfiddle.net/mhelvens/XLPY9/1/

A double-click on the map of this example will not trigger an update to the view because the coordinates are added to $scope.coordinates from an '$applyless' call-stack. In other words, Angular does not know about Google Maps events.

How and where to use $apply?

We can inform Angular about the event by using $scope.$apply():

function Controller($scope) {
    //...
    google.maps.event.addDomListener(map, 'dblclick', function (mouseEvent) {
        $scope.$apply(function () {
            $scope.coordinates.push(mouseEvent.latLng);
        });
    });
}

http://jsfiddle.net/mhelvens/XLPY9/2/

The rule is to do this first thing inside every callback function for an outside event. The "$apply already in progress" error is an indication that you're not following this rule. If you have to handle Google Maps events often, it makes sense to wrap this boilerplate code in a service:

app.factory('onGoogleMapsEvent', function ($rootScope) {
    return function (element, event, callback) {
        google.maps.event.addDomListener(element, event, function (e) {
            $rootScope.$apply(function () { callback(e); });
        });
    };
});

function Controller($scope, onGoogleMapsEvent) {
    //...
    onGoogleMapsEvent(map, 'dblclick', function (mouseEvent) {
        $scope.coordinates.push(mouseEvent.latLng);
    });
}

http://jsfiddle.net/mhelvens/XLPY9/3/

The onGoogleMapsEvent events are now Angular-aware, or 'inside events' if you will (I'm totally making these terms up, by the way). This makes your code more readable and allows you to forget about $apply in your day-to-day programming; just call the wrapper instead of the original event subscriber.

For a number of events, Angular has already done this for us. With the $timeout service, for example.

Debugging the "$apply already in progress" error

So let's say I use the wrapper, but, absentminded as I am, I still call $apply manually:

onGoogleMapsEvent(map, 'dblclick', function (mouseEvent) {
    $scope.$apply(function () { // Error: $apply already in progress
        $scope.coordinates.push(mouseEvent.latLng);
    });
});

http://jsfiddle.net/mhelvens/XLPY9/4/

This $apply call does not follow our placement rule (onGoogleMapsEvent events are not outside events, after all; we granted them 'insideness'). If you double-click on the map, you'll see the error appear in the logs, together with a stack-trace:

Error: [$rootScope:inprog] $apply already in progress
    ...
    at Scope.$apply (.../angular.js:11675:11)
    at http://fiddle.jshell.net/mhelvens/XLPY9/4/show/:50:16
    ...
    at Scope.$apply (.../angular.js:11676:23)
    at dl.ondblclick (http://fiddle.jshell.net/mhelvens/XLPY9/4/show/:33:24)
    ...

I've left only the relevant lines: the ones that refer to Scope.$apply. When you get this error message, you should always find two Scope.$apply calls on the stack (or Scope.$digest calls, which serve a similar function; $digest is called by $apply).

The topmost call indicates where we got the error. The other one indicates why. Go ahead and run the Fiddle with your Javascript console opened up. In this case we can conclude: "oh, this was an 'inside event' subscriber, I can just remove my manual $apply call". But in other situations you may find out that the call lower down on the stack is also not positioned properly.

And that's precisely why I believe the error is useful. Ideally you'd only get an error when you neglect to call $apply altogether. But if Angular could do that, there would be no need to call the function manually in the first place.

Community
  • 1
  • 1
mhelvens
  • 4,225
  • 4
  • 31
  • 55
  • This is a very *thorough* and **useful** answer (given that it can help solve existing problems), but I don't think it answers the actual question. The questioner may or may not appreciate the background and reasoning, but there is a simpler question there: why isn't `Scope#$apply` wrapped in a `$$phase` check? Wrapping that method's body in `if(!$scope.$$phase) { ... }` would eliminate many errors and have almost no overhead. It might not be *educational* for users, but it would certainly be as close to harmless as a line of code could be. – eyelidlessness Dec 28 '13 at 05:26
  • @eyelidlessness: Thanks for the feedback! The answer was in there, actually, in the sentence starting with: *"If such lapses are allowed to go by silently ..."*. But I agree it wasn't nearly as visible as it should be. I've now made it more explicit. --- I suppose, as I was writing this behemoth, I kind of lost sight of the main part of the question. ;-) – mhelvens Dec 28 '13 at 11:37
  • 1
    Thanks for the edit. Now I must explicitly disagree with your reasoning. The purpose of program exceptions is not to encourage best practice nor to discourage questionable practice (that's actually the role of warnings), and it's *certainly* not to use reverse psychology by creating error conditions in order to encourage the user to do more of the same elsewhere. The purpose of program exceptions is to alert the program to an inconsistent state and allow it to recover or fail. The Angular error does not do this, it punishes the user for, at worst, a noop. – eyelidlessness Dec 28 '13 at 21:32
  • @eyelidlessness: I would argue that `$apply` should *only* be used right inside 'foreign' callbacks, to notify Angular of new call-stacks. Using it anywhere else is not just bad practice; it's *wrong* and should be seen as an error. Unfortunately, Angular (or any JS library) is limited in what it can detect. Two `$apply` calls on the stack, though, indicates that at least one of them is placed incorrectly, so this would warrant an error. But... Javascript doesn't have a proper `assert` or `error` statement. An exception is the best Angular can do; at least it gives us a stack-trace. – mhelvens Dec 28 '13 at 21:50
  • 1
    We should stipulate that "should be seen as an error", in terms of exceptions, has a meaning. A program should throw an exception when it will be caused to enter an inconsistent or unreliable state. This is not the case with Angular's behavior. It throws an exception, ignores the `$apply` call, then continues its current $digest phase. This (ugly but simple) example demonstrates the behavior: http://jsfiddle.net/PT6W8/ - observe that the actual program behaves as expected. Removing the needless call to `$apply` does not alter behavior: http://jsfiddle.net/7uk2b/ (cont'd) – eyelidlessness Dec 29 '13 at 00:23
  • I am genuinely curious if it is even *possible* to violate the program state with `$apply()`. I welcome an example. Angular is absolutely developed by better programmers than myself. But in this case, I think this is a poor way to educate users on a very specific best-practice around an issue that carries special cultural weight due to the way Angular data-binding works. – eyelidlessness Dec 29 '13 at 00:26
  • @eyelidlessness: There is no way to violate the program state with `$apply` *because* the exception prevents nested digest cycles. Yes, they could use the `$scope.$$phase` test instead, but that's true for *any* exception use-case: it could be rewritten not to use exceptions. --- And when I say "should be seen as an error", I mean "should be seen as an invalid program that shouldn't pass static verification". This is a choice of the API / language designer. I agree that ideally, exceptions should not play that role. But Javascript provides no alternative means to report the error. – mhelvens Dec 29 '13 at 11:06
  • I don't think there's any error to report, that's the point. It is already doing the part that resolves the exception (checking `$scope.$$phase`). Throwing an exception at that point is not reporting an **actual** error, it's creating noise in the log. – eyelidlessness Dec 29 '13 at 20:06
  • @eyelidlessness: OK, we're officially going in circles. [`goto 31228376;`](http://stackoverflow.com/questions/20805534/why-doesnt-angular-ignore-subsequent-digest-invocations/20810496?noredirect=1#comment31228376_20810496) :-) – mhelvens Dec 29 '13 at 20:58
  • I think you must have missed my point. If Angular has *already resolved* the error condition (it checked `$scope.$$apply`), there *is no exception state*. An exception is **incorrect**, because Angular has already recovered from the inconsistent state. You're right that it is the choice of Angular's developers to report this way, but it *doesn't* make them correct. – eyelidlessness Dec 29 '13 at 21:10