Saga not re-firing after error

[MJLHThomassen-Eurocom]

Hey there, thanks for the lib! I have a small issue though. Don't know if its related to this repo or to redux-saga itself though.

I have the following setup:

const getDeviceControllerDevicessAsync = bindAsyncAction(devicesPageActions.getDeviceControllerDevices)(
    function* (id: string): SagaIterator
    {
        const deviceControllerDevices = yield call(getDeviceControllerDevices, id);
        return deviceControllerDevices;
    }
);

function* selectDeviceControllerAsync(action: Action<string>): SagaIterator
{
    yield call(getDeviceControllerDevicessAsync, action.payload);
}

function* selectDeviceControllerSagaAsync(): SagaIterator
{
    yield takeEvery(devicesPageActions.selectDeviceController.type, selectDeviceControllerAsync);
}

export function* devicePageSaga(): SagaIterator
{
    yield all([
        call(selectDeviceControllerSagaAsync)
    ]);
}

Whenever i fire a selectDeviceController action, the selectDeviceControllerAsync saga gets called, which starts the async actionCreator.async() created getDeviceControllerDevices async process.

All is well, untill the getDeviceControllerDevices throws an error. I see the getDeviceControllerDevices.failed state being fired. However, when i then fire another selectDeviceController action, nothing happens. The debugger also doesn't break anymore in the selectDeviceControllerAsync method.

To fix this, i had to do the following:

function* selectDeviceControllerAsync(action: Action<string>): SagaIterator
{
    try
    {
        yield call(getDeviceControllerDevicessAsync, action.payload);
    }
    catch (e)
    {
        // Failure already handled in devicesPageActions.getDeviceControllerDevices.failed
    }
}

Which is a bit weird in my opinion since the error is/should be already handeled by the getDeviceControllerDevices.failed path of the saga.

Is this intended behavior? How would i deal with somethign like this? I don't like having an empty catch clause there...

[aikoven]

Well, the idea was that bindAsyncAction decorator doesn't change the behavior of a wrapped function. Resulting generator returns the same value as the wrapped function and throws the same exception.

I see several options:

1. Custom effect. Something like safeCall instead of original call, that swallows exceptions (it should probably swallow the returned value as well — silentCall?).

function silentCall(fn, ...args) {
  return call(function*() {
    try {
      yield call(fn, ...args);
    } catch (e) {
      // ignore
    }
  });
}

You can use it to run the bound generator without worrying about exceptions. However, it doesn't seem too convenient with takeEvery/takeLatest, where you would have to introduce yet another generator:

yield takeEvery(someAction, function* () {
  yield silentCall(myBoundGenerator);
});

2. Decorator. A higher-order generator that swallows exceptions (and return value):

function silent(fn) {
  return function*(...args) {
    try {
      yield call(fn, ...args);
    } catch (e) {
      // ignore
    }
  }
}

Then wrap the bound generator with silent:

const myBoundGenerator = bindAsyncAction(...)(function*() {...});
const mySilentBoundGenerator = silent(myBoundGenerator);

// later...
yield takeEvery(someAction, mySilentBoundGenerator);

3. Add an option to bindAsyncAction:

   
   const myBoundGenerator = bindAsyncAction(myAsyncActions, {silent: true})(
   function*() {...}
   );

// later... yield takeEvery(someAction, mySilentBoundGenerator);

I think I'm ok with adding `options` argument, along with https://github.com/aikoven/typescript-fsa-redux-saga/pull/4.

What do you think?

[MJLHThomassen-Eurocom]

I like aproaches 2 and 3 both tbh.

I think having something like this inside the lib would be great, either via the silent() decorator or the option.

I dove into the code a bit and noticed that indeed you are re-throwing the caught exeption that fires the failed action purposefully and now that ive worked with Redux-saga a bit more i do see the advantage of doing this.

I've toyed around with it a bit, and doing option 3 is probably the least amount of work since option 2 requires alot of typings (depending on how many arguments the async action requires) to get typesafe. That being said, option 2 would have my preference if it would mean i could directly put the output of silent() into the takeEvery(). Currently I still require the "helper" generator with the "emtpy" catch clause in order for me to even use the output of bindAsyncAction() in a takeEvery. So if its possible to get it to work that way I would vote for option 2.

[aikoven]

If you need to use the output of a bound generator, i.e. its return value, you have to wrap its call in try/catch to handle a possible error. Otherwise, how would you know there's even an output?

If you don't like try/catch, you may consider this:

interface Success<T> {
  succeeded: true;
  value: T;
}
interface Failure {
  succeeded: false;
  error: any;
}
type Result<T> = Success<T> | Failure;

function safeCall(fn, ...args) {
  return call(function* () {
    try {
      const value = yield call(fn, ...args);
      return {succeeded: true, value};
    } catch (error) {
      return {succeeded: false, error};
    }
  });
}

// later in takeEvery
const result: Result<SomeType> = yield safeCall(...);
if (result.succeeded) {
  // do something with result.value
}

[MJLHThomassen-Eurocom]

I think ive just been using the library a "wrong" way. I've looked into it and now i have some more complex sagas which do benefit from the try/catch thing so this isn't as big an issue as I thought it was before.

[MJLHThomassen-Eurocom]

I've solved this by using yield spawn() instead of yield call().

spawn() starts its own seperate saga that does not propagate errors. Since in the case i mentioned above i can still handle errors (if need be) on the firing of the "failed" action, this suffices.