diff --git a/server/src/api/controllers/notificationController.ts b/server/src/api/controllers/notificationController.ts index 53dd7645c..e94cfa43e 100644 --- a/server/src/api/controllers/notificationController.ts +++ b/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 }, }); }); diff --git a/server/src/api/controllers/settingsController.ts b/server/src/api/controllers/settingsController.ts index cd36467e2..856973c68 100644 --- a/server/src/api/controllers/settingsController.ts +++ b/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, }); } diff --git a/server/src/domain/notifications/notification.service.ts b/server/src/domain/notifications/notification.service.ts index affc0ea8a..b852e8a49 100644 --- a/server/src/domain/notifications/notification.service.ts +++ b/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; diff --git a/server/src/domain/notifications/providers/email.ts b/server/src/domain/notifications/providers/email.ts index 01bbaa6be..6ea7ffbd3 100644 --- a/server/src/domain/notifications/providers/email.ts +++ b/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; diff --git a/server/src/service/emailService.ts b/server/src/service/emailService.ts index 3aca5362c..e257ed44b 100755 --- a/server/src/service/emailService.ts +++ b/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 = { + 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" }, diff --git a/server/src/utils/logger.ts b/server/src/utils/logger.ts index 3cd8570ca..6552a0bc9 100755 --- a/server/src/utils/logger.ts +++ b/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)})`; } diff --git a/server/test/unit/providers/notifications/emailProvider.test.ts b/server/test/unit/providers/notifications/emailProvider.test.ts index 3610523a4..ebac6cc76 100644 --- a/server/test/unit/providers/notifications/emailProvider.test.ts +++ b/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 () => { diff --git a/server/test/unit/services/emailService.test.ts b/server/test/unit/services/emailService.test.ts index d18b83917..484aeb495 100644 --- a/server/test/unit/services/emailService.test.ts +++ b/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", "

body

", 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", "

body

", 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", "

body

", 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", "

body

", 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", "

body

", makeTransportConfig())).rejects.toMatchObject({ - status: 500, + status: 502, details: { cause: "SMTP auth failed" }, }); }); diff --git a/server/test/unit/services/notificationsService.test.ts b/server/test/unit/services/notificationsService.test.ts index adfdb6c90..15ca97e85 100644 --- a/server/test/unit/services/notificationsService.test.ts +++ b/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 ──────────────────────────────────────────────────────