store: 🐞[BUG]: ErrorHandler triggered twice on one action

I’m submitting a…


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

Current behavior

I have action that retrives data from backend and throws error. I run action like this:

this.obiektyActions.change(data)
          .subscribe(() => {
             this.changed.emit(id);
        });

Action itself is something like this:

return this.apiService.change(action.data)
      .pipe(
        catchError((err) => {
// ...
          return throwError(err);
        }),
        tap(resp => {
          // ...
          this.patchCurrentObiekt(ctx, action.obiektId, val);
        })
      );

I have my custom ErrorHandler and it is invoked twice. As I read it’s because observable has two or more subscribers: https://stackoverflow.com/questions/47898101/why-my-global-error-handler-get-invoked-twice-in-my-angular-app

I suppose You are subscribing to action, so I get two subscriptions (and ErrorHandler is triggered twice).

If I change action execution to:

this.obiektyActions.change(data)
          .pipe(tap(() => {
             this.changed.emit(id);
        }));

Action is still triggered (so You have to subscribe) but on success this.changed.emit(id); is never triggered.

Edit: this.obiektyActions.change(data) = return this.store.dispatch(new ChangeDataAction(data));

Expected behavior

ErrorHandler should recive error info only once.

Minimal reproduction of the problem with instructions

https://stackblitz.com/edit/ngxs-error-throw?file=src/app/app.component.ts

What is the motivation / use case for changing the behavior?

Environment


Libs:
- @angular/core version: 7.2.2
- @ngxs/store version: 3.3.4


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 9
Comments: 20 (5 by maintainers)

Most upvoted comments

@MatissJanis y, you’re right, that could be a plan for v4.

arturovt on Dec 12, 2019

Having problems with this as well.

I’d like to have the global error handler as a fallback for when the error is not dealt with in the component.

But the error is triggered twice, once on the subscribe of the dispatch and also on the global error handler. Imo that is actually wrong behavior. The global error handler should only be called for unhandled errors.

I seriously ask myself how others are dealing with error handling + ngxs

cmaart on Jul 3, 2019

@MatissJanis

Trust me, I really understand your case and what you’d want to achieve. This logic (related to the ErrorHandler) has been the cornerstone from the start, so removing it would cause breaking changes and probably further defects or issues.

Since we are convinced that we cannot remove this logic then we could try to find some trade-offs. I remember that my solution was just to not pay attention to those errors from the ErrorHandler 🤣 (since original Angular’s ErrorHandler just invokes console.error and nothing more)

arturovt on Dec 12, 2019

Fair enough @arturovt

I saw another comment with identical snippet in this thread and assumed this issue would cover it as well (https://github.com/ngxs/store/issues/803#issuecomment-460255770).

Any suggestions how to move error handling into components? Since is many cases it doesn’t make sense to globally handle errors in actions, but it makes sense to handle them on a case-by-case basis in the consumer component.

MatissJanis on Dec 12, 2019

Just tested if this issue is fixed with the latest version: it is not.

Even though in my component I’ve handled the exception, it still reaches global error handler.

this.store.dispatch(new Login()).subscribe(
  () => {
    console.log('Success');
  },
  () => {
    console.log('Error is handled here');
  },
);

MatissJanis on Dec 12, 2019

I’ve also had a damn headache regarding this issue as my global error handler was printing in console 2 times the same error. First we’ve called data service directly and saw that actually it was NGXS who was throwing the error the second time. Now we were sure about the potential source of problem. But then we’ve tried some other places in our code that also were dispatching actions, and it entered our error handler only one time. So something was not right. The difference between first action and second one was that the first one has been subscribed on, and did some post action: this.comService.dispatchAction(new PatchRules(requestModel)) .subscribe(() => { this.comService.getActiveDialog(AddRuleModalComponent).close(this.data); }); The fix of the problem was: this.comService.dispatchAction(new PatchRules(requestModel)) .subscribe(() => { this.comService.getActiveDialog(AddRuleModalComponent).close(this.data); }, () => {} ); Providing the error case of the observable (even an empty one). After we found the fix we tried to understand the root cause of this behavior, and in NGXS documentation https://www.ngxs.io/advanced/errors we found NGXS uses Angular's default ErrorHandler class so if an action throws an error, Angular's ErrorHandler is called. So as the result NGXS calls Angular’s ErrorHandler, and we in our code also call our ErrorHandler. Thus it enters the error handler 2 times.

vmuresanu on Sep 13, 2019