angular: *Blocking Issue*: Change detection after error creating infinite loop

@swftvsn I would agree. I’ve added the manual change detector detach workaround to my error handler. As indicated, it seems to work in dev mode. But it really seems like this is an issue that should be more thoroughly addressed.

I would argue that it should be extremely difficult (ideally, impossible) to throw the framework into an error loop where the error handler keeps getting called repeatedly. Yes, this workaround works in dev mode. But what about a live server using prod mode? Something as trivial as an invalid property call in a template should never be able to lock the browser. Angular can’t prevent the developer from writing bad code, like an infinite while loop, that has nothing to do with the error handler. But if the error handler is handling an error, the framework should be doing everything it can to prevent it from freezing the browser.

I’m not sure what the optimal solution would be though. Only do post-error change detection if the initial component tree has finished rendering? Some error counter that clears itself on an interval, which stops change detection if too many error handler calls are made within a short amount of time?

This issue should be one if preority ones. Ignoring this Angular team shows that they do not care. We have just released Angular based pwa to 12 countries and you could not imagine how much code should i write to hack that infinite loop problem and from time to time it still exists. This is a shame for such powerful framework. And i think this is not problem of devs this is a problem of distributing preorities.

@mhevery OK after a lot of frustrating trial and error, I think I have tracked down the problem. Here is a repro case. http://plnkr.co/edit/cXEvOObHAlmXer4HVtr4 (This Plunker uses a custom error handler to aggregate the errors and eventually error out of tick() after 50 sequential errors, just to prevent it from locking up the browser).

The culprits seem to be having an initial error (e.g. property not found) and using Renderer to add a global listener such as mouse move. It looks like using Renderer2 will avoid the problem, but I need to test further to make sure.

Obviously, having this scenario totally lock up the browser seems like a big problem. Especially given how insanely time-consuming it was to track down the apparent source of the problem. This particular issue may also reveal some wider set of scenarios that weren’t accounted for when the “post-error change detection” changes were made in 4.1.1.

Let me know what you think or if you have any questions. Thanks.

This bugs us daily, and when a dev forgets to turn off Airbrake… it literally gets filled with exceptions.

Anyway, even without that, it still makes it hard to close the browser, and we usually must kill it. The normal oops-an-exception -> check-the-exception -> fix-it -> verify loop goes to oops-an-exception -> try-to-screencap-or-copy-the-error -> kill-the-browser -> fix-it -> start-the-browser -> navigate-to-the-place-where-exception-happened -> cross-fingers-it-doesn't-happen-again -> verify

My assertion is that If this works as expected, the expectation is wrong. This is a problem and slows down development.

Thank you @brian428 for chasing this so hard!

This is bugging me too. I only get the behavior for template errors and when using NgZone in the custom error handler; which I have to do in order for the error handler to always show a MatSnackBar - but I imagine that this problem would occur with any ‘toast’ component you want to show for any front-end error.

This SO post suggest using NgZone - which works for most errors, but causes this behavior for template-related errors: https://stackoverflow.com/questions/49474788/angular-material-snackbar-and-custom-errorhandler-to-notify-error

Is there any progress on this? I totaly agree with brian428 - you through application in infinite loop because of angula rproblem. I run into same error when i am doing ngFor on

[
                { text: 'ttt', href: '/questions' },
                { text: 'ttt', href: '/questions' },
                { text: 'ttt', href: '/questions' }
            ]

which comes from server. And sometimes server sends

[
                { text: 'ttt', href: '/questions' },
                null,
                { text: 'ttt', href: '/questions' }
            ]

then i am starting to receive inifinite error loop with Cannot read property 'href' of null. I know this is issue with the server but imagine some secret bug appears which breaks structure - and this causes browser to hang? Really? I do not think this should happen.

This should find a way into the documentation somehow, because this will hit anyone who is using a non-existing variable in a template, if I got it right.

I have a relatively good solution for this. Use rxjs debounce and distinctUntilchanged!

@Injectable()
export class ErrorHandlerService implements ErrorHandler {
  errorSubject = new Subject();

  constructor(private injector: Injector) {
    this.errorSubject.pipe(debounceTime(250),
                           distinctUntilChanged())
    .subscribe((error: Error | any) => {
      console.error(error); 
      // error handling done here
      const debugCtx = error['ngDebugContext'];
      const changeDetectorRef = debugCtx && debugCtx.injector.get(ChangeDetectorRef);
      if (changeDetectorRef) {
        changeDetectorRef.attach();
      }
    }

  public handleError(error: Error | any): void {
    const debugCtx = error['ngDebugContext'];
    const changeDetectorRef = debugCtx && debugCtx.injector.get(ChangeDetectorRef);
    if (changeDetectorRef) {
      changeDetectorRef.detach();
    }
    this.errorSubject.next(error);
  }
}

this method disables change detection ( you can pause! ) but keeps usability by turning it back on after handling the error. After the first time, the debounce will reduce the frequency to every 250ms or longer because of the distinctUntilChanged operator. So change detection is not off for longer than that.

Since I implemented this, template errors have stopped freezing my laptop, and chrome task manager is no longer needed.

many Thanks to this issue’s participants for the change detection idea, it has the same issue of not working in production, but the debounce will work.

I think this should be higher than severity2: inconvenient. If the browser locks up in production, it would be a detrimental user experience and comparable to any other bug. Also, for those using error reporting tools (Airbrake, NewRelic, …etc), it will spam them and mess with metrics.

My current workaround:

export class ErrorHandlerService implements ErrorHandler {
  private previousErrors: string[] = [];

  handleError(error: Error) {
    const currentError = error.toString();
    if (this.previousErrors.indexOf(currentError) === -1) {
      // do stuff with error
      this.previousErrors.push(currentError);
    }
  }
}

This though has the limitation of hiding actual multiple occurrences of the same error.

@vincentsels , same behavior here.

I created a stackblitz to reproduce the problem:

https://stackblitz.com/edit/global-error-handler-problem?file=src%2Fapp%2Fapp.component.html

Any idea to solve this? Tks!

@mhevery (or anyone else), I spent more than 20 hours tracking down this bug, so it would be nice to get a response here. Thanks.

Thanks, I was not aware of that workaround and it’s definitely not something I would have figured out myself. I’ll try that to see if it helps. Since I’m really only modifying templates in dev mode, this may deal with the problem “well enough”.

To be clear, I’ve also seen cases where Angular does literally go into an infinite loop of errors, regardless of moving the mouse. So there’s probably more going on (either in Angular or in the 3rd party code triggering the problem). I just spent so long tracking this down that I thought identifying the listener problem would be enough to get someone on the right track since it seemed to capture at least part of the problem.

Even if this works “as expected”, it still seems like a problem. I totally get that an infinite loop might be unavoidable if something throws an error, and the error handler itself triggers change detection that throws another error. But that’s not what’s happening here. I’m not sure how to say this properly, but it seems like the post-error change detection should really only become active after the initial component tree finishes its initial rendering?

In other words, if something goes wrong while the page is initially being rendered that stops parts (or all) of the app from even being created, aren’t we already way beyond the idea that a user could “keep using the app”? In this case, the app isn’t even being initially rendered. So “continuing to use the app” isn’t even an option.

In any event, I’ll see if the workaround you gave is enough to bypass the problem, at least in dev mode. I’ll report back once I’ve tried it out.

@mhevery or @chuckjaz: Is there any estimate when this might be addressed? As I said earlier, I’m still suck with using 4.1.0 until this is dealt with. Thanks.

A temporary fix I came up with was to rethrow the TypeError and avoid calling the zone.run() function. This has prevented my application to hang on an infinite loop so far.

For example:

handleError(err: any): void {
    ...

    if (err instanceof TypeError) {
        // Fallback to console logging.
        console.err(err.message);
        console.err(err.stack);
        throw err;
    }

    ...

    zone.run(() => { ... } );
}

Hope this helps anyone.

I decided to turn rollbar back on, within 24 hours I overwhelmed the error limit.

@mhevery @matsko @angular would it be too much to ask, for a definitive answer on this? Considering how it results in the spamming of errors to error reporting systems, how on earth isn’t this considered a priority?

No instructors on how to mitigate it, or tweak a setting in the Angular engine to stop attempting to render a template after the nth error. Literally gone backwards from Angular 1 in this respect. I don’t understand how the Angular team can neglect such an important aspect of the framework like this.

@mhevery not trying to pester, but just wanted to check whether my Plunker adequately shows the problem? Just making sure, since right now I’m stuck on version 4.1.0 until this gets resolved or a workaround is found.

I spent another 4 or 5 hours tonight trying to track this down, and have a few updates:

• First, it’s definitely caused by the post-error change detection introduced in 4.1.1. If I roll back to 4.1.0, the recursive error doesn’t happen.
• I cloned my app and started commenting stuff out. I’m able to pare it down to a scenario where the recursive loop doesn’t happen, but I’m still not sure what the actual cause is. The basic trigger I’m finding goes something like this:
  • App shell component starts being created
  • Child components / route target component starts being created
  • Various async processes/Observables are kicked off in various component ngOnInit blocks (loading data, etc.)
  • It looks like since this is all happening in the context of the App shell being created, a template error anywhere down the chain triggers the errorHandler. Somehow these async processes seem to cause the infinite error loop.

I’m fairly sure it has to be related to the async processes being triggered through the component hierarchy during creation, because if I comment out the children or stop the async processes from firing, there is no infinite loop of errors. I just get the single expected error.

I’ll try to spend more time on it over the weekend and see if I can get it replicated in a Plunkr. If not, I can try “sanitizing” my app and zipping it up as a repro case.

@mhevery OK after a lot of frustrating trial and error, I think I have tracked down the problem. Here is a repro case. http://plnkr.co/edit/cXEvOObHAlmXer4HVtr4 (This Plunker uses a custom error handler to aggregate the errors and eventually error out of tick() after 50 sequential errors, just to prevent it from locking up the browser).

The culprits seem to be having an initial error (e.g. property not found) and using Renderer to add a global listener such as mouse move. ~It looks like using Renderer2 will avoid the problem, but I need to test further to make sure.~

Obviously, having this scenario totally lock up the browser seems like a big problem. Especially given how insanely time-consuming it was to track down the apparent source of the problem. This particular issue may also reveal some wider set of scenarios that weren’t accounted for when the “post-error change detection” changes were made in 4.1.1.

Let me know what you think or if you have any questions. Thanks.

This solution halts exceptions after limited amount of time but it does affect UI after halting errors. Is there a way that we can re-render UI after the errors are done producing?

any update?

I want custom error handler to display most errors in toast, but cannot do it now due to this problem. I have to do toast in every subscription error handler

the error to continuously get thrown when using ngZone run to display toast messages in custom error handler (e.g. in the case of a template error). The browser tab get crashed and unable to proceed further. please provide solution for this case asap !

@brian428 Thanks for providing the plunker! I looked into it, and am seeing the following:

1. If I click “Run” and don’t move the mouse over the preview window, I only see 1 error being logged to the console.
2. Once I move the mouse over the preview window, I see more errors being logged to the console. Probably 1 for each mousemove event that the browser fires. This expected, as Angular performs a change detection cycle after each event that somebody listens to, as the event handler could have changed the data, so this is the expected behavior.

To clarify:

• This is not an infinite loop, but it’s just the problem that the mousemove event is fired pretty often by the browser
• This is not a template parse error, but a runtime error whenever Angular tries to evaluate foo.bar in the template, as foo is undefined.
• In prod mode (i.e. after calling enableProdMode()), Angular never disabled change detection on error, even in 2.x, …

Having said all of this, you can restore the old behavior of disabling the change detection of components with errors by adding the following code to your ErrorHandler:

...
handleError( error: any ) {
  let debugCtx = error['ngDebugContext'];
  let changeDetectorRef = debugCtx && debugCtx.injector.get(ChangeDetectorRef);
  if (changeDetectorRef) changeDetectorRef.detach();

Some notes about this workaround:

• it only works in dev mode, but not in prod mode (i.e. after calling enableProdMode()).
• Having this kind of logic can be very surprising to users as suddenly a chunk of the application stops working (see the issues you linked above).

To summarize:

• This is works as expected
• There is a workaround to restore the special behavior that we had for debug mode

@mhevery or @chuckjaz I’ve updating the issue title in the hope that someone…anyone…might look at this and respond in some way.

I’m having the same issue. Another thing I’ve noticed is that if the errors happens after the application has started and not in the first route (for example, the landing page loads ok, but the user clicks a button that uses routerLink to another page), the infine loop do not occur, but the ErrorHandler is called twice, the second time with the message: 'Uncaught (in promise): ’ + the original stacktrace.

If I use version 4.1.0 all is good.