Effect error logging should be customizable or use ErrorHandler

[jbedard]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[X] Feature request
[ ] Documentation issue or request

What is the current behavior?

Errors are sent to the internal ErrorReporter which logs to the console

Expected behavior:

Errors should be logged to ErrorHandler (from @angular/core) or ErrorReporter should be public and customizable/overridable to allow doing so (or both).

Version of affected browser(s),operating system(s), npm, node and ngrx:

4+ ?

Other information:

Personally I'd think just sending all errors to ErrorHandler would be best and the ErrorReporter can be removed, but maybe others depend on the console logging from ErrorReporter? Maybe just making ErrorReporter public/overridable is a good start without any breaking changes?

[jtomaszewski]

Related question: Is it possible to override ErrorReporter injected by ngrx effects? Like, can I do sth like providers: [{ provide: ErrorReporter, useClass: MyOwnErrorReporter }] in my ngmodule?

[jbedard]

Not while ErrorReporter is private, unless you really want to import and override ɵh.

[brandonroberts]

Exposing the ErrorReporter so you can override it seems reasonable to me. Any takers on a PR?

[jbedard]

I can do it. However I do question what the point of ErrorReporter is. Can we just delete it and use ErrorHandler instead?

[jbedard]

How should this be exposed?

1. make ErrorReporter public
2. replace ErrorReporter with ErrorHandler from core
3. expos errors some other way such as an Observable on EffectSources, maybe keeping ErrorReporter as a default observer of those errors

I'd would vote for 2 because it's mainly just deleting stuff, and I think that's what 99% of people would want... errors logged to the angular-standard ErrorHandler with no extra effort/hookup/integrating. It can maybe be a custom extension of Error so it can be typed and easily filtered by type?

Note that all of these make the error-information public.

[brandonroberts]

I vote for option 1 as its not a breaking change. If you want to override the ErrorReporter with the ErrorHandler or use some other service to capture errors it will be possible to do that.

[jbedard]

How about also changing ErrorReporter to have the same interface as ErrorHandler then? That way ErrorHandler can be a drop-in replacement without any wrapper classes being required...

[brandonroberts]

Sounds good to me

[amcdnl]

I have implemented a global error handler globally and for whatever reason my ngrx-effects are not picking it up. The error handler works for everything except ngrx effects.

My code looks like this:

@NgModule({
    declarations: [AppComponent],
    imports: [
        RouterModule.forRoot(routes),
        BrowserModule,
        BrowserAnimationsModule,
        StoreModule.forRoot(reducers),
        EffectsModule.forRoot(effects),
        !environment.production ? StoreDevtoolsModule.instrument({ maxAge: 25 }) : []
    ],
    providers: [
        {
            provide: ErrorHandler,
            useClass: GlobalErrorHandler
        }
    ],
    bootstrap: [AppComponent]
})
export class AppModule {
}

then i have:

import { ErrorHandler, Injectable, Injector } from '@angular/core';
import { Router } from '@angular/router';

@Injectable()
export class GlobalErrorHandler implements ErrorHandler {
    constructor(private _injector: Injector) {}

    handleError(error: any) {
        if (error.rejection) {
            if(error.rejection.status === 401 || error.rejection.status === 403) {
            // unachorized, redirect?
            } else if (error.rejection.status === 404) {
                const router = this._injector.get(Router);
                router.navigate(['/404']);
            }
        } else {

            try {
                console.error(`An error occurred`);
            } catch (e) { /* do nothing */ }
        }

        // rethrow error
        throw error;
    }
}

[mohyeid]

@amcdnl I am facing the same problem. Did you figure this out?

[brandonroberts]

@mohyeid the global error handler cannot be used with effects in that way. If your Observable stream errors, the stream will end. This is not an issue but how RxJS works. Correct usage is to handle your errors explicitly in your effect streams and return a new inner Observable.

Example

@Effect() someEffect = this.actions$.pipe(
  ofType(SomeAction)
  mergeMap(() =>
    doSomething().pipe(
      map(() => NewAction),
      catchError(() => EmptyObservable)
    )
  )

[mohyeid]

@brandonroberts Thanks for your response. I am trying to handle general error and actually I just got a working example on stackblitz:
This is working perfect as what I would expect, but not working on my app. On my app the error stops at the Global Error Handler and the action never gets forwarded to the effect to handle my error.
I matched all versions and everything, but still not able to figure out why it stops at the error handler. I will try to dig more into this.

[amcdnl]

@mohyeid - I did a follow up post on this - https://medium.com/@amcdnl/handling-errors-in-ngrx-effects-2a116640d6bb ... might interest you.

[mohyeid]

Thanks all. I found the root of my problem. I am sending the error object as my payload to the effect and the dev tools is trying to strengify the error to send it as a state change using window.postmessage. This will fail with "Converting circular structure to JSON" error. I will create my own error model and copy the items I am interested in to fix this. If I disabled the dev tools, everything will work as expected.