new behavior for global ErrorHandler and uncaught errors from promises

[captaincaius]

🐞 bug report

Affected Package

Well I thought this was zone.js wrapping the errors, but I rolled zone back only zone and it didn't change.

Is this a regression?

Yes, the previous version in which this bug was not present was 5.x

Description

Previously, unhandled errors from promises were handed to a custom error handler unchanged. But now, it seems the error is wrapped and you need to unwrap it to get the real error that's sitting on the "rejection" property of the error.

The docs make no mention of this here https://angular.io/api/core/ErrorHandler and neither does any googling surprisingly.

I noticed this only after upgrading from angular 5 to angular 7 and one of my e2e tests that relied on my general error handler started failing.

The only workaround I found was to have something like this at the top of the error handler's handleError() method like this:

    while(error.rejection) {
      error = error.rejection;
    }

but...

1. I'm quite sure there's a better way - now I have to type error as any
2. I'm not sure if there are any other situations that will require unwrapping (e.g. from observables?).

Either way, if this was some kind of intentional feature, it's definitely a gotcha that I think should be mentioned (with the RIGHT way to handle it) in the doc link I pasted.

🔬 Minimal Reproduction

1. follow the docs and implement a global error handler and try to handle unhandled requests uniformly - something like this:

@Injectable()
export class SpiffyErrorHandlerService implements ErrorHandler {
  constructor(private injector: Injector, private snackBar: MatSnackBar) {}
  public handleError(error: Error) {
    if (error instanceof HttpErrorResponse) {
      if (error.status === 401) {
        this.snackBar.open(
          'Oops.  It looks like you tried to access a logged-in resource while logged out.  Please log in again.',
          '',
          { duration: 3000 }
        );
    }
  }
}

2. inject HttpClient

3. await this.http.get('/some-404-returning-url').toPromise();

🔥 Exception or Error

EXPECTED: error handler works, gets the HttpErrorResponse, and shows the snackbar
ACTUAL: the HttpErrorResponse is on error.rejection instead of right on error.

🌍 Your Environment

Angular Version:

Angular CLI: 7.1.2
Node: 10.14.2
OS: linux x64
Angular: 7.1.3
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router, service-worker

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.10.7
@angular-devkit/build-angular     0.10.7
@angular-devkit/build-optimizer   0.10.7
@angular-devkit/build-webpack     0.10.7
@angular-devkit/core              7.0.7
@angular-devkit/schematics        7.1.2
@angular/cdk                      7.1.1
@angular/cli                      7.1.2
@angular/flex-layout              7.0.0-beta.19
@angular/material                 7.1.1
@ngtools/webpack                  7.0.7
@schematics/angular               7.1.2
@schematics/update                0.11.2
rxjs                              6.3.3
typescript                        3.1.6
webpack                           4.19.1
    

Anything else relevant?

[mhagnumdw]

Same behavior (problem?) here, but using Resolve.

If inside Resolve I make the call

this.http.get<Holiday>(`/some-404-returning-url/${id}`).subscribe();

the HttpErrorResponse in ErrorHandler is wrapped in error.rejection.

Versions:

Angular CLI: 6.2.3
Node: 8.11.1
Angular: 6.1.8

[benlesh]

It appears to be an issue with the type of error coming out of Zone.js changing... Here's a small stackblitz repro

NOTE that it does this with any rejected promise.

[JiaLiPassion]

Currently, in zone.js, unhandled promise reject will be wrapped as new Error with a rejection property which equals to the original error, I think we may need to handle this case in zone.js or angular, I will create a PR later to let zone.js return original error after zone.js is moved to angualr repo.

[JiaLiPassion]

This one has been reverted because some test failed, will try in the next version

[didii]

This issue is still present in Angular 8 as far as I can see. We're currently on version 8.2.14. Is there a fix ready, was this issue forgotten or will it not be fixed by design?
In any case, what would definitely help if there was a type specifying the known properties (i.e. rejection, promise, zone, etc) so we'd at least know when the API changes. Is there any available?

[simeyla]

@didii just do this and then your handler is future proof

    handleError(wrapperError: any) 
    {
            const err = wrapperError.rejection ? wrapperError.rejection : wrapperError;

            // now handle 'err'

[AndresSp]

The same problem here (Angular 8.2.14)

Maybe this could help someone:
I moved my error handler to an HTTP-interceptor, so I can handle the HttpErrorResponses without that wrapper

 export class ServerErrorInterceptor implements HttpInterceptor {
    constructor(private _errorRedirectedService: ErrorRedirectedService) {}

    public intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>>{

      return next.handle(request).pipe(
        catchError((error: HttpErrorResponse) => {
          //HttpErrorResponse
          throw error //goes to the Global-Error-Handler
      }))
   }
 }

[captaincaius]

Seeing as how this is still open, would the angular team welcome a little note in the docs simply explaining it?

[JiaLiPassion]

the previous PR was reverted, I will create a new PR to fix this issue.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}