Root runWorkflowLoop has inconsistent output behavior compared to its WorkflowNode children

[Blisse]

In RealWorkflowLoop#runWorkflowLoop, the order of operations after an output for the root workflow's runWorkflowLoop is:

1. output = select { .. }
2. doRender
3. output?.let { onOutput }

However, child workflows have their outputs immediately executed when the WorkflowAction is run.

• From WorkflowNode#tick we call WorkflowNode#applyAction, but notice that we call output?.let(emitOutputToParent) immediately.

This difference in behavior manifests in the root workflow performing an extra render pass before the root workflow runner can receive an output and potentially end the workflow.

This extra render pass can cause a crash if the workflow does not expect to have render called after emitting an output (because the presumption is that the output stop the workflow from running).

---

To work around this, I can wrap my root workflow R in another workflow P that has 2 states: Running and CancelledAfterOutput. Initially, P will be in the Running state, and call renderChild on R. But after 1 output from R, P transitions to the CancelledAfterOutput state, which doesn't call renderChild on R. This "fixes" the errant extra render pass.

[rjrjr]

Related to square/workflow#1021

[zach-klippenstein]

This ordering is by design, so that the internal state is consistent when the outside world sees the output: any state changes that were performed as outputs were bubbling up will have been applied when the output is emitted.

It's trivial to change the ordering, but then if you're paused in the debugger after an output is emitted, the internal state values of workflows will have been updated but the other data structures that are calculated from the state will be inconsistent since they are updated in the render pass, so it could be harder to look at the tree in the debugger, or the output from a diagnostic listener, and figure out what actions from the last tick actually did.

A more concrete example is if a middle workflow handles a worker output and the action bubbles an output up and changes its state such that, on the next render pass, the worker that emitted the output will be stopped. With the current ordering, by the time the external code sees the output, the worker will have been stopped (the action will have been fully applied). With the proposed ordering, the worker will still be running while the output is being handled.

It seems weird to me to notify external code while actions are only partially applied and the workflow tree is in an inconsistent. This is yet another reason we don't make any guarantees about how often render methods are called.

[zach-klippenstein]

Currently the main use for listening to outputs is cancelling the workflow/navigating away, so it's unlikely anyone is really interested in workflow state in those cases. I think I was over-indexing on "state consistency" here, and I agree it doesn't make sense to do another render pass if the main use case for emitting an output is to terminate the runtime. This should be an easy fix, but I'm going to change it in the new runtime only to reduce impact.

[zach-klippenstein]

Currently the main use for listening to outputs is cancelling the workflow/navigating away, so it's unlikely anyone is really interested in workflow state in those cases.

This turns out to be wrong in a very specific way. A very common pattern we use internally of using outputs to navigate "away" is to navigate "to" a legacy part of the app, where the workflow runtime is shutdown but the last snapshot is kept in the backstack, so if the user navigates back past the legacy screens, the workflow runtime is restarted from the last snapshot. If outputs are emitted before snapshots, that snapshot will be stale.

This is exactly what I was referring to by “consistency” in this comment, although I had not thought of this particular case. When the output is emitted, the runtime is in an “inconsistent” state – the action has only been “half” applied. Technically, when the rendering/snapshot is emitted first, the action has also only been “half” applied. However, renderings/snapshots and outputs are different types of streams. Rendering/snapshot is “state”, and output is an “event”. The significance of event streams is the actual emission of the event, and the timing matters. State is not time-sensitive, in a sense – there is always a “current” state, and the state may be different at two different instants but all that matters is the value of the state “now”. Another way to think about this is that event emissions "delineate" time, state changes occur “between” instants in time – between any two points in time, the state may have changed or not, but all that matters at any given instant is what the state is at that time, regardless of whether it changed from the previous instant. For this to work correctly with workflows, renderings/snapshots need to be emitted before events.