Repository: kestra-io/kestra
Issue: Flow with invalid type generate an empty graph
Labels: bug

If I have a missing plugins, the flow graph endpoint will generate an empty graph and not an error. 
We have remove the validation of the flow, because the we don't care missing properties, but the task type is necessary and should need an error

Issue comments:
relate to https://github.com/kestra-io/kestra/issues/1866
We should validate the flow when loading the graph on the topology tab (on the split editor the error status exists so no need to revalidate).

Pull request review comments:

Patch that resolved the issue:
diff --git a/ui/src/components/executions/ExecutionRoot.vue b/ui/src/components/executions/ExecutionRoot.vue
index a345e0f1fb..4a01bb7356 100644
--- a/ui/src/components/executions/ExecutionRoot.vue
+++ b/ui/src/components/executions/ExecutionRoot.vue
@@ -266,6 +266,8 @@
             this.closeSSE();
             window.removeEventListener("popstate", this.follow)
             this.$store.commit("execution/setExecution", undefined);
+            this.$store.commit("flow/setFlow", undefined);
+            this.$store.commit("flow/setFlowGraph", undefined);
         }
     };
 </script>
diff --git a/ui/src/components/executions/Topology.vue b/ui/src/components/executions/Topology.vue
index c4df520c29..1b6605443e 100644
--- a/ui/src/components/executions/Topology.vue
+++ b/ui/src/components/executions/Topology.vue
@@ -15,6 +15,9 @@
                 view-type="topology"
                 @expand-subflow="onExpandSubflow($event)"
             />
+            <el-alert v-else type="warning" :closable="false">
+                {{ $t("unable to generate graph")}}
+            </el-alert>
         </div>
     </el-card>
 </template>
diff --git a/ui/src/components/flows/FlowRoot.vue b/ui/src/components/flows/FlowRoot.vue
index 63035e3022..566903189d 100644
--- a/ui/src/components/flows/FlowRoot.vue
+++ b/ui/src/components/flows/FlowRoot.vue
@@ -240,6 +240,7 @@
         },
         unmounted () {
             this.$store.commit("flow/setFlow", undefined)
+            this.$store.commit("flow/setFlowGraph", undefined)
         }
     };
 </script>
diff --git a/ui/src/components/flows/Topology.vue b/ui/src/components/flows/Topology.vue
index fc10b953ea..2a0f627a50 100644
--- a/ui/src/components/flows/Topology.vue
+++ b/ui/src/components/flows/Topology.vue
@@ -12,6 +12,9 @@
                 view-type="topology"
                 @expand-subflow="onExpandSubflow($event)"
             />
+            <el-alert v-else type="warning" :closable="false">
+                {{ $t("unable to generate graph")}}
+            </el-alert>
         </div>
     </el-card>
 </template>
diff --git a/ui/src/components/inputs/EditorView.vue b/ui/src/components/inputs/EditorView.vue
index d41af84c72..209d41c455 100644
--- a/ui/src/components/inputs/EditorView.vue
+++ b/ui/src/components/inputs/EditorView.vue
@@ -706,6 +706,7 @@
         <div class="slider" @mousedown="dragEditor" v-if="combinedEditor" />
         <Blueprints v-if="viewType === 'source-blueprints' || blueprintsLoaded" @loaded="blueprintsLoaded = true" :class="{'d-none': viewType !== editorViewTypes.SOURCE_BLUEPRINTS}" embed class="combined-right-view enhance-readability" :top-navbar="false" prevent-route-info />
         <div
+            class="topology-display"
             :class="viewType === editorViewTypes.SOURCE_TOPOLOGY ? 'combined-right-view' : viewType === editorViewTypes.TOPOLOGY ? 'vueflow': 'hide-view'"
         >
             <LowCodeEditor
@@ -730,6 +731,9 @@
                     <ValidationError tooltip-placement="bottom-start" size="small" class="ms-2" :error="flowError" :warnings="flowWarnings" />
                 </template>
             </LowCodeEditor>
+            <el-alert v-else type="warning" :closable="false">
+                {{ $t("unable to generate graph")}}
+            </el-alert>
         </div>
         <PluginDocumentation
             v-if="viewType === editorViewTypes.SOURCE_DOC"
@@ -960,6 +964,7 @@
 
     .hide-view {
         width: 0;
+        overflow: hidden;
     }
 
     .plugin-doc {
@@ -985,4 +990,8 @@
             background-color: var(--bs-secondary);
         }
     }
+
+    .topology-display .el-alert {
+        margin-top: calc(3 * var(--spacer));
+    }
 </style>
diff --git a/ui/src/translations.json b/ui/src/translations.json
index 540ffe9bb9..fd98c20f9d 100644
--- a/ui/src/translations.json
+++ b/ui/src/translations.json
@@ -497,6 +497,7 @@
     "date format": "Date format",
     "timezone": "Timezone",
     "add task": "Add a task",
+    "unable to generate graph": "An issue occurred while generating the graph which prevent the topology to be displayed.",
     "attempts": "Attempt(s)"
   },
   "fr": {
@@ -989,6 +990,7 @@
     "date format": "Format de date",
     "timezone": "Fuseau horaire",
     "add task": "Ajouter une tâche",
+    "unable to generate graph": "Une erreur est survenue lors de la génération du graphe empêchant l'affichage de la topologie.",
     "attempts": "Tentative(s)"
   }
 }
diff --git a/webserver/src/main/java/io/kestra/webserver/controllers/FlowController.java b/webserver/src/main/java/io/kestra/webserver/controllers/FlowController.java
index f5d6eb636e..c04d61a1f3 100644
--- a/webserver/src/main/java/io/kestra/webserver/controllers/FlowController.java
+++ b/webserver/src/main/java/io/kestra/webserver/controllers/FlowController.java
@@ -4,6 +4,7 @@
 import io.kestra.core.exceptions.InternalException;
 import io.kestra.core.models.SearchResult;
 import io.kestra.core.models.flows.Flow;
+import io.kestra.core.models.flows.FlowWithException;
 import io.kestra.core.models.flows.FlowWithSource;
 import io.kestra.core.models.hierarchies.FlowGraph;
 import io.kestra.core.models.tasks.Task;
@@ -91,10 +92,25 @@ public FlowGraph flowGraph(
         @Parameter(description = "The flow revision") @QueryValue Optional<Integer> revision,
         @Parameter(description = "The subflow tasks to display") @Nullable @QueryValue List<String> subflows
     ) throws IllegalVariableEvaluationException {
-        return flowRepository
+        Flow flow = flowRepository
             .findById(namespace, id, revision)
-            .map(throwFunction(flow -> graphService.flowGraph(flow, subflows)))
             .orElse(null);
+
+        String flowUid = revision.isEmpty() ? Flow.uidWithoutRevision(namespace, id) : Flow.uid(namespace, id, revision);
+        if(flow == null) {
+            throw new NoSuchElementException(
+                "Unable to find flow " + flowUid
+            );
+        }
+
+        if(flow instanceof FlowWithException fwe) {
+            throw new IllegalStateException(
+                "Unable to generate graph for flow " + flowUid +
+                    " because of exception " + fwe.getException()
+            );
+        }
+
+        return graphService.flowGraph(flow, subflows);
     }
 
     @ExecuteOn(TaskExecutors.IO)


Test patch:
diff --git a/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java b/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java
index b1eb0429c9..005a8f9c38 100644
--- a/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java
+++ b/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java
@@ -103,8 +103,7 @@ void taskNotFound() {
 
     @Test
     void graph() {
-        FlowGraph result = client.toBlocking().retrieve(HttpRequest.GET("/api/v1/flows/io.kestra" +
-            ".tests/all-flowable/graph"), FlowGraph.class);
+        FlowGraph result = client.toBlocking().retrieve(HttpRequest.GET("/api/v1/flows/io.kestra.tests/all-flowable/graph"), FlowGraph.class);
 
         assertThat(result.getNodes().size(), is(38));
         assertThat(result.getEdges().size(), is(42));
@@ -114,6 +113,14 @@ void graph() {
         ));
     }
 
+    @Test
+    void graph_FlowNotFound() {
+        HttpClientResponseException exception = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().retrieve(GET("/api/v1/flows/io.kestra.tests/unknown-flow/graph")));
+
+        assertThat(exception.getStatus(), is(NOT_FOUND));
+        assertThat(exception.getMessage(), is("Not Found: Unable to find flow io.kestra.tests_unknown-flow"));
+    }
+
     @Test
     void idNotFound() {
         HttpClientResponseException e = assertThrows(HttpClientResponseException.class, () -> {