Merge pull request #3987 from bluewave-labs/fix/email-error-handling

fix: improve email error handling

server/src/api/controllers/notificationController.ts

@@ -35,11 +35,18 @@ class NotificationController implements INotificationController {
 
 	testNotification = catchAsync(async (req: Request, res: Response) => {
 		const notification = testNotificationBodyValidation.parse(req.body);
-		const success = await this.notificationsService.sendTestNotification(notification);
+		let success = false;
+		let reason: string | undefined;
+		try {
+			success = await this.notificationsService.sendTestNotification(notification);
+		} catch (error: unknown) {
+			if (!(error instanceof AppError)) throw error;
+			reason = error.message;
+		}
 
 		return res.status(200).json({
 			success,
-			msg: success ? "Notification sent successfully" : "Notification could not be sent — check the destination details.",
+			msg: success ? "Notification sent successfully" : (reason ?? "Notification could not be sent — check the destination details."),
 			details: { service: SERVICE_NAME },
 		});
 	});

server/src/api/controllers/settingsController.ts

@@ -9,6 +9,8 @@ import { IProxiesService } from "@/domain/proxies/proxy.service.js";
 import { IEgressStateService } from "@/domain/egress/egress-state.service.js";
 import { Settings } from "@/domain/app-settings/app-settings.type.js";
 
+const SERVICE_NAME = "SettingsController";
+
 export interface ISettingsController {
 	getAppSettings: RequestHandler;
 	updateAppSettings: RequestHandler;
@@ -119,7 +121,7 @@ class SettingsController implements ISettingsController {
 
 		const html = await this.emailService.buildEmail("testEmailTemplate", context);
 		if (!html) {
-			throw new AppError({ message: "Failed to build email template.", status: 500 });
+			throw new AppError({ message: "Failed to build email template.", status: 500, service: SERVICE_NAME, method: "sendTestEmail" });
 		}
 		let messageId: string;
 		try {
@@ -142,8 +144,11 @@ class SettingsController implements ISettingsController {
 			// Surface the underlying SMTP failure: diagnosing the settings is the whole
 			// point of the test email endpoint.
 			throw new AppError({
-				message: error instanceof Error ? `Failed to send test email. ${error.message}` : "Failed to send test email.",
-				status: 500,
+				message: error instanceof AppError ? error.message : "Failed to send test email.",
+				status: error instanceof AppError ? error.status : 500,
+				service: SERVICE_NAME,
+				method: "sendTestEmail",
+				details: error instanceof AppError ? error.details : undefined,
 			});
 		}
 

server/src/domain/notifications/notification.service.ts

@@ -148,7 +148,17 @@ export class NotificationsService implements INotificationsService {
 
 	testAllNotifications = async (notificationIds: string[]) => {
 		const notifications = await this.notificationsRepository.findNotificationsByIds(notificationIds);
-		const tasks = notifications.map((notification) => this.sendTestNotification(notification));
+		const tasks = notifications.map((notification) =>
+			this.sendTestNotification(notification).catch((error: unknown) => {
+				this.logger.warn({
+					message: `Test notification failed: ${error instanceof Error ? error.message : "Unknown error"}`,
+					service: SERVICE_NAME,
+					method: "testAllNotifications",
+					details: { notificationId: notification.id, type: notification.type },
+				});
+				return false;
+			})
+		);
 		const outcomes = await Promise.all(tasks);
 		const succeeded = outcomes.filter(Boolean).length;
 		const failed = outcomes.length - succeeded;

server/src/domain/notifications/providers/email.ts

@@ -1,6 +1,7 @@
 const SERVICE_NAME = "EmailProvider";
 import type { Notification } from "@/domain/notifications/notification.type.js";
 import { NotificationProvider } from "@/domain/notifications/providers/INotificationProvider.js";
+import { AppError } from "@/utils/AppError.js";
 import { buildTestEmail } from "@/domain/notifications/providers/utils.js";
 import type { NotificationMessage } from "@/domain/notifications/notification.type.js";
 import type { ILogger } from "@/utils/logger.js";
@@ -40,11 +41,13 @@ export class EmailProvider extends NotificationProvider {
 			return true;
 		} catch (error: unknown) {
 			this.logger.warn({
-				message: "Email test alert failed",
+				message: `Email test alert failed: ${error instanceof Error ? error.message : "Unknown error"}`,
 				service: SERVICE_NAME,
 				method: "sendTestAlert",
+				details: error instanceof AppError ? error.details : undefined,
 				stack: error instanceof Error ? error.stack : undefined,
 			});
+			if (error instanceof AppError) throw error;
 			return false;
 		}
 	}
@@ -73,9 +76,10 @@ export class EmailProvider extends NotificationProvider {
 			return true;
 		} catch (error: unknown) {
 			this.logger.warn({
-				message: "Email notification failed",
+				message: `Email notification failed: ${error instanceof Error ? error.message : "Unknown error"}`,
 				service: SERVICE_NAME,
 				method: "sendMessage",
+				details: error instanceof AppError ? error.details : undefined,
 				stack: error instanceof Error ? error.stack : undefined,
 			});
 			return false;

server/src/service/emailService.ts

@@ -12,6 +12,23 @@ const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
 
 const SERVICE_NAME = "EmailService";
+
+const SMTP_FAILURE_REASONS: Record<string, string> = {
+	EAUTH: "rejected the login. Check the user name and password",
+	ENOAUTH: "requires a login. Set the user name and password",
+	EDNS: "could not be resolved. Check the host name",
+	ECONNECTION: "refused the connection. Check the host and port",
+	ESOCKET: "refused the connection. Check the host and port",
+	ETIMEDOUT: "did not respond. Check the port and the secure setting",
+	ETLS: "failed the TLS handshake. Check the secure and TLS settings",
+	EPROTOCOL: "sent an invalid response. Check the port and the secure setting",
+};
+
+const describeSmtpFailure = (error: unknown, host: string, port: number): string => {
+	const code = typeof error === "object" && error !== null && "code" in error ? String((error as { code: unknown }).code) : undefined;
+	const reason = (code !== undefined && SMTP_FAILURE_REASONS[code]) || "failed verification";
+	return `SMTP host ${host}:${port} ${reason}.`;
+};
 type MjmlFn = typeof mjml2html;
 type FileSystem = typeof fs;
 type PathModule = typeof path;
@@ -123,6 +140,15 @@ export class EmailService implements IEmailService {
 			systemEmailRejectUnauthorized,
 		} = config;
 
+		if (!systemEmailHost) {
+			throw new AppError({
+				message: "Email is not configured. Set the system email host in settings.",
+				status: 400,
+				service: SERVICE_NAME,
+				method: "sendEmail",
+			});
+		}
+
 		const emailConfig = {
 			host: systemEmailHost,
 			port: Number(systemEmailPort),
@@ -146,17 +172,20 @@ export class EmailService implements IEmailService {
 		try {
 			await this.transporter.verify();
 		} catch (error: unknown) {
+			const cause = error instanceof Error ? error.message : "Unknown error";
 			this.logger.warn({
-				message: "Email transporter verification failed",
+				message: `Email transporter verification failed: ${cause}`,
 				service: SERVICE_NAME,
 				method: "verifyTransporter",
+				details: { host: systemEmailHost, port: emailConfig.port },
 				stack: error instanceof Error ? error.stack : undefined,
 			});
 			throw new AppError({
-				message: "Email transporter verification failed",
+				message: describeSmtpFailure(error, systemEmailHost, emailConfig.port),
+				status: 502,
 				service: SERVICE_NAME,
 				method: "sendEmail",
-				details: { cause: error instanceof Error ? error.message : "Unknown error" },
+				details: { cause },
 			});
 		}
 
@@ -180,6 +209,7 @@ export class EmailService implements IEmailService {
 			});
 			throw new AppError({
 				message: "Failed to send email",
+				status: 502,
 				service: SERVICE_NAME,
 				method: "sendEmail",
 				details: { cause: error instanceof Error ? error.message : "Unknown error" },

server/src/utils/logger.ts

@@ -60,21 +60,18 @@ class Logger implements ILogger {
 			if (formattedDetails) msg += ` (details: ${formattedDetails})`;
 
 			if (typeof stack === "string") {
-				const stackTrace = stack
-					.split("\n")
-					.slice(1) // Remove first line (error message)
-					.map((line: string) => {
-						const match = line.match(/at\s+(.+?)\s+\((.+?):(\d+):(\d+)\)/);
-						if (match) {
-							return {
-								function: match[1],
-								file: match[2],
-								line: parseInt(match[3] ?? "0", 10),
-								column: parseInt(match[4] ?? "0", 10),
-							};
-						}
-						return line.trim();
-					});
+				const stackTrace = stack.split("\n").map((line: string) => {
+					const match = line.match(/at\s+(.+?)\s+\((.+?):(\d+):(\d+)\)/);
+					if (match) {
+						return {
+							function: match[1],
+							file: match[2],
+							line: parseInt(match[3] ?? "0", 10),
+							column: parseInt(match[4] ?? "0", 10),
+						};
+					}
+					return line.trim();
+				});
 				msg += ` (stack: ${JSON.stringify(stackTrace, null, 2)})`;
 			}
 

server/test/unit/providers/notifications/emailProvider.test.ts

@@ -4,6 +4,7 @@ import { createMockLogger } from "../../../helpers/createMockLogger.ts";
 import { makeNotification, makeMessage } from "../../../helpers/notificationMessage.ts";
 import { testNotificationProviderContract } from "../../../helpers/notificationProviderContract.ts";
 import type { IEmailService } from "../../../../src/service/emailService.ts";
+import { AppError } from "../../../../src/utils/AppError.ts";
 
 // ── Helpers ──────────────────────────────────────────────────────────────────
 
@@ -50,11 +51,29 @@ describe("EmailProvider", () => {
 			expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Failed to build test email content" }));
 		});
 
-		it("returns false when sendEmail rejects", async () => {
+		it("returns false and logs the cause when sendEmail rejects with a plain error", async () => {
 			const { provider, emailService, logger } = createProvider();
 			(emailService.sendEmail as jest.Mock).mockRejectedValue(new Error("SMTP auth failed"));
 			expect(await provider.sendTestAlert(makeNotification())).toBe(false);
-			expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Email test alert failed" }));
+			expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Email test alert failed: SMTP auth failed" }));
+		});
+
+		it("rethrows an AppError from sendEmail so the caller can report the reason", async () => {
+			const { provider, emailService, logger } = createProvider();
+			const error = new AppError({
+				message: "Email is not configured. Set the system email host in settings.",
+				status: 400,
+				details: { cause: "no host" },
+			});
+			(emailService.sendEmail as jest.Mock).mockRejectedValue(error);
+
+			await expect(provider.sendTestAlert(makeNotification())).rejects.toBe(error);
+			expect(logger.warn).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Email test alert failed: Email is not configured. Set the system email host in settings.",
+					details: { cause: "no host" },
+				})
+			);
 		});
 	});
 
@@ -84,11 +103,29 @@ describe("EmailProvider", () => {
 
 		// NotificationsService fans providers out through Promise.all, so a failed email must
 		// resolve to false rather than reject and take the other channels down with it.
-		it("returns false when sendEmail rejects", async () => {
+		it("returns false and logs the cause when sendEmail rejects", async () => {
 			const { provider, emailService, logger } = createProvider();
 			(emailService.sendEmail as jest.Mock).mockRejectedValue(new Error("SMTP auth failed"));
 			expect(await provider.sendMessage(makeNotification() as any, makeMessage())).toBe(false);
-			expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Email notification failed" }));
+			expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Email notification failed: SMTP auth failed" }));
+		});
+
+		it("returns false rather than throwing when sendEmail rejects with an AppError", async () => {
+			const { provider, emailService, logger } = createProvider();
+			(emailService.sendEmail as jest.Mock).mockRejectedValue(
+				new AppError({
+					message: "SMTP verification failed for smtp.example.com:587: 535 Username and Password not accepted",
+					status: 502,
+					details: { cause: "ECONNREFUSED" },
+				})
+			);
+			expect(await provider.sendMessage(makeNotification() as any, makeMessage())).toBe(false);
+			expect(logger.warn).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Email notification failed: SMTP verification failed for smtp.example.com:587: 535 Username and Password not accepted",
+					details: { cause: "ECONNREFUSED" },
+				})
+			);
 		});
 
 		it("builds correct subject for monitor_down", async () => {

server/test/unit/services/emailService.test.ts

@@ -335,17 +335,60 @@ describe("EmailService", () => {
 			const { service, logger } = createService({ nodemailer });
 
 			await expect(service.sendEmail("to@example.com", "Subject", "<p>body</p>", makeTransportConfig())).rejects.toThrow(
-				"Email transporter verification failed"
+				"SMTP host smtp.example.com:587 failed verification."
 			);
 			expect(logger.warn).toHaveBeenCalledWith(
 				expect.objectContaining({
-					message: "Email transporter verification failed",
+					message: "Email transporter verification failed: SMTP auth failed",
 					service: "EmailService",
 					method: "verifyTransporter",
+					details: { host: "smtp.example.com", port: 587 },
 				})
 			);
 		});
 
+		it.each([
+			["EAUTH", "SMTP host smtp.example.com:587 rejected the login. Check the user name and password."],
+			["ETIMEDOUT", "SMTP host smtp.example.com:587 did not respond. Check the port and the secure setting."],
+			["EDNS", "SMTP host smtp.example.com:587 could not be resolved. Check the host name."],
+			["ECONNECTION", "SMTP host smtp.example.com:587 refused the connection. Check the host and port."],
+			["ETLS", "SMTP host smtp.example.com:587 failed the TLS handshake. Check the secure and TLS settings."],
+		])("describes a %s verification failure without the server's reply text", async (code, expected) => {
+			const transporter = createMockTransporter();
+			(transporter.verify as jest.Mock).mockRejectedValue(Object.assign(new Error("535-5.7.8 Username and Password not accepted"), { code }));
+			const nodemailer = createMockNodemailer(transporter);
+			const { service } = createService({ nodemailer });
+
+			await expect(service.sendEmail("to@example.com", "Subject", "<p>body</p>", makeTransportConfig())).rejects.toMatchObject({
+				message: expected,
+				status: 502,
+				details: { cause: "535-5.7.8 Username and Password not accepted" },
+			});
+		});
+
+		it("throws a 400 without building a transport when the host is not configured", async () => {
+			const nodemailer = createMockNodemailer();
+			const { service } = createService({ nodemailer });
+
+			await expect(
+				service.sendEmail("to@example.com", "Subject", "<p>body</p>", makeTransportConfig({ systemEmailHost: undefined }))
+			).rejects.toMatchObject({
+				status: 400,
+				message: "Email is not configured. Set the system email host in settings.",
+			});
+			expect(nodemailer.createTransport).not.toHaveBeenCalled();
+		});
+
+		it("treats an empty host as not configured", async () => {
+			const { service } = createService();
+
+			await expect(service.sendEmail("to@example.com", "Subject", "<p>body</p>", makeTransportConfig({ systemEmailHost: "" }))).rejects.toMatchObject(
+				{
+					status: 400,
+				}
+			);
+		});
+
 		it("reports the underlying cause when verification fails", async () => {
 			const transporter = createMockTransporter();
 			(transporter.verify as jest.Mock).mockRejectedValue(new Error("SMTP auth failed"));
@@ -353,7 +396,7 @@ describe("EmailService", () => {
 			const { service } = createService({ nodemailer });
 
 			await expect(service.sendEmail("to@example.com", "Subject", "<p>body</p>", makeTransportConfig())).rejects.toMatchObject({
-				status: 500,
+				status: 502,
 				details: { cause: "SMTP auth failed" },
 			});
 		});

server/test/unit/services/notificationsService.test.ts

@@ -311,6 +311,23 @@ describe("NotificationsService", () => {
 			const result = await service.testAllNotifications(["notif-1"]);
 			expect(result).toBe(false);
 		});
+
+		it("counts a provider that throws as a failed test and still tests the others", async () => {
+			const { service, notificationsRepository, emailProvider, slackProvider, logger } = createService();
+			emailProvider.sendTestAlert.mockRejectedValue(new Error("Email is not configured. Set the system email host in settings."));
+			(notificationsRepository.findNotificationsByIds as jest.Mock).mockResolvedValue([
+				makeNotification({ type: "email" }),
+				makeNotification({ type: "slack" }),
+			]);
+
+			const result = await service.testAllNotifications(["notif-1", "notif-2"]);
+
+			expect(result).toBe(false);
+			expect(slackProvider.sendTestAlert).toHaveBeenCalled();
+			expect(logger.warn).toHaveBeenCalledWith(
+				expect.objectContaining({ message: "Test notification failed: Email is not configured. Set the system email host in settings." })
+			);
+		});
 	});
 
 	// ── CRUD operations ──────────────────────────────────────────────────────