From 8a277a0ad9f076988e6dfb0500e651129affd879 Mon Sep 17 00:00:00 2001
From: HugoCasa <hugo@casademont.ch>
Date: Wed, 25 Sep 2024 09:10:41 +0200
Subject: [PATCH] do not call workspace error handler if flow has error handler
 (#4434)

* do not call workspace error handler if flow has error handler

* optimize + UI improvements for error handler
---
 backend/windmill-queue/src/jobs.rs            | 17 +++++++++-
 .../src/lib/components/FlowGraphViewer.svelte | 10 ++++--
 .../lib/components/FlowGraphViewerStep.svelte | 33 +++++++++++++------
 .../src/lib/components/graph/graphBuilder.ts  |  4 +++
 4 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/backend/windmill-queue/src/jobs.rs b/backend/windmill-queue/src/jobs.rs
index 68f415e7cf69a..ec2e688871caa 100644
--- a/backend/windmill-queue/src/jobs.rs
+++ b/backend/windmill-queue/src/jobs.rs
@@ -475,6 +475,11 @@ where
     }
 }
 
+#[derive(Deserialize)]
+struct RawFlowFailureModule {
+    failure_module: Option<Box<RawValue>>,
+}
+
 #[instrument(level = "trace", skip_all)]
 pub async fn add_completed_job_error<R: rsmq_async::RsmqConnection + Clone + Send>(
     db: &Pool<Postgres>,
@@ -922,7 +927,17 @@ pub async fn add_completed_job<
             )
             .await;
         } else if !skip_downstream_error_handlers
-            && matches!(queued_job.job_kind, JobKind::Flow | JobKind::Script)
+            && (matches!(queued_job.job_kind, JobKind::Script)
+                || matches!(queued_job.job_kind, JobKind::Flow)
+                    && queued_job
+                        .raw_flow
+                        .as_ref()
+                        .and_then(|v| {
+                            serde_json::from_str::<RawFlowFailureModule>((**v).get())
+                                .ok()
+                                .and_then(|v| v.failure_module)
+                        })
+                        .is_none())
             && queued_job.parent_job.is_none()
         {
             let result = serde_json::from_str(
diff --git a/frontend/src/lib/components/FlowGraphViewer.svelte b/frontend/src/lib/components/FlowGraphViewer.svelte
index afeb0438b1fc3..ec5f4523dc846 100644
--- a/frontend/src/lib/components/FlowGraphViewer.svelte
+++ b/frontend/src/lib/components/FlowGraphViewer.svelte
@@ -38,8 +38,14 @@
 				failureModule={flow?.value?.failure_module}
 				preprocessorModule={flow?.value?.preprocessor_module}
 				on:select={(e) => {
-					const mod = dfs(flow?.value?.modules ?? [], (m) => m).find((m) => m?.id === e?.detail)
-					stepDetail = mod ?? e.detail
+					if (e?.detail === 'failure') {
+						stepDetail = flow?.value?.failure_module
+					} else if (e?.detail === 'preprocessor') {
+						stepDetail = flow?.value?.preprocessor_module
+					} else {
+						stepDetail = dfs(flow?.value?.modules ?? [], (m) => m).find((m) => m?.id === e?.detail)
+					}
+					stepDetail = stepDetail ?? e?.detail
 					dispatch('select', stepDetail)
 				}}
 			/>
diff --git a/frontend/src/lib/components/FlowGraphViewerStep.svelte b/frontend/src/lib/components/FlowGraphViewerStep.svelte
index 789c0eed93d84..b2b7b9d381d00 100644
--- a/frontend/src/lib/components/FlowGraphViewerStep.svelte
+++ b/frontend/src/lib/components/FlowGraphViewerStep.svelte
@@ -103,12 +103,17 @@
 	{:else if typeof stepDetail != 'string' && stepDetail.value}
 		<div class="">
 			<div class="sticky top-0 bg-surface w-full flex items-center py-2">
-				{#if stepDetail.id}
+				{#if stepDetail.id && stepDetail.id != 'failure' && stepDetail.id != 'preprocessor'}
 					<Badge color="indigo">
 						{stepDetail.id}
 					</Badge>
 				{/if}
-				<span class="ml-2 font-medium text-lg">
+				<span
+					class={twMerge(
+						'font-medium text-lg',
+						stepDetail.id !== 'failure' && stepDetail.id !== 'preprocessor' ? 'ml-2' : ''
+					)}
+				>
 					{#if stepDetail.summary}
 						{stepDetail.summary}
 					{:else if stepDetail.value.type == 'identity'}
@@ -124,6 +129,10 @@
 						Inner flow
 					{:else if stepDetail.value.type == 'whileloopflow'}
 						While loop
+					{:else if stepDetail.id === 'failure'}
+						Error handler
+					{:else if stepDetail.id === 'preprocessor'}
+						Preprocessor
 					{:else}
 						Anonymous step
 					{/if}
@@ -147,10 +156,12 @@
 				An identity step returns its inputs as outputs
 			</p>
 		{:else if stepDetail.value.type == 'rawscript'}
-			<div class="text-xs">
-				<h3 class="mb-2 font-semibold mt-2">Step Inputs</h3>
-				<InputTransformsViewer inputTransforms={stepDetail?.value?.input_transforms ?? {}} />
-			</div>
+			{#if stepDetail.id !== 'preprocessor'}
+				<div class="text-xs">
+					<h3 class="mb-2 font-semibold mt-2">Step Inputs</h3>
+					<InputTransformsViewer inputTransforms={stepDetail?.value?.input_transforms ?? {}} />
+				</div>
+			{/if}
 
 			<div>
 				<div class="mb-2 mt-4 flex justify-between items-center">
@@ -180,10 +191,12 @@
 				</div>
 			</div>
 		{:else if stepDetail.value.type == 'script'}
-			<div class="text-2xs">
-				<h3 class="mb-2 font-semibold mt-2">Step Inputs</h3>
-				<InputTransformsViewer inputTransforms={stepDetail?.value?.input_transforms ?? {}} />
-			</div>
+			{#if stepDetail.id !== 'preprocessor'}
+				<div class="text-2xs">
+					<h3 class="mb-2 font-semibold mt-2">Step Inputs</h3>
+					<InputTransformsViewer inputTransforms={stepDetail?.value?.input_transforms ?? {}} />
+				</div>
+			{/if}
 			{#if stepDetail.value.path.startsWith('hub/')}
 				<div class="flex flex-col grow">
 					<div class="mb-2 flex justify-between items-center">
diff --git a/frontend/src/lib/components/graph/graphBuilder.ts b/frontend/src/lib/components/graph/graphBuilder.ts
index 6805627db0229..92f6b4942336f 100644
--- a/frontend/src/lib/components/graph/graphBuilder.ts
+++ b/frontend/src/lib/components/graph/graphBuilder.ts
@@ -451,6 +451,10 @@ export default function graphBuilder(
 		addEdge(id, 'Input', { type: 'empty' })
 	}
 
+	if (failureModule && !extra.flowModuleStates) {
+		addNode(failureModule, 0, 'module')
+	}
+
 	Object.keys(parents).forEach((key) => {
 		const node = nodes.find((n) => n.id === key)