Repository: kestra-io/kestra
Issue: Missing plugins are not correctly handled 
Labels: bug
medium-sized-issue

- create a flow with only 1 task from a plugins 
- remove the plugin and restart the instance 
- go on the flow editor 

Issues: 
1. The flow is show as valid without any errors 
![image](https://github.com/kestra-io/kestra/assets/2064609/3acb3ede-0a9f-40c8-8cc2-dc2b95653e80)
2. Error on the console 
![image](https://github.com/kestra-io/kestra/assets/2064609/ee4a1b02-e41f-49f9-9f85-51f741715254)
3. The flow endpoint silently remove the tasks (see `tasks: []`), in previous version it should be a [FlowWithException](https://github.com/kestra-io/kestra/blob/develop/core/src/main/java/io/kestra/core/models/flows/FlowWithException.java) object returned
![image](https://github.com/kestra-io/kestra/assets/2064609/2caf002b-0ca0-4fb6-8541-02b893c85fcf)
4. If I start the execution, the flow failed with no proper log, no proper tasks : 
```
2023-08-09 08:45:17,021 WARN  jdbc-queue_4 i.k.core.models.executions.Execution [namespace: com.kestra.lde] [flow: hot-reload] [execution: 2lo2XYrQHNFouG2d3SYQfj] Flow failed from executor in 00:00:01.366 with exception 'Cannot invoke "java.util.List.size()" because "resolvedTasks" is null'
java.lang.NullPointerException: Cannot invoke "java.util.List.size()" because "resolvedTasks" is null
	at io.kestra.core.models.executions.Execution.isTerminated(Execution.java:352)
	at io.kestra.core.models.executions.Execution.isTerminated(Execution.java:342)
	at io.kestra.core.runners.ExecutorService.handleEnd(ExecutorService.java:487)
	at io.kestra.core.runners.ExecutorService.process(ExecutorService.java:66)
	at io.kestra.jdbc.runner.JdbcExecutor.lambda$executionQueue$13(JdbcExecutor.java:204)
	at io.kestra.jdbc.repository.AbstractJdbcExecutionRepository.lambda$lock$21(AbstractJdbcExecutionRepository.java:684)
```


Issue comments:
When updating the flow, an exception message is correctly displayed.

When loading the flow, a FlowWithException is correctly created, this is done at the deserialization level and set an empty list of tasks wich is normal as one of the task didn't exist so we must remove all tasks (as we don't know wich one ... and there can be several).

So the issue is:
1. On the UI: no validation is performed when opening the editor of an existing flow
2. On the UI: the graph didn't show an error when a flow is invalid
3. This has been like this since at least one year and it's a good thing as at least one of the task is invalid so it cannot be rendered properly
4. We must generates a comprehensive message for such cases

What complexifies stuff a little is that, on the UI, everything is loaded when first loading the flow, for ex the graph is not computed when displaying the topology tab but when loading the flow.

I'll see what I can do to improve the overall experience.
- The exception is not returned to the ui, see the screenshot : 
![image](https://github.com/kestra-io/kestra/assets/2064609/2caf002b-0ca0-4fb6-8541-02b893c85fcf)
- I really think the `FlowWithException` should not extends directly the `Flow` one to avoid empty task
- I will add we should never be able to start a `FlowWithException` which is actually possible 
> I really think the FlowWithException should not extends directly the Flow one to avoid empty task

This would require a lot of refactoring as this class is directly used inside the serializer used in database queries. But maybe we should directly add the exception (or an invalid boolean) in the flow itself so it'll be available in the UI.

> I will add we should never be able to start a FlowWithException which is actually possible

It's no more possible after https://github.com/kestra-io/kestra/pull/1900
> The exception is not returned to the ui, see the screenshot :

You tried with https://github.com/kestra-io/kestra/pull/1900? It's returned not when loading the flow (as there is no issue at this stage) but when validating the flow. As the API is using a Flow and not a FlowWithException, there is no exception on the UI as the API is not aware of it.

So, https://github.com/kestra-io/kestra/pull/1900 is a step in the right direction, but to go deeper we should get rid of the FlowWithException and add an exception attribute directly inside the FLow so it will be available in the UI. This can be a lot of work as a lot of places will be impacted (everywhere where a flow is loaded this field should be checked).
@loicmathieu what's the state of this? there were multiple PRs - is this done, or still planned for 0.14 or for 0.15?
@anna-geller it is waiting for feedback from Ludo, I think most of the issues are resolved now.
working well, nice :+1: 

Pull request review comments:

Patch that resolved the issue:
diff --git a/core/src/main/java/io/kestra/core/runners/Executor.java b/core/src/main/java/io/kestra/core/runners/Executor.java
index 2695ed42d3..a0a1a2b64e 100644
--- a/core/src/main/java/io/kestra/core/runners/Executor.java
+++ b/core/src/main/java/io/kestra/core/runners/Executor.java
@@ -4,6 +4,7 @@
 import io.kestra.core.models.executions.Execution;
 import io.kestra.core.models.executions.TaskRun;
 import io.kestra.core.models.flows.Flow;
+import io.kestra.core.models.flows.FlowWithException;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 
@@ -37,7 +38,7 @@ public Executor(WorkerTaskResult workerTaskResult) {
     }
 
     public Boolean canBeProcessed() {
-        return !(this.getException() != null || this.getFlow() == null || this.getExecution().isDeleted());
+        return !(this.getException() != null || this.getFlow() == null || this.getFlow() instanceof FlowWithException || this.getExecution().isDeleted());
     }
 
     public Executor withFlow(Flow flow) {
diff --git a/core/src/main/java/io/kestra/core/tasks/flows/Flow.java b/core/src/main/java/io/kestra/core/tasks/flows/Flow.java
index 1b12d9e2e9..182cf36d9e 100644
--- a/core/src/main/java/io/kestra/core/tasks/flows/Flow.java
+++ b/core/src/main/java/io/kestra/core/tasks/flows/Flow.java
@@ -8,6 +8,7 @@
 import io.kestra.core.models.executions.Execution;
 import io.kestra.core.models.executions.ExecutionTrigger;
 import io.kestra.core.models.executions.TaskRun;
+import io.kestra.core.models.flows.FlowWithException;
 import io.kestra.core.models.flows.State;
 import io.kestra.core.models.tasks.RunnableTask;
 import io.kestra.core.models.tasks.Task;
@@ -143,6 +144,10 @@ public Execution createExecution(RunContext runContext, FlowExecutorInterface fl
             throw new IllegalStateException("Cannot execute disabled flow");
         }
 
+        if (flow instanceof FlowWithException fwe) {
+            throw new IllegalStateException("Cannot execute an invalid flow: " + fwe.getException());
+        }
+
         return runnerUtils
             .newExecution(
                 flow,
diff --git a/webserver/src/main/java/io/kestra/webserver/controllers/ExecutionController.java b/webserver/src/main/java/io/kestra/webserver/controllers/ExecutionController.java
index f41f76bd55..2cbaafe02e 100644
--- a/webserver/src/main/java/io/kestra/webserver/controllers/ExecutionController.java
+++ b/webserver/src/main/java/io/kestra/webserver/controllers/ExecutionController.java
@@ -9,6 +9,7 @@
 import io.kestra.core.models.executions.ExecutionKilled;
 import io.kestra.core.models.executions.TaskRun;
 import io.kestra.core.models.flows.Flow;
+import io.kestra.core.models.flows.FlowWithException;
 import io.kestra.core.models.flows.State;
 import io.kestra.core.models.hierarchies.FlowGraph;
 import io.kestra.core.models.storage.FileMetas;
@@ -410,6 +411,10 @@ private Execution webhook(
             throw new IllegalStateException("Cannot execute disabled flow");
         }
 
+        if (flow instanceof FlowWithException fwe) {
+            throw new IllegalStateException("Cannot execute an invalid flow: " + fwe.getException());
+        }
+
         Optional<Webhook> webhook = (flow.getTriggers() == null ? new ArrayList<AbstractTrigger>() : flow
             .getTriggers())
             .stream()
@@ -465,12 +470,17 @@ public Execution trigger(
             return null;
         }
 
-        if (find.get().isDisabled()) {
+        Flow found = find.get();
+        if (found.isDisabled()) {
             throw new IllegalStateException("Cannot execute disabled flow");
         }
 
+        if (found instanceof FlowWithException fwe) {
+            throw new IllegalStateException("Cannot execute an invalid flow: " + fwe.getException());
+        }
+
         Execution current = runnerUtils.newExecution(
-            find.get(),
+            found,
             (flow, execution) -> runnerUtils.typedInputs(flow, execution, inputs, files),
             parseLabels(labels)
         );


Test patch: