Caching results of a node if they panic

[radeusgd]

The discussion under the PR fixing caching of dataflow errors (#7193) highlights a 'conflict' between two functionalities that we want.

We are caching regular results, and with #7193 we will be (are? if read later 🙂) caching results if they are a dataflow error. We are still not caching panics, as this has its own repercussions.

The dichotomy is as follows:

1. Panics are not cached.
   • This means that, as described in If a node fails with dataflow error in batch run, it immediately falls back to a dry run afterwards #7140 - when a run using the Play button will cause one of the nodes to Panic, after the run finishes that node will be almost immediately recomputed again in dry run mode. This is a problem, because: it triggers a spurious recomputation; and it is much harder now to debug this Panic. If it only occurs in non-dry run mode the user will see the panic for only a brief moment before it is recomputed and disappears. We can probably debug it by enabling output actions for just this node using the 'refresh' button on the node.
2. Panics are cached.
   • This means that, as described in Panic "Name ... could not be found" does not disappear after adding proper import #1549, if a Panic is caused by a missing import, it may not be recomputed after adding the import - thus even if an added import resolves the error, this may be not reflected in the graph until some other changes cause the cache to invalidate and recompute the panic. Maybe we need to invalidate the caches in some more 'smarter' way when adding imports? But that definitely sounds highly non-trivial.

Currently we are picking (1) as this is what we used to have. But that has its drawbacks. Should we keep (1) or switch to (2)? Or maybe there is some smart way to reconcile the two conflicting features?

[wdanilo]

How does it connect with the plans that we want to replace panics with dataflow errors altogether in the future? Would it not solve this issue?

[radeusgd]

I don't know enough details of how this migration is supposed to work (what changes in comparison to how currently panics were handled) to be able to say.

It seems however like it would not solve the issue immediately. It's not about panics really - it's about caching import resolution errors. Whether that error is a panic or a dataflow error - does not matter that much for this issue.

The issue is - once an import resolution error is cached - when we modify the imports, possibly fixing it, it is not automatically recomputed - because it is already cached and the engine does not 'know' about a dependency between imports and that node.

The possible solution could also be independent of the panic/dataflow-error division - we could characterize import resolution errors by some common subclass and a) not cache these; OR b) (even better IMO) cache them too, but when imports are updated - iterate through cached values and re-run the ones that represent the import resolution errors.