From 5e9b4cfa99d2f160590c7e07a3abd42e1ee6e97f Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 11 Feb 2026 06:27:21 +0000 Subject: [PATCH] nit UI + more tests --- backend/tests/error_handler.rs | 411 ++++++++++++++++++ .../lib/components/NumberTypeNarrowing.svelte | 4 +- .../settingsPanel/CSSMigrationModal.svelte | 2 +- .../editor/settingsPanel/GridNavbar.svelte | 2 +- .../apps/editor/settingsPanel/GridTab.svelte | 2 +- .../settingsPanel/InputsSpecsEditor.svelte | 2 +- .../editor/settingsPanel/TableActions.svelte | 2 +- .../secondaryMenu/SecondaryMenu.svelte | 2 +- .../lib/components/common/CloseButton.svelte | 9 +- .../components/common/button/Button.svelte | 2 +- .../common/drawer/DrawerContent.svelte | 2 +- .../lib/components/common/modal/Modal.svelte | 2 +- .../common/seconds/SecondsInput.svelte | 10 +- .../components/select/DraggableTags.svelte | 2 +- .../lib/components/select/MultiSelect.svelte | 2 +- .../src/lib/components/select/Select.svelte | 2 +- .../DataTableSettings.svelte | 2 +- .../workspaceSettings/DucklakeSettings.svelte | 2 +- .../workspaceSettings/StorageSettings.svelte | 2 +- 19 files changed, 436 insertions(+), 28 deletions(-) create mode 100644 backend/tests/error_handler.rs diff --git a/backend/tests/error_handler.rs b/backend/tests/error_handler.rs new file mode 100644 index 0000000000..cad9336ab7 --- /dev/null +++ b/backend/tests/error_handler.rs @@ -0,0 +1,411 @@ +use sqlx::{Pool, Postgres}; + +mod common; +use common::*; + +/// Test that workspace error handler can be set and removed via database operations +#[cfg(feature = "deno_core")] +#[sqlx::test(fixtures("base"))] +async fn test_error_handler_settings(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let _server = ApiServer::start(db.clone()).await?; + + // Initially error_handler should be NULL + let initial = sqlx::query_scalar!( + r#"SELECT error_handler->>'path' FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert!(initial.is_none()); + + // Set error handler with all options + sqlx::query!( + r#" + UPDATE workspace_settings + SET error_handler = '{"path": "script/f/test/error_handler", "extra_args": {"notify": true}, "muted_on_cancel": true, "muted_on_user_path": false}'::jsonb + WHERE workspace_id = 'test-workspace' + "# + ) + .execute(&db) + .await?; + + let after_set = sqlx::query_scalar!( + r#"SELECT error_handler->>'path' FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert_eq!( + after_set, + Some("script/f/test/error_handler".to_string()) + ); + + // Verify extra_args + let extra_args = sqlx::query_scalar!( + r#"SELECT error_handler->'extra_args' FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert!(extra_args.is_some()); + + // Verify muted_on_cancel + let muted_on_cancel = sqlx::query_scalar!( + r#"SELECT (error_handler->>'muted_on_cancel')::boolean FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert_eq!(muted_on_cancel, Some(true)); + + // Verify muted_on_user_path + let muted_on_user_path = sqlx::query_scalar!( + r#"SELECT (error_handler->>'muted_on_user_path')::boolean FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert_eq!(muted_on_user_path, Some(false)); + + // Remove error handler + sqlx::query!( + r#" + UPDATE workspace_settings + SET error_handler = NULL + WHERE workspace_id = 'test-workspace' + "# + ) + .execute(&db) + .await?; + + let after_remove = sqlx::query_scalar!( + r#"SELECT error_handler->>'path' FROM workspace_settings WHERE workspace_id = 'test-workspace'"# + ) + .fetch_one(&db) + .await?; + assert!(after_remove.is_none()); + + Ok(()) +} + +/// Test that a failed job triggers the workspace error handler +#[cfg(all(feature = "deno_core", feature = "enterprise", feature = "private"))] +#[sqlx::test(fixtures("base"))] +async fn test_error_handler_triggered_on_failure(db: Pool) -> anyhow::Result<()> { + use windmill_common::jobs::JobPayload; + use windmill_common::runnable_settings::{ConcurrencySettings, DebouncingSettings}; + use windmill_common::scripts::{ScriptHash, ScriptLang}; + + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + + // Create the error handler script + let error_handler_code = r#" +export async function main(path: string, email: string, job_id: string, is_flow: boolean, workspace_id: string, error: any) { + console.log("Error handler called for job:", job_id); + return { handled: true, original_path: path }; +} +"#; + + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock) + VALUES ('test-workspace', 1111111111, 'f/test/error_handler', $1, 'deno', 'script', 'test-user', '{}', 'Error handler script', 'Handles failed job completions', '') + "#, + error_handler_code + ) + .execute(&db) + .await?; + + // Create a script that will fail + let failing_script_code = "export function main() { throw new Error('intentional failure'); }"; + let failing_script_hash: i64 = 2222222222; + + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock) + VALUES ('test-workspace', $1, 'f/test/failing_script', $2, 'deno', 'script', 'test-user', '{}', 'Failing test script', 'A script that always fails', '') + "#, + failing_script_hash, + failing_script_code + ) + .execute(&db) + .await?; + + // Set up the error handler in workspace_settings + sqlx::query!( + r#" + UPDATE workspace_settings + SET error_handler = '{"path": "script/f/test/error_handler"}'::jsonb + WHERE workspace_id = 'test-workspace' + "# + ) + .execute(&db) + .await?; + + // Create the error_handler group + sqlx::query!( + r#" + INSERT INTO group_ (workspace_id, name, summary, extra_perms) + VALUES ('test-workspace', 'error_handler', 'The group the error handler acts on behalf of', '{"u/test-user": true}') + ON CONFLICT DO NOTHING + "# + ) + .execute(&db) + .await?; + + // Run the failing script + let completed_job = RunJob::from(JobPayload::ScriptHash { + hash: ScriptHash(failing_script_hash), + path: "f/test/failing_script".to_string(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + language: ScriptLang::Deno, + priority: None, + apply_preprocessor: false, + concurrency_settings: ConcurrencySettings::default(), + debouncing_settings: DebouncingSettings::default(), + }) + .run_until_complete(&db, false, server.addr.port()) + .await; + + // Verify the job actually failed + assert!(!completed_job.success, "Job should have failed"); + + let main_job_id = completed_job.id; + + // Wait for the error handler job to be created + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + // Verify the error handler job was created + let error_handler_job = sqlx::query!( + r#" + SELECT + id, + runnable_path, + permissioned_as_email, + parent_job + FROM v2_job + WHERE workspace_id = 'test-workspace' + AND permissioned_as_email = 'error_handler@windmill.dev' + ORDER BY created_at DESC + LIMIT 1 + "# + ) + .fetch_optional(&db) + .await?; + + assert!( + error_handler_job.is_some(), + "Error handler job should have been created" + ); + + let handler_job = error_handler_job.unwrap(); + + assert_eq!( + handler_job.runnable_path.as_deref(), + Some("f/test/error_handler"), + "Error handler should run the configured script" + ); + assert_eq!( + handler_job.permissioned_as_email.as_str(), + "error_handler@windmill.dev", + "Error handler should run as error_handler user" + ); + assert_eq!( + handler_job.parent_job, + Some(main_job_id), + "Error handler should have the failed job as parent" + ); + + Ok(()) +} + +/// Test that error handler is NOT triggered when ws_error_handler_muted is set on the script +#[cfg(all(feature = "deno_core", feature = "enterprise", feature = "private"))] +#[sqlx::test(fixtures("base"))] +async fn test_error_handler_muted_on_script(db: Pool) -> anyhow::Result<()> { + use windmill_common::jobs::JobPayload; + use windmill_common::runnable_settings::{ConcurrencySettings, DebouncingSettings}; + use windmill_common::scripts::{ScriptHash, ScriptLang}; + + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + + // Create the error handler script + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock) + VALUES ('test-workspace', 3333333333, 'f/test/error_handler', 'export function main() { return "handled"; }', 'deno', 'script', 'test-user', '{}', '', '', '') + "#, + ) + .execute(&db) + .await?; + + // Create a failing script with ws_error_handler_muted = true + let failing_script_hash: i64 = 4444444444; + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock, ws_error_handler_muted) + VALUES ('test-workspace', $1, 'f/test/muted_failing_script', 'export function main() { throw new Error("fail"); }', 'deno', 'script', 'test-user', '{}', '', '', '', true) + "#, + failing_script_hash, + ) + .execute(&db) + .await?; + + // Set up the error handler + sqlx::query!( + r#" + UPDATE workspace_settings + SET error_handler = '{"path": "script/f/test/error_handler"}'::jsonb + WHERE workspace_id = 'test-workspace' + "# + ) + .execute(&db) + .await?; + + sqlx::query!( + r#" + INSERT INTO group_ (workspace_id, name, summary, extra_perms) + VALUES ('test-workspace', 'error_handler', 'Error handler group', '{"u/test-user": true}') + ON CONFLICT DO NOTHING + "# + ) + .execute(&db) + .await?; + + // Run the muted failing script + let completed_job = RunJob::from(JobPayload::ScriptHash { + hash: ScriptHash(failing_script_hash), + path: "f/test/muted_failing_script".to_string(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + language: ScriptLang::Deno, + priority: None, + apply_preprocessor: false, + concurrency_settings: ConcurrencySettings::default(), + debouncing_settings: DebouncingSettings::default(), + }) + .run_until_complete(&db, false, server.addr.port()) + .await; + + assert!(!completed_job.success, "Job should have failed"); + + // Wait and check that NO error handler job was created + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + let error_handler_job = sqlx::query_scalar!( + r#" + SELECT id + FROM v2_job + WHERE workspace_id = 'test-workspace' + AND permissioned_as_email = 'error_handler@windmill.dev' + "# + ) + .fetch_optional(&db) + .await?; + + assert!( + error_handler_job.is_none(), + "Error handler should NOT have been triggered for a muted script" + ); + + Ok(()) +} + +/// Test that error handler is NOT triggered on successful job completion +#[cfg(all(feature = "deno_core", feature = "enterprise", feature = "private"))] +#[sqlx::test(fixtures("base"))] +async fn test_error_handler_not_triggered_on_success(db: Pool) -> anyhow::Result<()> { + use windmill_common::jobs::JobPayload; + use windmill_common::runnable_settings::{ConcurrencySettings, DebouncingSettings}; + use windmill_common::scripts::{ScriptHash, ScriptLang}; + + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + + // Create the error handler script + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock) + VALUES ('test-workspace', 5555555555, 'f/test/error_handler', 'export function main() { return "handled"; }', 'deno', 'script', 'test-user', '{}', '', '', '') + "#, + ) + .execute(&db) + .await?; + + // Create a successful script + let success_script_hash: i64 = 6666666666; + sqlx::query!( + r#" + INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, summary, description, lock) + VALUES ('test-workspace', $1, 'f/test/success_script', 'export function main() { return "ok"; }', 'deno', 'script', 'test-user', '{}', '', '', '') + "#, + success_script_hash, + ) + .execute(&db) + .await?; + + // Set up the error handler + sqlx::query!( + r#" + UPDATE workspace_settings + SET error_handler = '{"path": "script/f/test/error_handler"}'::jsonb + WHERE workspace_id = 'test-workspace' + "# + ) + .execute(&db) + .await?; + + sqlx::query!( + r#" + INSERT INTO group_ (workspace_id, name, summary, extra_perms) + VALUES ('test-workspace', 'error_handler', 'Error handler group', '{"u/test-user": true}') + ON CONFLICT DO NOTHING + "# + ) + .execute(&db) + .await?; + + // Run the successful script + let completed_job = RunJob::from(JobPayload::ScriptHash { + hash: ScriptHash(success_script_hash), + path: "f/test/success_script".to_string(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + language: ScriptLang::Deno, + priority: None, + apply_preprocessor: false, + concurrency_settings: ConcurrencySettings::default(), + debouncing_settings: DebouncingSettings::default(), + }) + .run_until_complete(&db, false, server.addr.port()) + .await; + + assert!(completed_job.success, "Job should have succeeded"); + + // Wait and check that NO error handler job was created + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + let error_handler_job = sqlx::query_scalar!( + r#" + SELECT id + FROM v2_job + WHERE workspace_id = 'test-workspace' + AND permissioned_as_email = 'error_handler@windmill.dev' + "# + ) + .fetch_optional(&db) + .await?; + + assert!( + error_handler_job.is_none(), + "Error handler should NOT have been triggered for a successful job" + ); + + Ok(()) +} diff --git a/frontend/src/lib/components/NumberTypeNarrowing.svelte b/frontend/src/lib/components/NumberTypeNarrowing.svelte index d66727f16a..e7a07a6d16 100644 --- a/frontend/src/lib/components/NumberTypeNarrowing.svelte +++ b/frontend/src/lib/components/NumberTypeNarrowing.svelte @@ -51,7 +51,7 @@ {/snippet} min?.toString(), (v) => (min = v ? parseInt(v) : undefined)} + bind:value={() => min?.toString(), (v) => (min = v !== '' && v != null ? parseInt(v) : undefined)} /> @@ -64,7 +64,7 @@ {/snippet} max?.toString(), (v) => (max = v ? parseInt(v) : undefined)} + bind:value={() => max?.toString(), (v) => (max = v !== '' && v != null ? parseInt(v) : undefined)} /> diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/CSSMigrationModal.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/CSSMigrationModal.svelte index f4250d44ae..2b4ea2b6bd 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/CSSMigrationModal.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/CSSMigrationModal.svelte @@ -157,7 +157,7 @@ >
Migrate to CSS editor
(migrationModalOpen = false)} + onClick={() => (migrationModalOpen = false)} />
diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/GridNavbar.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/GridNavbar.svelte index 80e78020b3..d530d0b204 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/GridNavbar.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/GridNavbar.svelte @@ -164,7 +164,7 @@ { + onClick={() => { items = items.filter((_, i) => i !== index) }} /> diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/GridTab.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/GridTab.svelte index b1111244db..28e5316146 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/GridTab.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/GridTab.svelte @@ -205,7 +205,7 @@ bind:value={items[index].value} />
- deleteSubgrid(index)} /> + deleteSubgrid(index)} />
diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/InputsSpecsEditor.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/InputsSpecsEditor.svelte index 19dd309bec..e3ed15992c 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/InputsSpecsEditor.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/InputsSpecsEditor.svelte @@ -104,7 +104,7 @@ /> {#if deletable}
- dispatch('delete', k)} /> + dispatch('delete', k)} />
{/if} {/if} diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/TableActions.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/TableActions.svelte index 17b301d58c..3a121d7e61 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/TableActions.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/TableActions.svelte @@ -195,7 +195,7 @@
deleteComponent(component.id, item.originalIndex)} + onClick={() => deleteComponent(component.id, item.originalIndex)} />
diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/secondaryMenu/SecondaryMenu.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/secondaryMenu/SecondaryMenu.svelte index 877c48bf35..1e94952c4a 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/secondaryMenu/SecondaryMenu.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/secondaryMenu/SecondaryMenu.svelte @@ -29,7 +29,7 @@
- secondaryMenu?.close()} /> + secondaryMenu?.close()} /> {#if $secondaryMenu?.props?.type === 'style'}
Style Panel
diff --git a/frontend/src/lib/components/common/CloseButton.svelte b/frontend/src/lib/components/common/CloseButton.svelte index d7d0a58a7b..1387049ee6 100644 --- a/frontend/src/lib/components/common/CloseButton.svelte +++ b/frontend/src/lib/components/common/CloseButton.svelte @@ -1,5 +1,4 @@
{title ?? ''} diff --git a/frontend/src/lib/components/common/modal/Modal.svelte b/frontend/src/lib/components/common/modal/Modal.svelte index ee1e062eb2..806795a286 100644 --- a/frontend/src/lib/components/common/modal/Modal.svelte +++ b/frontend/src/lib/components/common/modal/Modal.svelte @@ -90,7 +90,7 @@ {style} > {#if kind == 'X'} -
(open = false)} />
(open = false)} />
{/if}
diff --git a/frontend/src/lib/components/common/seconds/SecondsInput.svelte b/frontend/src/lib/components/common/seconds/SecondsInput.svelte index 9ca42379d4..d7b50aeb91 100644 --- a/frontend/src/lib/components/common/seconds/SecondsInput.svelte +++ b/frontend/src/lib/components/common/seconds/SecondsInput.svelte @@ -142,7 +142,7 @@ onblur={handleBlur} /> {day && day > 1 ? 'days' : 'day'}{day !== 1 ? 'days' : 'day'}
@@ -166,7 +166,7 @@ onblur={handleBlur} /> {hour && hour > 1 ? 'hrs' : 'hr'}{hour !== 1 ? 'hrs' : 'hr'}
@@ -190,7 +190,7 @@ onblur={handleBlur} /> {min && min > 1 ? 'mins' : 'min'}{min !== 1 ? 'mins' : 'min'}
@@ -214,7 +214,7 @@ onblur={handleBlur} /> {sec && sec > 1 ? 'secs' : 'sec'}{sec !== 1 ? 'secs' : 'sec'}
@@ -223,7 +223,7 @@ class="bg-transparent text-secondary hover:text-primary" noBg small - on:close={() => { + onClick={() => { seconds = defaultValue }} /> diff --git a/frontend/src/lib/components/select/DraggableTags.svelte b/frontend/src/lib/components/select/DraggableTags.svelte index adb6f49e52..fc6a5838b8 100644 --- a/frontend/src/lib/components/select/DraggableTags.svelte +++ b/frontend/src/lib/components/select/DraggableTags.svelte @@ -56,7 +56,7 @@ (onRemove(item), e.stopPropagation())} + onClick={(e) => { e.stopPropagation(); onRemove(item) }} /> {/if} diff --git a/frontend/src/lib/components/select/MultiSelect.svelte b/frontend/src/lib/components/select/MultiSelect.svelte index 6de40aa235..57d2b6d7c6 100644 --- a/frontend/src/lib/components/select/MultiSelect.svelte +++ b/frontend/src/lib/components/select/MultiSelect.svelte @@ -158,7 +158,7 @@ noBg class="ml-2 remove-all bg-transparent text-hint" small - on:close={(e) => (clearValue(), e.stopPropagation())} + onClick={(e) => { e.stopPropagation(); clearValue() }} /> {/if} {:else if RightIcon} diff --git a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte index 50ca7b2436..42ed0c6eef 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte @@ -280,7 +280,7 @@ {/if} - removeDataTable(dataTableIndex)} /> + removeDataTable(dataTableIndex)} /> {/each} diff --git a/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte b/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte index a7e26228e4..041817ba05 100644 --- a/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte @@ -378,7 +378,7 @@ - removeDucklake(ducklakeIndex)} /> + removeDucklake(ducklakeIndex)} /> {/each} diff --git a/frontend/src/lib/components/workspaceSettings/StorageSettings.svelte b/frontend/src/lib/components/workspaceSettings/StorageSettings.svelte index 0ce937bd9b..9af70ce88d 100644 --- a/frontend/src/lib/components/workspaceSettings/StorageSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/StorageSettings.svelte @@ -227,7 +227,7 @@ {#if tableRow[0] !== null} { + onClick={() => { if (s3ResourceSettings.secondaryStorage) { s3ResourceSettings.secondaryStorage.splice(idx - 1, 1) s3ResourceSettings.secondaryStorage = [...s3ResourceSettings.secondaryStorage]