Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time-travel and hot reloading #22

Open
slorber opened this issue Dec 27, 2015 · 18 comments
Open

Time-travel and hot reloading #22

slorber opened this issue Dec 27, 2015 · 18 comments
Labels

Comments

@slorber
Copy link
Contributor

slorber commented Dec 27, 2015

Suppose we have a saga to handle authentication of an user like in your example.

What bothers me is that I'm not sure the behavior would be correct when associated with time-travel debugging. I mean if the saga has an initial variable let userConnected = false, then the user connects so userConnected = true, and then we time travel back to the beginning. Here the saga will still have userConnected = true right?

But I'm not sure it's actually a problem as this project is a middleware and what I understand of Redux devtools is that during time travel the actions do not go through the middleware chain again so the saga would not change state.

What about hot reloading of Saga code? I'm not sure here it will work at all either but not sure it is really possible to do something nice about it right?

@dts
Copy link

dts commented Dec 30, 2015

Debugging/time-travel is a "hard" problem to do with timing-dependent code. It is not easy with any of the similar middlewares (that I know of), but Saga introduces an API that could in principle make it possible to debug in a reasonable way - see #5 for some ideas in this department.

@slorber
Copy link
Contributor Author

slorber commented Dec 30, 2015

I'm more concerned about state that the sagas have. I mean if you have a long-lived saga and it uses local variables, then when we time-travel it's not easy to update this state accordingly, and worse the saga also has some kind of "progress state" (I mean at which execution point the Saga is currently).

Generators seems to allow an user-friendly syntax but also seems to introduce some kind of implicit state and I don't really see how to deal with that.

@yelouafi
Copy link
Member

Generators seems to allow an user-friendly syntax but also seems to introduce some kind of implicit state and I don't really see how to deal with that.

IMO it's not only about Generators. Although I agree that the flexibility offered by Generators makes it a bit harder to achieve. I think it's more related to the complex nature of long-running flows, I mean flows which span across multiple actions.

Even if we implement a saga as a state machine which reacts to a sequence of actions like we can do actually with redux-thunk (or even in a more restrictive way like in Cerebral). It doesn't mean we can time travel - correctly - that code. Time travel is driven by the event-log/actions (at least in its actual conception). Give me an action, the previous state. I'll give you an output and the next state. But imagine a long-running authorization flow

SIGN_IN -> authorization -> SIGN_OUT

In theory, you can time travel the code managing the above flow - and probably it`s easier with a state machine like approach -. But imagine you change requirements of the above flow into something like

SIGN_IN -> authorization -> (refresh with timeout of 5 mins) -> SIGN_OUT

You change your code and hot reload it, how to replay actions with the new state machine code ? the whole flow has changed, it means the state machine will probably produce a new sequence of actions: the recorded event log is no longer valid, so it wont make sens to reply it.

@dts
Copy link

dts commented Jan 2, 2016

In my experience debugging sagas, there were a few different behaviors that would have made sense at different times, and that was in the span of a very short time. The general takeaway that I had was that trying to debug these effects without the redux dev tools visible and able to "turn off" some of the effects from a previous-reload saga, it was awkward to debug stuff. I'd change some code to change the behavior of a saga, which would hot-load things, prevent the previous saga from completing, and I'd have to undo the actions the saga performed (leaving it in a now-incorrect state), so that I could trigger it again. This was fine in my case, because of the simplicity involved. Using the reset/revert/sweep/commit buttons would make this approachable for more complex interactions.

All this is to say that this does complicate the dream of "just keep iterating, and don't touch the app much", though I don't see a way of dealing with this unless we build sagas into a dev tool. In such a dev tool, we could indicate what kind of behavior we want, such as "I am interested in THIS point in the saga" - if we add first-class support for timeouts in the sagas API, we can have the saga timeline appear with a "scrubber" that moves in response to user action & the flow of time, but we can also "peg" it somewhere. Every time a reload happens, we play back the actions and the saga flow until we get to that point in the saga.

@yelouafi
Copy link
Member

yelouafi commented Jan 5, 2016

Time travelling Sagas, IMO, is not the real challenge. If we record the states at the different breakpoints (i.e. yields) of a saga. We can travel to any point in the past and see the app/state at that point. We can even time travel different Sagas independently, as if there is a local time for each saga (vs the global time defined in the current devtools)

what's really challenging is hot reloading saga code. And this is a conceptual issue, because modifying an existing saga code can lead to an action path totally different from the recorded one. Hot reloading Sagas can change the past. At the point of time a hot reloaded Saga dispatches an action different from the one dispatched in the old saga code, the remaining actions in the log no longer make sens, because we can now have a future totally different from the recorded one.

A possible way to hot reload control flow, IMO, is to replay the control flow from the beginning. We have to exclude any 'saga triggered action' from the log and take 'the user triggered actions' as a single source of truth: something similar to what @slorber called 'command sourcing' (#8 (comment)). Then we have to replay the Saga from the beginning by re-dispatching the recorded user actions to the store. So we'll effectively recompute a new action log. A side issue of this approach is that replaying the Saga will re-triggers all api calls inside the Saga (or possibly will trigger new api calls with the modified saga code). But at least we don't have to repeat the UI actions manually to test the new Saga code.

Those UI triggered actions seems more like Cerebral signals. A user triggered action can lead to different cascaded actions (like REQUEST -> SUCCESS) fired by the Saga. So instead of a flat view of the action log (like the actual devtools view), we'd have a structured view : user actions/program actions

this approach would also enable devtools to work with any other async middleware, as long the devtools can

1- make a distinction between user actions/program actions
2- link program actions to their parent user actions

@tappleby
Copy link

tappleby commented Jan 5, 2016

A side issue of this approach is that replaying the Saga will re-triggers all api calls inside the Saga

This seems like an undesired effect (pun intended), especially if you have API calls creating resources on the server.

This is more of a brain dump; I'm not sure if this makes sense or is possible to implement in a maintainable way:

With "pure effect/operations" it seems like you could safely replay these to get the saga back into the proper state without actually executing the effects (would involve capturing the effect description + result); The challenge is what to do with impure operations (and how to detect them).

Once you hit a point where a saga starts to branch from the previous timeline any future actions or operations (in redux devtools / all sagas) are no longer valid since they could depend on values which were only valid in the "alternate universe".

Git almost has a similar issue with merge conflicts where with manual user intervention you can get to the desired end result; I could see a devtool possibly having a similar interface which would give you the choice to keep the previous result or execute the effect again when we detect a possible branch point; If the effect had the same result it would be safe to continue replaying future actions + operations, otherwise we would have to stop at the point it branched.

@kurtharriger
Copy link

Technically it is possible to externalize the state of an iterator. See ramdajs.unfold for example. Perhaps some sort of babel transform might make it possible to capture the state of a generator.

However you will run into the issue that not all state is serializable in a meaningful way, a websocket once closed is closed, a completed promise once completed wont complete again unless restarted. You would probably need some sort of hook that can be called when generator state is being serialized and deserialized to allow this state to restore the state or reset it as needed.

A bit like suspending a laptop, internet connections will be lost and it may or may not be possible to restore them when the laptop resumes... but its a feature I use everyday and have all but forgotten how much time I wasted shutting down and booting up my laptop and with a few hooks from the os to inform the process that it is being suspended and resumed it works quite well.

I don't think it will be easy to do with generators, but it can be done with externalized state and a few hooks.

@slorber
Copy link
Contributor Author

slorber commented Jan 6, 2016

Just to wonder, anybody have a real usecase where he would like to alter the past with a redux-saga?

I think it's quite complicated to manage. If hot-reloading of the saga only affects the future I'll be fine with that.

@ghost
Copy link

ghost commented Jan 20, 2016

I've been exploring various strategies centered around redux for handling side effects, and this is one of the key implementation details that has kept me from adopting any solution: side effects break time travel (by their very nature). However, we might be able to skirt around this issue with regards to sagas.

As @slorber has mentioned, the real source of truth for an event sourced application is not a collection of states, but rather a log of events. If we capture and replay this log, we should be able to time travel by starting with a fresh application and replaying events 0...N. The trick here I believe is to morph sagas while events are being replayed. I say "morph" here because I don't think "disable" is the right word.

Consider "disabling" a saga:

In the simplest case, if you have a saga that waits for event_a then eventually performs task_a_1 ... task_a_n, you must disable the saga middleware because you don't want to duplicate task_a_1 ... task_a_n when replaying event_a. You (more than likely) already have the results of those tasks (along with any data they fetched) stored inside the event log, so as far as the non-middleware part of the application is concerned, the saga did actually trigger and the resulting side effects were pumped into the system.

In the non-simple case, you are part-way through a saga's side effects when the time travel begins. Consider a saga at step 2 out of 4 steps. If we start with a fresh application and replay events 0...N with sagas disabled, our reset saga will never receive the necessary events to move from step 0 back to step 2 where it belongs, and thus future events that would have triggered step 3 and step 4 are dropped on the floor.

My initial solution in this case is that we need to have sagas enabled, but somehow prevent them from performing their side effects... just let them capture incoming events to move into their appropriate positions. I can't think of a generic way to do this as it would, at the very least, require disabling any xhr during replay, and more than likely require other hacks of a similar fashion. But even then, disabling xhr that ultimately returns to emit another event means our saga will get stuck in an abandoned state.

Another possible solution is (at least for the purposes of debugging) to only replay events in acceptable "chunks" corresponding to sagas. If your saga is 3 steps, but the event log only has 1/3 steps completed, only replay 0...N-1 to where the saga has not yet started. I don't know how this would hold up to scrutiny.

@slorber
Copy link
Contributor Author

slorber commented Jan 20, 2016

@aft-luke instead of disabling state, the processor that executes them can simply memoize the effects. Instead of reexecuting the effect, it will simply yield its memoized return directly.

For example if you have a statement like:

const {stop, tick} = yield race({
        stop   : take('STOP'),
        tick : call(wait, ONE_SECOND);
})

During real execution, the race could return from one of the 2 effects.

During replay, if the result of the race is memoized, there's no wait/take to call at all and we simply return the value that has already been returned during real execution. It is easy to reconstruct saga state by using the memoized effects, the complicated part is to stop at a given point in time as it means tracking at which event/action we are and not executing effects after this action/effect.

Also when time-travelling it's probably not a good idea to emit any effect in the past and go to the future: it could produce weird things. However it makes sense to rollback to a past state and restart real app execution from this point, and sagas should be able to be set in an appropriate state during that rollback.

@ghost
Copy link

ghost commented Jan 20, 2016

Does that require a memoization wrapper around everything exported by io that extracts a value, such as race/take, or are we talking about a higher level memoization of the saga itself (as you say, by the executor)? I can't see how a higher level memoization of the executor would allow you to invoke the sagas to "put them back where they should be" when replaying events... so it must be the first case?

I like this idea of memoization, but it gets more complex if your side effects aren't idempotent. Given event_a, move saga to step 1 if state a - or move saga to step 2 if state b. I'm digging through a few github projects to find the comments/issues stream where others were talking about this issue; how can we eliminate getState() entirely from either a thunk or a saga, so it's simply handed exactly what it needs to know and remains idempotent.

@ghost
Copy link

ghost commented Jan 21, 2016

After some more thought, I wonder if we can get away with ignoring this issue entirely and accept a "good enough" solution. What are the chances that you want to debug something not related to a saga, but takes place during the lifetime of a saga? Seems like a very edge case to me. With that in mind, If we just accept the hard truth that "when writing/debugging sagas, we are going to have to f5", simple solutions become more plausible.

Just disable sagas when replaying events... don't worry about trying to memoize their results or capture their partially-completed state or "guarantee" that they are restored to the proper state during a replay. The caveat here is that if you want to step back in time, wait until your saga has completed (returned xhr, timeout, whatever) so the event log has everything it needs and no saga is left in a partial state.

Does that sound reasonable? Can someone think of use cases where it becomes such an inconvenience to not have time-travel in sagas that we have to have a complicated solution implementing state serialization/deserialization, generator forwarding, side effect memoization, and whatever else might be required?

@SoftMemes
Copy link

Catching up on everything redux and came across #5.

Assuming there already is a log of all past effects (as per above issue), appropriately interleaved with other events, there shouldn't be any need for explicit memoization in order to "dry run" effects. When replaying, could not a call() be turned into a take() on the event that represents the already known result? Likewise, puts would be ignored as they "have already happened".

This would cause problems when the logic / control flow of the saga changed as per comments made already, but with enough information stored about the effects, this could be made quite clear, i.e. you could flag that "last time, the saga was waiting for something else at this point".

@gaearon
Copy link
Contributor

gaearon commented Feb 29, 2016

If hot-reloading of the saga only affects the future I'll be fine with that.

I agree. Just pausing existing sagas and restarting hot reloaded ones would cover most cases where you’d care about hot reloading them, in my opinion. Sure, it’s not perfect, but it’s better than nothing, isn’t it?

@hoschi
Copy link

hoschi commented May 11, 2016

Here is my try to get HMR working with sagas. Time traveling doesn't work. Feedback is greatly appreciated :)

https://gist.github.com/hoschi/6538249ad079116840825e20c48f1690

@yelouafi
Copy link
Member

yelouafi commented May 12, 2016

@gaearon

just pausing existing sagas and restarting hot reloaded ones would cover most cases where you’d care about hot reloading them

the issue is most of the time you have only one root saga that is visible to the external world; the rest of Sagas are started 'internally' using fork/call. W'll have to determine where the modified Saga is actually in the execution tree (there maybe many instances)? and more importantly we need to determine the impact of hot reloading this Saga on other running Sagas. The most predictable way I can think of is to replay the top level Saga withe the event log (see below)

@hoschi(thanks for sharing this)

So in your solution you restart all Sagas. In the case of simple watchers (watch-and-fork) it's sufficient. But note in the case of a more complex Saga (say an authorization saga in the middle of a login/logout) restarting the Saga will put it at the beginning of the flow.

There is also the issue of the already running tasks, normally if you have only forked sagas (non spawned) it should be sufficient because cancelling the root Saga will cancel all forked tasks on the execution tree. But in the case of non attached forks (via spawn Effects) it won't work.

The solution I had in mind is to replay the Sagas with the past event log: But then we have to distinguish between 2 classes of actions

  1. External Actions (User, Websockets, ...): I'll call those Events
  2. Internal Actions (dispatched by the app in reaction to Events) I'll call them just Actions

So if we hot-reload the Saga code we replay the Sagas only with past Events, which may trigger a different internal Action log (e.g. Saga changed from dispatching ACTION_1 to dispatching ACTION_2 in reaction to an event).

There is still the issue of the api calls: we can memoize api calls to avoid hitting the server, but in the case the changed Saga code trigger Api calls with different arguments then in this case w're forced to make a real API call (or throw a message to the user/developer)

@hoschi
Copy link

hoschi commented May 12, 2016

@yelouafi thanks for the warning that problems occure with spawn effects 👍 I add a note to my code repo for my team members.

Your idea with replaying an event log sounds interresting, but has this approach the same problem with serializing things discussed already? This ist just something which cames to my mind when thinking about it, I have no deep knowledge about saga/effects, yet.

@reem
Copy link

reem commented Sep 23, 2016

I was thinking more about this problem and it occurred to me that there is already a very similar mechanism to reloading: cancellation. The simplest thing would be to just cancel any reloaded saga and run it again, but as discussed above this has the downside of reseting state.

We can look to the bare usage of HMR for a solution to this problem - any module with state that needs to be transferred to the newly loaded module can do so using dispose. We can introduce a similar mechanism for sagas:

function* reloadableSaga(hot) {
  let someState = hot.someState || defaultValue;

  while (true) {
    try {
      someState = yield call(doSomethingWith, someState);
    } finally {
      if (yield reloaded()) { // potentially reloaded can be rolled in with cancelled
         yield reloadWith({ someState });
      }
    }
  }
}

where the data passed to reloadWith is passed to the new version of reloadableSaga.

Unfortunately unlike react components sagas don't already nicely isolate their state, so users would have to do it semi-manually, but by implementing reloading support in many saga combinators (takeEvery, takeLatest, etc.) a lot of simple sagas will gain reloading support automatically.

EDIT: I think with additional libraries that feature saga helpers like takeEvery and takeLatest which make writing declarative sagas easier we could cover even more sagas automatically.

For instance an always combinator could look like this:

function* always(effect) {
  while (true) {
    try {
      yield effect;
    } finally {
       if (yield reloaded()) { // this takes care of reloading the always saga itself, not the parent
         yield reloadWith(effect);
       }
    }
  }
}

EDIT2: Thinking about it more, we can take care of the parent saga reloading by having the reloaded effect return us our new arguments from the parent, and the saga can then choose to override some of those with reloadWith or even just continue on with the new state.

willdurand referenced this issue in mozilla/addons-frontend Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants