Effect stops working after an exception occurs

[tlaak]

I'm submitting a...

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

What is the current behavior?

An effect stops working when an exception occurs, i.e. the effect only runs once and further dispatched actions don't trigger the effect anymore.

Example: User loads a page and a GET_STUFF action is dispatched, but the login has expired causing a 401 response from the API endpoint. An error observable is returned from the effect and user is redirected to the login page. User logs in and navigates back to the previous page, GET_STUFF is dispatched again, but this time the effect doesn't run and no API call is made.

Recovering from this needs a full page reload.

Expected behavior:

The effect should always run when a matching action is dispatched.

Minimal reproduction of the problem with instructions:

Here's the code that I have in my effect (imports excluded).

@Injectable()
export class StuffEffects {
  constructor(
    private actions$: Actions,
    private dataService: DataService
  ) {}

  private getStuff(): Observable<Action> {
    return this.dataService.getStuff().pipe(
      map((data: Stuff[]) => {
        return { type: GET_STUFF_SUCCESS, payload: data };
      }),
      catchError(error =>
        ErrorObservable.create({
          type: GET_STUFF_FAILED,
          payload: error,
        })
      )
    );
  }

  @Effect()
  getStuff$: Observable<Action> = this.actions$
    .ofType(GET_STUFF)
    .pipe(
      switchMap(() => this.getStuff())
    );
}

Code for the dataService.getStuff is simply returning the response from the request:

  getStuff(): Observable<any> {
    return this.http.get('//server.example.org/apiEndpoint');
  }

I have an interceptor adding authorisation headers in the call.

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

Node 8.9.0, npm 5.5.1, ngrx 4.1.1, rxjs 5.5.5, Angular 5.1

Other information:

There are similar issues in the version 2 of the library, e.g. https://github.com/ngrx/effects/issues/89 and https://github.com/ngrx/effects/issues/155

[brandonroberts]

This is not a bug, but how RxJS works. You catch the error in the inner observable and return a new non-error observable to keep the outer stream from "dying".

[tlaak]

Confirmed: Returning of({ ... }) from the catchError block works so TIL.

[bbaia]

See also https://github.com/ngrx/effects/issues/158

[Brandinga]

Thank you guys! I also struggled with this.

[brandonroberts]

I'll add a section on error handling to the docs since this is a reoccurring thing that people trip up on

[alex-okrushko]

During my presentation I specifically highlight this in the Gotchas section. Yeah, it has nothing to do with @Effects but instead this is how Observables work.

[akrueger]

Does this mean it's not possible to use Angular's global error handling with Effects?

I have submitted a question on SO regarding this.

I currently make use of it to throw errors and route them to the correct places:

@Injectable()
export class GlobalErrorHandler implements ErrorHandler {
  private messagesService: MessagesService
  private router: Router

  constructor(
    private injector: Injector, // DI workaround (https://stackoverflow.com/a/41585902)
    private errorLoggerService: ErrorLoggerService
  ) {
    // DI workaround (https://stackoverflow.com/a/41585902)
    setTimeout(() => (this.messagesService = injector.get(MessagesService)))
    setTimeout(() => (this.router = injector.get(Router)))
  }

  handleError(error) {
    if (error instanceof HttpErrorResponse) {
      this.handleServerError(error)
    } else if (error instanceof ClientError) {
      this.handleClientError(error)
    } else {
      this.handleUnexpectedError(error)
    }
  }

[imohiosen]

Hi, I believe i am catching the error properly, but after the first error the effect stops working.

@Effect()
    startAd: Observable<Action> = this.actions$
        .ofType(AdvertActions.START_AD)
        .pipe(
            tap(action => console.log(action)),
            map(action => action.payload),
            switchMap(campaign => this.advertService.startAd(campaign.name)),
            map(nameOfUploadedFile => new AdvertActions.StartAdDone(nameOfUploadedFile)),
            catchError(err => of(new AdvertActions.StartAdFailed(err))),
        );

Any idea what else could cause this?

[brandonroberts]

Hey @imohiosen. You should be handling errors inside the switchMap. That way the stream continues after an error occurs.

@Effect()
    startAd: Observable<Action> = this.actions$
        .ofType(AdvertActions.START_AD)
        .pipe(
            tap(action => console.log(action)),
            map(action => action.payload),
            switchMap(campaign => 
              this.advertService.startAd(campaign.name).pipe(
                map(nameOfUploadedFile => new AdvertActions.StartAdDone(nameOfUploadedFile)),
                catchError(err => of(new AdvertActions.StartAdFailed(err))
             )
          )
        );

[imohiosen]

Thanks @brandonroberts. That fixed it.

[alex-okrushko]

Off-topic @imohiosen Also consider using concatMap instead of switchMap as a first choice. https://blog.angularindepth.com/switchmap-bugs-b6de69155524

[metasong]

@imohiosen and @brandonroberts here is another solution to do error handling at the end ot the pipe operators (stream global Effect error handler 👍 ) : instead of write:

    catchError(() => { return of(obj); })

we should write:

    catchError(err,caught)=>{ 
     // do something here; i.e. send some err message to store with store.dispatch(...)
      return caught; //return the caught observable to retry.
     } 

@imohiosen 's example could be written as:

    @Effect()
    startAd: Observable<Action> = this.actions$
        .ofType(AdvertActions.START_AD)
        .pipe(
            tap(action => console.log(action)),
            map(action => action.payload),
            switchMap(campaign => this.advertService.startAd(campaign.name)),
            map(nameOfUploadedFile => new AdvertActions.StartAdDone(nameOfUploadedFile)),
            catchError((err,caught) => {
               this.store.dispatch(new AdvertActions.StartAdFailed(err)); 
               return caught;
            })
        );

Explanation:

CatchError operator allows to handle error, but doesn't change nature of observable - error is terminal condition to given observable so emission will stop. CatchError allows to emit desires value when it occurs instead of raising observer's error callback.

In here, catch is handled in main stream, so error raised in inner observable (in switchmap) bubbles up to main stream, substituted into handled value, then completes (stops). the catchError operator also provide a second parameter to retry the source observable, above is this solution used as a global stream error handler. You could also handle error in inner observable like 'switchmap(x => x.catch())' (solution of @brandonroberts )then error raised in innerobervable will not bubble up, so won't stop main observable.

Conlution:

• you should always add a catchError operator as a global Effect error handler to prevent the Effect stop!

[ghost]

https://medium.com/city-pantry/handling-errors-in-ngrx-effects-a95d918490d9

[ghaithAqidi]

@brandonroberts you saved my ass thanks!

[akmjenkins]

@metasong

Yours is the solution I was looking for. Continually having to go inside each switchMap to catch errors brings back the often despised callback hell that people have spent significant effort to avoid. Not sure why people have deemed it acceptable when it comes to rxjs (or is it ngrx? I'm new to observables in general, so please take it easy on me!).

Anyway, thanks for your solution! It is rather annoying that we can't just return actions from the final catchError, but I understand that's not the current API.

[dcanning]

Is there a reason or preference for using @metasong vs @brandonroberts solution to catching errors? These both work and appear equally valid, but I'm not intimately familiar with inner workings, so should we be using one over the other?

[alex-okrushko]

@brandonroberts solution is preferred (handle before flattening). If error stays unhandled then a variation of @metasong solution would be automatically applied (https://ngrx.io/guide/effects/lifecycle#resubscribe-on-error)

[dcanning]

@alex-okrushko are those versions tied to @angular/core version, or can ngrx 8 be used with angular 6?

[alex-okrushko]

@dcanning My understanding is that NgRx 8 would have to be used with angular 8+.

[redwolfgang24]

@brandonroberts it really works, thank you soo much.

[ekhmoi]

@metasong thank you, it works. But how do you test these effects? As it doesn't return new AdvertActions.StartAdFailed(err) anymore and dispatches it to store What is the recommended approach? To spy on store.dispatch? I don't like this to be honest. Any tips?

[ekhmoi]

@brandonroberts regarding catching errors inside switchMap, how do you get around having multiple switchMaps which might possibly throw an error?

What I want to do is:

  createUser$ = createEffect(() =>
    this.actions$.pipe(
      ofType(UsersFeature.actions.createUser),
      switchMap(({ item })=> this.createAPIUser(item).pipe(
        catchError(error => of(UsersFeature.actions.createUserFailure({ error }))),
        map(apiUser => ({ apiUser, item })),
      )),
      switchMap(({ apiUser, item }) =>
        this.createFirebaseDocument(UsersFeature.collectionName, item.id, { ...item, cicp_id: apiUser.cicp_id }).pipe(
          map(firebaseDoc => UsersFeature.actions.createUserSuccess({ item: { ...firebaseDoc, password: apiUser.password } })),
          catchError(error => of(UsersFeature.actions.createUserFailure({ error })))
        )
      )
    )
  );

And it doesn't work. Throwing below error

error TS2339: Property 'cicp_id' does not exist on type '({ error: any; } & TypedAction<string>) | Partial<IUser>'.
      Property 'cicp_id' does not exist on type '{ error: any; } & TypedAction<string>'.
    src/app/users/store/users.effects.ts(45,113): error TS2339: Property 'password' does not exist on type '({ error: any; } & TypedAction<string>) | Partial<IUser>'.
      Property 'password' does not exist on type '{ error: any; } & TypedAction<string>'.

[alex-okrushko]

@ekhmoi You'll nest it within the outer switchMap.

 createUser$ = createEffect(() =>
    this.actions$.pipe(
      ofType(UsersFeature.actions.createUser),
      switchMap(({ item })=> this.createAPIUser(item).pipe(
        switchMap((apiUser) =>
        this.createFirebaseDocument(UsersFeature.collectionName, item.id, { ...item, cicp_id: apiUser.cicp_id }).pipe(
          map(firebaseDoc => UsersFeature.actions.createUserSuccess({ item: { ...firebaseDoc, password: apiUser.password } })),
          catchError(error => of(UsersFeature.actions.createUserFailure({ error })))
        )
        catchError(error => of(UsersFeature.actions.createUserFailure({ error }))),
      )),      
      )
    )
  );

[ekhmoi]

@alex-okrushko, thank you for the response. I ended up using mergeMap with one outter catchError. Seems working so far.

However I still don't like this nested hell just to catch error. Wish there was a better way

[alex-okrushko]

Even in the example that I gave you you can drop the inner catchError - depends if you want to differentiate how your app handles it or not. If you drop the inner catchError then you don't need to nest.

[yuliankarapetkov]

When chaining multiple requests, I cannot catch an error occurring in the first request (mock1).

See here: https://stackoverflow.com/questions/59578148/how-to-catch-ngrx-effects-errors-properly

getMockDataEffect$ = createEffect(
    () => this.actions$.pipe(
      ofType(ApiGetMockData),
      concatMap(() => {
        return this.mockApi.mock1().pipe(
          concatMap(() => {
            return this.mockApi.mock2();
          }),
          concatMap(() => {
            return this.mockApi.mock3();
          }),
          map(res => ApiSuccess({ data: res })),
          catchError(error => of(ApiError({ error }))),
        )
      }
      )
    )
  )

[alex-okrushko]

@yuliankarapetkov You got the correct response at stackoverlow (that's why it's a better avenue for questions like this). https://stackblitz.com/edit/ngrx-effects-example-jtvmjn?embed=1&file=src/app/services/mockapi.service.ts

The issue is indeed in the service method as it throws error when it's called, and not in the Observable that is returned.

[Pingear]

Please help) can you provide an example of how to catch exceptions with ngrh / DATA. In the dock - https://github.com/johnpapa/angular-ngrx-data/blob/master/docs/entity-actions.md#action-error said - The primary use case for error is to catch reducer exceptions. Ngrx stops subscribing to reducers if one of them throws an exception. Catching reducer exceptions allows the application to continue operating. I cant get where ngrx / DATA has this reducer to catch reducer exceptions.

[willyou]

I prefer to handle promises in this way. Check the code

Although it's not very pretty for some developers, however, it's easier to understand and avoid tricky CatchError. But if it compares to this.store.dispatch, I'm okay with the promise style.

[crisz]

@metasong that's a good solution, but we don't need to inject the store. We can go from this:

   catchError((err,caught) => {
     this.store.dispatch(new AdvertActions.StartAdFailed(err)); 
     return caught;
  })

To a more concise solution, using the merge operator:

  catchError((err,caught) => {
    return merge(caught, of(new AdvertActions.StartAdFailed(err));
   })

Dropping the injector dependency is important because in this way we can now extract the operator into a higher order function:

export function dispatchOnError<T extends string, E = any>(callback: (param: E) => TypedAction<T>) {
  return <U>(source$: Observable<U>) =>
    source$.pipe(
      catchError((error: E, caught$) => {
        return merge(caught$, of(callback(error)));
      })
    );
}

And this can now be used as a straight replacement to catchError:

    this.actions$.pipe(
      ofType(getTodos),
      switchMap(({ queryParams }) => this.todoService.getTodos(queryParams)),
      map(todos => {
        return getTodosSuccess({ response: todos });
      }),
      dispatchOnError(error => getTodosFailure({ error }))
    )

This code is tested and working. I came up with this solution because the aforementioned antipattern with catchError was used everywhere around the application I was working, the observables kept closing and since it is a large codebase it was hard to refactor everything using the mergeMap solution. The fact that in 2024 there are still a lot of questions about this indicates that it's not much intuitive, and maybe updating the docs wasn't enough. Can we think about introducing a similar operator in the ngrx/effects package?