Store: .disptach Observable does not propagte error

[juliusstoerrle]

When an async action calls a service which fails, the error is not propagated to a subscriber of the .dispatch() method, instead the next() handler is called.

Versions

* ngxs: ^3.0.1-dev.master-46a4cc5
* @angular/core: 6.0.0

Repro steps

• create a async action which calls an API Service
• let the service return an error (for example "401 Unauthorized")
• send the action and subscribe to the return value of the dispatch() method
• the observable will run the next handler of the subscriber

I created a sample repo:
https://github.com/juliusstoerrle/repro-ngxs
https://stackblitz.com/github/juliusstoerrle/repro-ngxs?file=src%2Fapp%2Fapp.state.ts

• open stackblitz /run ng serve
• press the button to dispatch the action
• open your devtools and look at the console.
• you should see "Completed Action Succesfully"

Observed behavior

Instead of the error handler the next handler of the subscriber is executed

Desired behavior

I expected that the error handler of the subscriber would be called

[mkashifwk]

@juliusstoerrle i have same issue, basically I want to reuse
AddTodoAction of TodoStatein FormState , so in FormStatei have SubmitAction , i just simply want to reuse and dispatch AddTodoAction, so i have to pipe it as well for both "tap and catchError" operators, it will look like this.

class TodoState{

    @Action(AddTodoAction)
      add(){
              return todoService.postTodo().pipe(tap => set state)
       }

}

class TodoFormState{

    @Action(SubmitFormAction)
      submit(ctx){
              return  ctx.dispatch(new AddTodoAction())  // reuseable AddTodoAction
                         .pipe(tap=>setformstate, catchError=> seterrors )
       }

}

[juliusstoerrle]

Could this be related to this catchError()? https://github.com/ngxs/store/blob/bb9eb5a7271ee255424457cac446755759780be6/packages/store/src/store.ts#L35-L40

So that's related to this feature: https://ngxs.gitbook.io/ngxs/advanced/error-handling
So I think there might be different use cases here with different needs. Some want to handle errors centralized and some would like to handle errors in the dispatching components, am I right?

[mkashifwk]

@juliusstoerrle I think https://ngxs.gitbook.io/ngxs/advanced/error-handling is for general purpose error handling, I need it for AsycnFormValidation, like when a field is not valid, form validation error will come from backend, then will show up in form

[juliusstoerrle]

@mkashifwk yes, I do understand what you wan't to achieve and I think I covered your use case in my last comment.

On a side note I think for UX you should use Async Validators. What you are trying to achieve in your particular case does seem a bit sketchy to me. If you would like more discussion on that, we should open a thread in Slack.

[mkashifwk]

@juliusstoerrle I Async Validator, yes I have read that, I would love to start thread on that, I am on slack, if you are free we can discuss it now, and thanks for help :)

[markwhitfeld]

I updated the version of ngxs to the dev version after the PR was merged and the issue seems to be fixed:
https://stackblitz.com/edit/github-qfjnen?file=src%2Fapp%2Fapp.component.ts

The fix will be included in the next published release.

[juliusstoerrle]

@markwhitfeld Thanks a lot for your quick fix!!! I pulled in your fix into my project and its working great!

[leksinteractive]

Hi @markwhitfeld ,

I came across this error recently and managed to fix it by updating to the latest version of ngxs/store (3.1.4). Thanks!

However, something I noticed while testing this with a global error handler is that it seems the global error handler is invoked twice when it first catches an error that's thrown by the action. Any subsequent errors seem to trigger it only once though. This is the behaviour if I re-throw the error in the global error handler like so:

public handleError(error: any): void {
      console.log('GlobalErrorHandler', error);
      throw error;
}

To make things more interesting, if I don't re-throw the error, the handleError method gets invoked twice on every error (regardless of whether it's the first one or subsequent ones). Any suggestions for getting around this? Also, as in my app I do some redirection in the global error handler eg. for 401 errors, so I wouldn't want to re-throw the error in that case.

Here's a stackblitz: https://github-qfjnen-drrq77.stackblitz.io

Cheers,
Aleks

[amcdnl]

I wonder if the shareReplay did this? @deebloo

[markwhitfeld]

@leksinteractive Could you log this as a new issue? It is related because it is to do with error handling, but it is actually a different bug.

@amcdnl I think that it could be to do with shareReplay (which I think has since also been added to the inner dispatch, within the compose). In PR #376 I opted for a subscribe over a catchError pipe because shareReplay only stores the last replay result of the value in the stream and not an error. Using a shareReplay over a catchError may result in the handler being executed twice. cc @deebloo

[leksinteractive]

Thanks @markwhitfeld and @amcdnl . Created a new issue: #463