From 5b0a159a018662ea7836e72d8ee95d3aebd30cef Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Mon, 10 Aug 2026 21:21:14 +0200
Subject: [PATCH] fix(smtp): explain why a test email failed instead of
 'deadline has elapsed' (#10620)

* fix(smtp): explain why a test email failed instead of 'deadline has elapsed'

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(smtp): keep non-SMTP error codes and retire a stale test alert

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to f0df8b82c4c089d384423ed64b8504506084820d

This commit updates the EE repository reference after PR #721 was merged in windmill-ee-private.

Previous ee-repo-ref: 1ffaf3dea81e007c6c11146c1e12e97e83f5b938

New ee-repo-ref: f0df8b82c4c089d384423ed64b8504506084820d

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
---
 backend/ee-repo-ref.txt                       |  2 +-
 backend/windmill-api-settings/src/lib.rs      | 23 +++++++++++++--
 backend/windmill-common/src/email_oss.rs      |  5 ++++
 .../instanceSettings/SmtpSettings.svelte      | 29 +++++++++++++++++--
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt
index 91d0c51cf7..f6c4f387b4 100644
--- a/backend/ee-repo-ref.txt
+++ b/backend/ee-repo-ref.txt
@@ -1 +1 @@
-181fa0c206d7f84a289b4396a7f7764bc815d284
+f0df8b82c4c089d384423ed64b8504506084820d
diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs
index 92621d526c..7c2453d848 100644
--- a/backend/windmill-api-settings/src/lib.rs
+++ b/backend/windmill-api-settings/src/lib.rs
@@ -52,7 +52,7 @@ use windmill_common::secret_backend::{
 use windmill_common::{
     auth::is_super_admin_email,
     ee_oss::{get_license_plan, LicensePlan},
-    email_oss::send_email_plain_text,
+    email_oss::{send_email_plain_text, SMTP_ENABLED},
     error::{self, pg_error_message, JsonResult, Result},
     get_database_url,
     global_settings::{
@@ -237,10 +237,19 @@ pub async fn test_email(
     Json(test_email): Json<TestEmail>,
 ) -> error::Result<String> {
     require_super_admin(&db, &authed.email).await?;
+    if !SMTP_ENABLED {
+        return Err(error::Error::Generic(
+            axum::http::StatusCode::NOT_IMPLEMENTED,
+            "This Windmill build was compiled without SMTP support, so no email can be sent."
+                .to_string(),
+        ));
+    }
     let smtp = test_email.smtp;
     let to = test_email.to;
 
-    let client_timeout = Duration::from_secs(3);
+    // A connection attempt covers TCP, the TLS handshake, EHLO and authentication against a remote
+    // provider; a tighter budget times out before the server ever states why it refused.
+    let client_timeout = Duration::from_secs(20);
     send_email_plain_text(
         "Test email from Windmill",
         "Test email content",
@@ -248,7 +257,15 @@ pub async fn test_email(
         smtp,
         Some(client_timeout),
     )
-    .await?;
+    .await
+    // The SMTP layer already phrases its failures for an instance admin; the anyhow wrapper it
+    // comes back in would bury that behind "Internal: ... @<source location>".
+    .map_err(|e| match e {
+        error::Error::Anyhow { error, .. } => {
+            error::Error::Generic(axum::http::StatusCode::BAD_REQUEST, format!("{error:#}"))
+        }
+        e => e,
+    })?;
 
     Ok("Sent test email".to_string())
 }
diff --git a/backend/windmill-common/src/email_oss.rs b/backend/windmill-common/src/email_oss.rs
index 2813ebe458..38d634699b 100644
--- a/backend/windmill-common/src/email_oss.rs
+++ b/backend/windmill-common/src/email_oss.rs
@@ -5,6 +5,11 @@ pub use crate::email_ee::*;
 #[cfg(not(feature = "private"))]
 use crate::server::Smtp;
 
+/// Every send below is a no-op in this build, so callers that report success to a user (the
+/// instance-settings SMTP test) have to say so instead of claiming the email went out.
+#[cfg(not(feature = "private"))]
+pub const SMTP_ENABLED: bool = false;
+
 #[cfg(not(feature = "private"))]
 pub async fn send_email(
     _subject: &str,
diff --git a/frontend/src/lib/components/instanceSettings/SmtpSettings.svelte b/frontend/src/lib/components/instanceSettings/SmtpSettings.svelte
index f7607c5d6c..f87dce7ed6 100644
--- a/frontend/src/lib/components/instanceSettings/SmtpSettings.svelte
+++ b/frontend/src/lib/components/instanceSettings/SmtpSettings.svelte
@@ -12,7 +12,7 @@
 </script>
 
 <script lang="ts">
-	import { Button } from '$lib/components/common'
+	import { Alert, Button } from '$lib/components/common'
 	import Password from '../Password.svelte'
 	import Toggle from '../Toggle.svelte'
 	import { SettingService } from '$lib/gen'
@@ -29,8 +29,19 @@
 	let { values, disabled = false }: Props = $props()
 
 	let testEmail = $state('')
+	let testing = $state(false)
+	let lastFailure = $state<{ settings: string; message: string } | undefined>(undefined)
+
+	// The failure describes one exact set of settings, so editing any of them retires it. The
+	// settings object is mutated in place, hence comparing content rather than identity.
+	let smtpSettingsKey = $derived(JSON.stringify($values['smtp_settings'] ?? {}))
+	let testError = $derived(
+		lastFailure?.settings === smtpSettingsKey ? lastFailure.message : undefined
+	)
 
 	async function testSmtpSettings() {
+		testing = true
+		lastFailure = undefined
 		try {
 			await SettingService.testSmtp({
 				requestBody: {
@@ -48,7 +59,15 @@
 			})
 			sendUserToast('Test email sent successfully')
 		} catch (error) {
-			sendUserToast('Failed to send test email: ' + error.message, true)
+			// The backend spells out which SMTP step failed and which setting to change, so it goes
+			// in a persistent alert rather than a toast that scrolls away while it is read.
+			lastFailure = {
+				settings: smtpSettingsKey,
+				message: error.body || error.message || 'Unknown error'
+			}
+			sendUserToast('Failed to send test email', true)
+		} finally {
+			testing = false
 		}
 	}
 </script>
@@ -171,6 +190,7 @@
 					unifiedSize="md"
 					variant="accent"
 					onclick={testSmtpSettings}
+					loading={testing}
 					disabled={!testEmail || !isSmtpSettingsValid($values['smtp_settings']) || disabled}
 					btnClasses="text-xs"
 					startIcon={{ icon: Mail }}
@@ -178,6 +198,11 @@
 					Send test email
 				</Button>
 			</div>
+			{#if testError}
+				<Alert type="error" title="Test email failed" size="xs" class="mt-1">
+					<span class="whitespace-pre-wrap break-words">{testError}</span>
+				</Alert>
+			{/if}
 		</div>
 	</div>
 </div>