Saga misses multiple actions dispatched within the same event queue

[redux-saga]

Saga misses multiple actions dispatched within the same event queue

[yelouafi]

@emirotin

Sorry if I sound irritated here. It wasn't meant to criticize the project by any means. I really value how you listen to your users and look open to adapting the library behavior to real use-cases

No worry :). i'm also thankful to all people who are trying this lib in their apps although it's still on its early stages.

I think the actual problem here is the mental dissonance. Sagas are middleware and other middlewares are guaranteed to be called for every action dispatched. When one reads the saga code it looks like it will be the case as well, with just the code being suspended between the events. So one (me :)) imagines the unhandled actions queued until the saga unshifts them and executes.

Agree 100%. The pull approach suggests that the saga is actually iterating over an event queue. so the user expects to not miss any event.

And I'm conscious that library users shouldn't concern themselves with underlying implementation details (promises, microtasks, ...) in order to use the library, but only with its communicated semantics. Because it'll add another mental overhead while the library has already a steep learning curve (generators, event handlng paradigm shift, processes ...)

When implementing the take effect. There were 2 choices for its semantics

1- Either queue all incoming actions for the Saga. So it wont miss any action; even if it's blocked on an api call when the action arrives

2- discard the incoming action if the Saga isn't waiting for it

The 1st behavior is more close to how CSP channels work. While the 2nd looks more close to the Actor model.

Both approaches has pros and cons. Depending on the use case, the developer will sometimes want the incoming actions to be queued (buffered) while in other use cases it's not necessary to buffer the actions.

From a conceptual view, event buffering is more close to how iterators works: when i'm iterating over some iterable, i don't expect to miss some value. But the iterator can't be shared between multiple consumers, each consumer has to create its own iterator from a given iterable (array, generator, event queue).

So in a order to share the store channel between many Sagas, w'll have to create an event iterator (or event queue/bufffer) for each Saga. However, buffering all actions can lead to more or less serious memory issues: A generic action queue can't determine in advance which action to keep or discard (i.e. will be taken later by the saga); so it'l have to buffer all incoming actions even if those actions will never be taken by a Saga.

So this is how somewhat I ended up with the actual behavior.

I was of course thinking in adding dedicated channel support which could support the iterator semantics (buffering). Could be something like

function* saga() {
  // buffered channel, wont miss any API_CALL action
  const apiChannel = yield channel('API_CALL')

  while(true) {
    const apiAction = yield take(apiChannel)
    ....
  }
}

Or could be something generic created outside of sagas, and called with a simple call. This has the advantage of being usable also with other event sources (Observables, websocket messages, even raw DOM events)

const apiCallChannel= createChannel(fromStoreActions(store, 'API_CALL'))

function* saga() {
 while(true) {
    const apiAction = yield call(apiCallChannel.nextEvent)
    ....
  }
}

So I'd have to comeup with clear semantics and API; (and also find the time :) to do it ).

This will create a normal middleware that picks every CALL_API action and forks the callApi method. Such kind of notation will also make the actual sagas code shorter as I always find myself writing

yeah. that also crossed my mind, but w'd also have to define some way to coordinate those forked daemons. Otherwise, the solution won't have a real advantage above normal callback based solutions (like thunks) when managing complex flow for all forked daemons. We could provide some saga coordinator to the daemon function; But then we shift away from the simple synchronous-like mental model of generators to a more complex async/push/callback model.

[yelouafi]

After examining all issues. It become clear that reliance on promise isn't going to play nice with synchronous actions. Channels may solve issues for saga inter-communications and are certainly useful. but there are other issues which can't be solved like Sagas taking some time to start (cc @tgriesser like use yield [fork(a), fork(b)] works but yield fork(a); fork(b) not).

I think as long as the core relies on promises, there will be always some issue reported kind of this thing started before this thing. This is inavoidable because of the nature of promise, we ont have control of order of things (which sounds a bit ironic for a lib whose purpose is to allow people to manage order of things).

So to tackle that problem I made a more radical choice, rewrite the redux-saga core without relying on promise to drive the generators but only callbacks (this is only an internal impl. detail and doesnt affect the external API). The advantage its that now w've total control of order of things, and effects like fork are truly non blocking, even call effects which return non promise results are non blocking.

It a was bit (well more than a bit) trickier to achieve (cancellation propagation, had to reinvent race/parallel ...) But finally it works, and now all tests cases pass, even with sync actions. And no more requestAnimationFrame are necessary now.

The code is the no-promise branch. I'll merge it with the master after adding some more edge-case tests.

[yelouafi]

I think the problem is inside the fnB. If you put delay between the 3 puts (like yield call(delay, 0)). I think it'd work

The issue come from the fact that Sagas runner uses promise chaining to drive the generator progression, So every effect, even non blocking ones like fork, are processed asynchronously inside promise.then chains.

You can find issue there related issue #34.

I feel a bit uncomfortable with using the store as a communication channel between sagas. If possible, it's always preferable to factor the common code into a callable saga and call it from inside the 2 communicating sagas. It makes control-flow more reliable and easier to understand

[yelouafi]

For the same reason, it doesn't make sense to change the API (which I don't own) to handle these combined requests.

I see. Indeed this is not the right solution

Can you please give me a better idea what are "synchronous" actions and what would be "asynchronous" in that case?

when you're inside a component for example and you do something like

dispatch(action1)
dispatch(action2)

You are dispatching 2 consecutive actions synchronously. And even if they are fired from 2 different component they are still fired synchronously (Just called from different place).

Since redux-saga uses promises chaining, when a saga receives the 1st action; it does something like

resolveTakeEffect(action1)
   .then( goAndFetchNextEffect )

The problem is that the then callback will not execute right away, instead the underlying promise runtime will schedule the goAndFetchNextTakeEffect in a microtak queue. It simply means, the then will wait for the current flow (for exampe the current event handler, or componentDidMount or whatever fired the action) to terminate and then execute goAndFetchNextTakeEffect. Meantime your code continues and executes dispatch(action2). But at this time, the Saga hasn't yet fetched the next effect (e.g. take or fork) so it is not ready yet to take the action.

The problem wont occur when you dispatch actions in reaction to external events (like xhr/event callbacks) because those callbacks wont execute until all promise then callbacks execute first. So even if 2 event come at a very fast rate, JS runtime will still schedule their callbacks after all promise callbacks.

Initially I thought the above issue wont occur because of how event loop works as described above. But the issue was reported many times; so we need a consistent solution to handle this kind of weird side effect

Until then; You can the workaround from

https://github.com/tomkis1/redux-side-effect-example/blob/master/src/components/Application.jsx#L21-L31

ie. Schedulin the 2 dispatch in 2 macrotasks. This way, the 2nd dispatch will always execute after the Saga is ready to take it.

You don't have to use the brute setTimeout. You can use the https://github.com/YuzuJS/setImmediate for a more efficient way

componentDidMount() {
    setImmediate(() =>
      this.props.dispatch(myAction);
  }

For now, it looks like I have to drop sagas here in favor of the custom middleware.

The project is still young and evolving (2 months). So you can't expect it to handle every possible use case yet. It can only evolve and mature by gathering feedback from developers and then handling reported issues through reusable and consistent solutions.

[emirotin]

It seems to be related for my problem though I'm not sure how to fix it.

I have the following XHR API saga: http://pastebin.com/rpkiZzXv
It receives an XHR request config and issue the request, communicating the result to the reducer by dispatching different actions.

Now I have an app with 2 pages (index and content). Both of them have the same sidebar, and the second page (content) also has a component in the page body. Both the sidebar and the component dispatch the API call actions.

If I first go to index I see the sidebar API request dispatched, taken by the saga and executed. Then if I navigate to the content page I see the 2nd request properly dispatched and executed.

But if I directly visit the 2nd page, so that the 2 actions are dispatched almost simultaneously, the 2nd action is not taken by the saga. I see both actions properly dispatched through the redux chrome extension, but the 2nd one doesn't reach the saga code. Any hints?

[yelouafi]

As redux-saga actually relies on promise chaining. It cant't handle consecutive synchronous actions. So you'd have to dispatch the 2 api calls in one batch, and take the whole in sagas (Also may be better for performance to send one api than 2).

[emirotin]

The 2 actions are from the different components, so TBH I have no idea how can I coordinate them and if I want to (actually I know that I don't — they are completely unrelated, 2 independent requests for different pieces of data). For the same reason, it doesn't make sense to change the API (which I don't own) to handle these combined requests.

Can you please give me a better idea what are "synchronous" actions and what would be "asynchronous" in that case? I really don't mind how things are called, the only thing I care about is that my middleware doesn't miss a single API request and that these requests can be dispatched independently. For now, it looks like I have to drop sagas here in favor of the custom middleware.

[emirotin]

Thanks for your detailed response, I really appreciate it!

For now, it looks like I have to drop sagas here in favor of the custom middleware.

The project is still young and evolving (2 months). So you can't expect it to handle every possible use case yet. It can only evolve and mature by gathering feedback from developers and then handling reported issues through reusable and consistent solutions.

Sorry if I sound irritated here. It wasn't meant to criticize the project by any means. I really value how you listen to your users and look open to adapting the library behavior to real use-cases (that should be a pain to address side-effects issues for someone fascinated with FP I assume :)).

I have a good understanding of promises and what you say makes sense. I should've actually considered the timeout/immediate trick myself.

Initially I thought the above issue won't occur because of how event loop works as described above.
Right, I had the same feeling that the two dispatches coming from different components should be separated by the event loop ticks.

I think the actual problem here is the mental dissonance. Sagas are middleware and other middlewares are guaranteed to be called for every action dispatched. When one reads the saga code it looks like it will be the case as well, with just the code being suspended between the events. So one (me :)) imagines the unhandled actions queued until the saga unshifts them and executes.

Thinking about this how about this possible solution: a special saga kind (daemon) that declaratively defines its trigger:

function* callApi(getState, action) { ... }

export default const myDaemon = daemon(CALL_API , callApi)

This will create a normal middleware that picks every CALL_API action and forks the callApi method.
Such kind of notation will also make the actual sagas code shorter as I always find myself writing

function* myDaemon(getState) {
  while (true) {
    const action = take(CALL_API)
    // do actual stuff here
  }
}

[emirotin]

Update: setImmediate (Chrome has native implementation and in this case the library bails out) doesn't work for me. setTimeout or raf (https://www.npmjs.com/package/raf) work fine.

[emirotin]

Just sharing my needs as the user: I care about 2 things:

• not missing actions (its really hard for me to imagine the case where I really want the middleware to miss the action. If the events need debouncing (scroll events, for example) that should be addressed in the events source IMO. Redux actions in my understanding are intents that should not be missed
• having the expressive, readable and manageable code

Sagas indeed have a steep learning curve, but I managed to grasp it quite quickly.
The biggest advantage I have is with complex flows like: after the API request X is done emit the socket event Y to get further updates for the resource, also subscribe to socket event Z and translate it to action T, as well as notify the store that it was assigned the subscription ID U so that we can later unsubscribe.
Saga works really well here in terms of expressiveness and manageability.
As long as that saga is started I really don't care how it runs, I only want it to run forever and be fired on every action X.

As sagas already work here I've migrated the API middleware to saga as well. Again I want it to run as a daemon and I don't need any coordination of that - just run forever and issue API calls on each CALL_API intent action.

I actually like the idea of channels. It could be even like that:

const apiChannel = channel.fromStoreActions(CALL_API) // it can get the store from the wrapping middleware I think
// const eventsChannel = channel.fromObservable()
// const socketEventsChannel = channel.fromGenerator()
// const thirdPartyServiceChannel = channel.fromCallback(service.sibscribe.bind(service, someArgs))

export default sagaFromChannel(channel, singleActionSaga)

A generic action queue can't determine in advance which action to keep or discard

Yeah, that's why I suggest the API where it declares in advance which action it depends on
But channels look more generic and flexible indeed. Though they add another level of abstraction

[yelouafi]

Actually I think we may need both solutions.

The daemon approach could serve many uses cases where all we're doing is replicating the same behavior inside a while(true). it also doesn't have the flaw of missing actions

function* rootSaga() {

 // avoids state-less/logic-less while(true)
  yield on('SOME_ACTION', handlerSaga)
  ...
}

The channel approach would serve for non trivial complex flow and also solves the problem of missing actions

and even daemon sagas instances can coordinate using a dedicated channel

[emirotin]

I like it :)

On Mon, Jan 25, 2016 at 5:42 PM Yassine Elouafi [email protected]
wrote:

Actually I think we may need both solutions.

The daemon approach could serve many uses cases where all we're doing is
replicating the same behavior inside a main true. it also doesn't have the
flaw of missing actions

function* rootSaga() {

// avoids state-less/logic-less while(true)
yield on('SOME_ACTION', handlerSaga)
...
}

The channel approach would serve for non trivial complex flow and also
solves the problem of missing actions

and even daemon sagas instances can coordinate using a dedicated channel

—
Reply to this email directly or view it on GitHub
#50 (comment).

[tgriesser]

Excellent explanation and proposed solution!

[mxstbr]

👍 Amazing work @yelouafi!

[jbaxleyiii]

Awesome stuff @yelouafi! I love the explanation (both here and on twitter). Super informative

[emirotin]

Great news, eager to try it 👍

[andrewhickey]

Nice, thanks! Looking forward to trying this out

[fhelwanger]

Great work @yelouafi ! 👍

This issue has just affected me, I was dispatching on componentDidMount.

I'll just use setTimeout for now.

[yelouafi]

@fhelwanger v0.6.0 is already out with the new changes. You can try it and see.

[fhelwanger]

Perfect! Tested it and is working as expected.

Thank you so much @yelouafi

[jskz]

Thank you for the clarification and fix @yelouafi

[tgriesser]

Cool thanks for all that @yelouafi!

Do you still think on is something you'll consider adding for the case of a stateless while(true)?

[yelouafi]

@tgriesser with the new promise-less version; on helper can be created like this

function* on(actionType, workerSaga) {
  while(true) {
    const nextAction = yield take(actionType)
    yield fork(workerSaga)
  }
}

// start incrementAsync on each INCREMENT_ASYNC
on(INCREMENT_ASYNC, incrementAsync)

Other concurrency patterns can also be created (as discussed here #68 (comment))

So I'm not sure if an addition to the core is needed. Thinking of creating a separate repo like redux-saga-contrib where we can gather multiple helpers for different concurrencey patterns

[emirotin]

+1 for separate repo, minimalistic core API is good
On Ср, 3 февр. 2016 г. at 20:57 Yassine Elouafi [email protected]
wrote:

@tgriesser https://github.com/tgriesser with the new promise-less
version; on helper can be created like this

function* on(actionType, workerSaga) {
while(true) {
const nextAction = yield take(actionType)
yield fork(workerSaga)
}
}
// start incrementAsync on each INCREMENT_ASYNCon(INCREMENT_ASYNC, incrementAsync)

Other concurrency patterns can also be created (as discussed here #68
(comment)
#68 (comment))

So I'm not sure if an addition to the core is needed. Thinking of creating
a separate repo like redux-saga-contrib where we can gather multiple
helpers for different concurrencey patterns

—
Reply to this email directly or view it on GitHub
#50 (comment).

[tgriesser]

Oh of course! Looks great, thanks!