Open brian428 opened 7 years ago
Please provide a plunker so that we can have a reproduction case.
@mhevery: unfortunately, after spending 4 or 5 hours trying to reproduce this in a Plunker, still not able to. For now, I have a hack in place in my error handler to track the last error, and if the last stack matches the current error stack, I grab the ApplicationRef and call tick() on it, which triggers the check in tick() to prevent recursive calls to it. This breaks out of the infinite error loop and at least stops the browser from hanging with full CPU use.
I've used breakpoints in the error handler and tried to go down through the stack trace, but it's very hard to figure out what is going on because everything in the trace is either internal to Angular, SystemJS, or Zone. It also doesn't help that there's a lot going on during the initial load (bootstrapping, routes being resolved, templates being loaded and compiled, etc.).
The related issues all seemed tied to triggering change detection in a custom error handler. So I'm still fairly sure this has to be some subtle bug introduced in #16426, simply because even if I take out my custom ErrorHandler completely, the issue still occurs. And if it is somehow an error in my own code, I think that needs to be addressed in some way too. I can understand that if someone keeps manually triggering change detection in their error handler, there's not much the framework can do about that. But beyond that, it shouldn't be possible for a missing template variable to throw the framework into an infinite error loop.
I'd like to help narrow this down so that I can try again to create a Plunker repro case. Can you (or anyone else) think of any situation I could be causing that would trigger an infinite error loop simply by referencing an invalid property in a component HTML template (other than triggering change detection in the error handler, which can't be the problem in this case)?
I spent another 4 or 5 hours tonight trying to track this down, and have a few updates:
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.
I was wrong, the infinite loop occurs whether Renderer
or Renderer2
is used.
@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'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.
@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.
@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.
Same here using 4.2.3
@mhevery or @chuckjaz I've updating the issue title in the hope that someone...anyone...might look at this and respond in some way.
@brian428 Thanks for providing the plunker! I looked into it, and am seeing the following:
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:
mousemove
event is fired pretty often by the browserfoo.bar
in the template, as foo
is undefined.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:
enableProdMode()
).To summarize:
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.
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.
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!
@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?
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.
I am running into this as well, but only when I've extended the built-in $exceptionHandler with a decorator, and not on all errors.
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
@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!
@swftvsn I always have my browser's DevTools open and when this problem occurs, I just hit "pause" on the debugger and it will stop throwing more exceptions. This way I can at least check the issue and most of the time, I don't need to kill the browser, because I can just close the tab.
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 !
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
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.
I had also meet the same problem I use angular 7 and material ui
I've been running into this a lot as well. Errors get thrown, get caught in an infinite loop. Obviously applications should avoid having any errors, but this does seem like strange behaviour.
@jasonaden I think this issue has been going on for far too long, I've been around since the early ng1 days and this is a really ugly wart on this framework. I'm sick of killing my browser tab because of it locking up, and these workarounds make using an error reporter more complicated than it has to be. Severity should go up as this wastes countless man hours and either clear documentation for the 'correct' handling, or a flag to stop that hanging from happening should be introduced.
Preferably this decade please, literally 2 years of this issue being open?!
Angular team ignores most important things....
@devversion seriously, this issue deserves WAY more attention that it's getting.
same issue .... haiz
We have the same issue. ErrorHandler opens a MatDialog in an NgZone which throws an error, bam infinite loop and browser lock. If we do not run MatDialog in an NgZone then it does not render properly and can not be closed.
I've had to ditch using Rollbar because any user errors cause me to spam the hell out of their API, using up my account limit. @kara can we get some sort of response here?
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.
Tracking rollbar issue here:
@jpike88 In the meantime can you not simply set a isErroring
variable somewhere and not spawn a request to your API / instantiate a dialog / whatever until isErroring
is false
(ie when the user dismisses the error)?
Any update on this thread ? I am also in the same state. Even if i do not have any template errors and if i use the globalErrorhandler + zone as follow
handleError(errorObj) {
const router = this.injector.get<Router>(Router as Type<Router>);
const zone = this.injector.get<NgZone>(NgZone as Type<NgZone>);
this.onUnhandledException.next(errorObj);
errorObj = errorObj.rejection ? errorObj.rejection : errorObj; // .rejection is required in case of promise rejections.
let errorDetails: ErrorInfo;
if (this.isServerError(errorObj)) {
errorDetails = errorObj.error;
} else {
errorDetails = this.getClientError(errorObj);
}
errorDetails.status = errorDetails.status;
// Angular has a bug where it fails to render the component fast enough as it delays the onInit life
cycle hook.
// Also handleError() by design runs outside of zone.
// Hence trigger a change detection inside zone to make everything work fine.
// Ref: https://github.com/angular/angular/issues/18816
// Ref: https://github.com/angular/angular/issues/15946
zone.runGuarded(() => {
setTimeout(() => router.navigate(['/error'], { queryParams: { error: JSON.stringify(errorDetails) } }));
});
}
i see that the applciation enters an infinte loop and freezes the browser tab. Also in my case i do not see this errors in chrome rather only in IE11.
My error trace
It would be great if there are any officially confirmation on the issue and a temporary workaround as it is very annoying blocker issue.
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.
Still no answer on this??
I'm fighting change detection on my ErrorHandler almost daily! If I have any kind of template error, it goes into an infinite console.error loop and I have to force close the browser to exit.
None of the ngZone or changeDetectorRef workarounds are working for me.
I honestly can't see how this isn't priority #1 on the Angular team??? This happens to me on every single project!
Incase if someone is in the same pickle, Using runGuarded() instead of run() solved the issue temporarily for me.
Read about the api here: https://angular.io/api/core/NgZone#runguarded
Well, here's some effort - deleting my, I admit, very sharp, borderline rude and yet valid comment. I take it I was right in my assumption. Sincerely, I hoped I was wrong.
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.
@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 usingRenderer2
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?
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.
Hello, I am finding when there is a connection lost the infinite loop of the task kicks in and floods the console. Is this issue ever going to be addressed?
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.
See that omarmeky? Your remark was adressed, not even 6 months later. The stack overflow causing issue that exists in angular's core module is: an inconvenience no more!
Now it's a low-priority issue that needs to be resolved.
Eventually.
Or maybe it gets autofixed if ignored for long enough.
What I'm very curious to see is, if this is low-priority, what kinda BIG BAD BOI qualifies for critical
I am having a similar infinite loop issue when an error happens and is picked up by my global error handler:
import { ErrorHandler, Injectable, NgZone } from '@angular/core';
import { Subject } from 'rxjs';
import { distinctUntilChanged, switchMap } from 'rxjs/operators';
import { ApiService } from '../services/api/api.service';
import { environment } from '../../../environments/environment';
import { AuthService } from 'src/app/modules/auth/services/auth.service';
import { Router } from '@angular/router';
@Injectable()
export class GlobalErrorHandler implements ErrorHandler {
private errorSubject = new Subject<string>();
constructor(
private zone: NgZone,
private authService: AuthService,
private apiService: ApiService,
private router: Router
) {
this.errorSubject
.pipe(
distinctUntilChanged(),
switchMap(error => this.reportToSlack(error))
).subscribe()
}
handleError(error: any) {
if (!environment.production) {
console.log(error);
}
// at one occasion the stack property was undefined
if (!error.stack) {
error.stack = '';
}
const authState = this.authService.currentAuthState;
const id = authState && authState.id ? authState.id : 'unknown';
const email = authState && authState.email ? authState.email : 'unknown';
const message = `[GLOBAL ERROR HANDLER] \n${error.message ?? ''}\n ${error.stack ?? ''} \n User: ${id}, ${email}`;
this.errorSubject.next(message);
// prevent infinite loop of error occurances
window.setTimeout(() => {
this.zone.run(() => {
this.router.navigate(['error']);
})
}, 0)
}
reportToSlack(message: string) {
console.log('Error reported:', message);
// if (environment.production) {
const statement = `
query reportError($message: String!) {
reportError(message: $message)
}
`;
const useIam = this.authService.currentAuthState && this.authService.currentAuthState.isLoggedIn ? false : true;
return this.apiService.graphql<string>(statement, {
message
}, 'reportError', useIam);
// } else {
// return of(true);
// }
}
}
When it tries to do the router navigate, it would loop and throw thousands of console logs at the top of the handleError function. Only when I do the router Navigate, with the setTimeout, I am able to break the loop and go to an error page. If I comment out the Navigate, I get infinite loop of the same Error being emitted.
This happened to us as well. Unfortunately, it is relatively hard to provide a small, reproducible example. We'll try a workaround.
In our case, we were attempting to send the error details to our logging service through an http call. Could that be the source of the problem?
@miensol yes, this is very likely the source of your infinite loop
I dont know what logging library you are using, but for example ngx-logger has this issue (https://github.com/dbfannin/ngx-logger/issues/324) unless you specify serverCallsOutsideNgZone
in your logging configuration
I've tried that. However, it turns out that my error handler is already called inside runOutsideAngular
by Angular core. Adding another runOutsideAngular
didn't help.
it's 2024 and this is still one of the most problematic issue in Angular, definitely need to be addressed
I'm submitting a ... (check one with "x")
Current behavior Possibly relates to #9531 and #16426. Since the change to continue change detection after an error (4.1.1), I'm getting an infinite loop of errors if there's a problem with template parsing (such as a bound variable doesn't exist). It causes the browser to completely hang, and the browser process must be manually killed.
The console shows the infinite errors like this:![change detection error loop](https://cloud.githubusercontent.com/assets/140338/26435569/2cca83a0-40de-11e7-9c9b-558a927b4fc8.png)
If I add a breakpoint in my error
handleError()
method, I can get a stack trace which may help with diagnosis. Since it is long, I'll attach it rather than embed. angular error loop stacktrace.txtThe issue is not my error handler triggering some sort of change detection, because I can completely remove the custom error handler class and the issue still manifests.
If you look through the stack trace, you can see that everything going on is internal to Angular, SystemJS (maybe it keeps trying to make XHR calls to load files, which triggers change detection?), and/or Zone. I'm not sure yet if it has to do with the fact that this is happening during bootstrap, or possibly something related to Zone.js, or something related to SystemJS.
Expected behavior The browser should not completely hang due to something as basic as a template compilation error or binding error.
Minimal reproduction of the problem with instructions I'm trying to create a Plunkr, but there are a number of pieces to put in place to mimic what's going on, and it is slow going. I'll keep trying.
Please tell us about your environment: Windows 2010 x64
Angular version: Angular 2.1.3, Zone 0.8.10, System 0.19.41.
Browser: all
Language: TypeScript 2.3.2