ReduxAction.reduceWithState not working

[kuhnroyal]

ReduxAction.reduceWithState is not working as the _store instance hasn't been set yet.
It is also no longer mentioned in the docs so I am not sure if this is still intended to work.

I am currently working around the problem with a simple extension:

FutureOr<AppState> reduceWith(Store<AppState> store, AppState state) {
    setStore(store);
    return reduceWithState(state);
}

My main use case for using this is that I have an action Action1 which dispatches other actions in their reducer like this: await dispatchFuture(Action2())
If Action2 throws an exception which is wrapped in a UserException then Action1 does not get canceled.

So instead I use await Action2().reduceWith(store, state) so that it throws but now I have to handle the wrapping of the exception also in Action1.

I am not sure what is the best way to tackle this.

[kuhnroyal]

I was also using a lot of return null; in reducers but due to my reduceWithState usage, I started to also use return state; so that I can reduce multiple actions in a row without dispatching them.

For both sync and async reducers, returning a new state is optional. If you don't plan on changing the state, simply return null. This is the same as returning the state unchanged.

But there is a difference for async reducers, returning null behaves correctly, but returning state introduces the misbehaviors mentioned in

When your reducer returns Future you must make sure you do not return a completed future. In other words, all execution paths of the reducer must pass through at least one await keyword.

[marcglasberg]

I should probably remove reduceWithState because it's very easy to make a mistake when using it (and if I don't remove it, I should fix it as you described it).

Can you create for me a runnable test, with minimum code, that demonstrates the problem you are having? (without using reduceWithState)?

[kuhnroyal]

I'll try tomorrow.

[kuhnroyal]

import 'package:async_redux/async_redux.dart';
import "package:test/test.dart";

class AppState {
  final String name;

  AppState(this.name);

  AppState copyWith(final String name) => AppState(name);
}

class Action1 extends ReduxAction<AppState> {
  @override
  Future<AppState> reduce() async {
    await dispatchFuture(Action2());
    // Action2 wraps the thrown argument error in a UserException thus this reduce never fails.
    // This may be intented but Action1 never knows what happened.
    return AppState('Action1');
  }
}

class Action2 extends ReduxAction<AppState> {
  @override
  Future<AppState> reduce() async {
    await Future.delayed(const Duration(milliseconds: 100), () {
      throw ArgumentError();
    });
    return AppState('Action2');
  }

  @override
  Object wrapError(dynamic error) {
    return const UserException('error');
  }
}

void main() {
  test('Action throws', () async {
    final store = StoreTester(initialState: AppState('FOO'));

    store.dispatchFuture(Action1());

    final info = await store.waitUntilErrorGetLast(error: ArgumentError);

    print(info);
    expect(info.error, isNotNull);
    expect(info.errors, hasLength(1));
    expect(info.dispatchCount, 2);
    expect(info.state.name, 'FOO');
  });
}

The error wrapping my also happen in the global WrapError handler so that Action2 has no influence over wether the exception should be wrapped. Action2 may be an action that is used for its side effects from several other actions and its error should be displayed to the user. However if this action fails, the other (Action1) actions can not proceed.

[marcglasberg]

Yes, UserExceptions being swallowed (not throwing) is per design. I could create a special user exception class that is displayed to the user but not swallowed. This would solve the above problem but would create another: it would throw the error even when Action2 is not used inside of Action1, which I guess is not what you want.

Please get version: [2.12.1] (directly from GitHub), and try this:

var action2 = Action2();
await dispatchFuture(action2);
if (!action2.hasFinished) throw UserExceptionError("Problems here.");

I don't think this is a good solution, but at the moment I can't think of a good one.

What do you think?

[kuhnroyal]

I will try this but I am also not sure that it is a good idea.
The correct way would probably be to detect from the resulting state, if the action was successful, this however is not feasible for me in all cases.

My main problem is the handling in the WrapError object which I use extensively.

I just had the idea to create some custom BaseAction that has a bool isSubAction which indicates if the action was fired from within another action or from the top-level dispatch. I could then check on the action object inside WrapError if the error should be rethrown or handled. This would require the action to be available inside WrapError just like in ErrorObserver.

Maybe this flag could also go into the ReduxAction and the dispatch/dispatchFuture that is available inside an action can set this to true by default. The flag can then be checked against in either ReduxAction.wrapError or in WrapError.
The default behavior stays the same so this is not breaking.

Does this sound sane?

Edit: Maybe the flag should be the other way around, like isRoot/TopLevelAction or something.

[marcglasberg]

I can add the action to the global WrapError, yes, that could be useful for other use cases as well.

However, with the ErrorObserver alone you can already do what you described above. Define your own ErrorObserver that will check if your action is of type MyBaseAction, and if so, check your isSubAction flag. If you decide you want the UserException to throw, simply return true from the ErrorObserver.

[kuhnroyal]

Yes I know about the ErrorObserver and that I can force it to throw, however I need to prevent the exception from the inner action to be wrapped as UserException at all, otherwise I have 2 dialogs.

I think it would help me a lot if the action is available in WrapError.

Something like this should work I guess:

abstract class CustomReduxAction<St> extends ReduxAction<St> {

  bool _isTopLevel = true;

  bool get isTopLevel => _isTopLevel;

  @override
  DispatchFuture<St> get dispatchFuture {
    return (ReduxAction<St> action, {bool notify}) {
      if (action is CustomReduxAction<St>) {
        action._isTopLevel = false;
      }
      return super.dispatchFuture(action, notify: notify);
    };
  }

  @override
  Dispatch<St> get dispatch {
    return (ReduxAction<St> action, {bool notify}) {
      if (action is CustomReduxAction<St>) {
        action._isTopLevel = false;
      }
      return super.dispatch(action, notify: notify);
    };
  }
}

Do you think this is something that makes sense to be added to the library?

[marcglasberg]

No, I don't really think it makes sense to add it to the library. I think that's too confusing to use, and nothing prevents the caller to call store.dispatch directly, therefore bypassing the action's own dispatch method. But I think this may be a good solution for you, since you developed it yourself and it won't be confusing for you.

Please see version [2.12.2] (in GitHub, not pub.dev yet) for WrapError with action, and deprecated reduceWithState.

I'm going to close this issue, but feel free to come back here with other ideas, in case you find them.

[kuhnroyal]

Sounds good, will try from master later.
Don't forget the original problem with reduceWithState :)