diff --git a/server/src/api/controllers/settingsController.ts b/server/src/api/controllers/settingsController.ts index 3cb032e93..a8113ed2b 100644 --- a/server/src/api/controllers/settingsController.ts +++ b/server/src/api/controllers/settingsController.ts @@ -93,24 +93,30 @@ class SettingsController implements ISettingsController { if (!html) { throw new AppError({ message: "Failed to build email template.", status: 500 }); } - const messageId = await this.emailService.sendEmail(to, subject, html, { - systemEmailHost, - systemEmailPort, - systemEmailUser, - systemEmailAddress, - systemEmailDisplayName, - systemEmailPassword, - systemEmailConnectionHost, - systemEmailSecure, - systemEmailPool, - systemEmailIgnoreTLS, - systemEmailRequireTLS, - systemEmailRejectUnauthorized, - systemEmailTLSServername, - }); - - if (!messageId) { - throw new AppError({ message: "Failed to send test email.", status: 500 }); + let messageId: string; + try { + messageId = await this.emailService.sendEmail(to, subject, html, { + systemEmailHost, + systemEmailPort, + systemEmailUser, + systemEmailAddress, + systemEmailDisplayName, + systemEmailPassword, + systemEmailConnectionHost, + systemEmailSecure, + systemEmailPool, + systemEmailIgnoreTLS, + systemEmailRequireTLS, + systemEmailRejectUnauthorized, + systemEmailTLSServername, + }); + } catch (error: unknown) { + // 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, + }); } return res.status(200).json({ diff --git a/server/src/domain/invites/invite.service.ts b/server/src/domain/invites/invite.service.ts index 3dcffd9e0..8182f4026 100644 --- a/server/src/domain/invites/invite.service.ts +++ b/server/src/domain/invites/invite.service.ts @@ -96,13 +96,15 @@ export class InviteService implements IInviteService { }); } - const result = await this.emailService.sendEmail(invite.email, "Welcome to Uptime Monitor", html); - if (!result) { + try { + await this.emailService.sendEmail(invite.email, "Welcome to Uptime Monitor", html); + } catch (error: unknown) { throw new AppError({ message: "Failed to send invite e-mail... Please verify your settings.", service: SERVICE_NAME, method: "sendInviteEmail", status: 500, + details: { cause: error instanceof Error ? error.message : "Unknown error" }, }); } }; diff --git a/server/src/domain/notifications/providers/email.ts b/server/src/domain/notifications/providers/email.ts index fa84be66d..2b5cde245 100644 --- a/server/src/domain/notifications/providers/email.ts +++ b/server/src/domain/notifications/providers/email.ts @@ -35,16 +35,18 @@ export class EmailProvider extends NotificationProvider { return false; } - const messageId = await this.emailService.sendEmail(notification.address, subject, html); - if (!messageId) { + try { + await this.emailService.sendEmail(notification.address, subject, html); + return true; + } catch (error: unknown) { this.logger.warn({ message: "Email test alert failed", service: SERVICE_NAME, method: "sendTestAlert", + stack: error instanceof Error ? error.stack : undefined, }); return false; } - return true; } async sendMessage(notification: Notification, message: NotificationMessage): Promise { @@ -64,16 +66,20 @@ export class EmailProvider extends NotificationProvider { return false; } - const messageId = await this.emailService.sendEmail(notification.address, subject, html); - if (!messageId) { + // Providers must resolve to a boolean: NotificationsService fans these out through + // Promise.all, so a rejection here would abort the other channels' alerts too. + try { + await this.emailService.sendEmail(notification.address, subject, html); + return true; + } catch (error: unknown) { this.logger.warn({ message: "Email notification failed", service: SERVICE_NAME, method: "sendMessage", + stack: error instanceof Error ? error.stack : undefined, }); return false; } - return true; } private buildSubject(message: NotificationMessage): string { diff --git a/server/src/domain/users/user.service.ts b/server/src/domain/users/user.service.ts index f042863aa..58a3ec1a3 100644 --- a/server/src/domain/users/user.service.ts +++ b/server/src/domain/users/user.service.ts @@ -30,7 +30,7 @@ export interface IUserService { currentUserEmail: string ): Promise; checkSuperadminExists(): Promise; - requestRecovery(email: string): Promise; + requestRecovery(email: string): Promise; validateRecovery(recoveryToken: string): Promise; resetPassword(password: string, recoveryToken: string): Promise<{ user: User; token: string }>; deleteUser(params: { userId: string; teamId: string; roles: UserRole[] }): Promise; diff --git a/server/src/service/emailService.ts b/server/src/service/emailService.ts index 26b89f84b..3aca5362c 100755 --- a/server/src/service/emailService.ts +++ b/server/src/service/emailService.ts @@ -2,6 +2,7 @@ import { fileURLToPath } from "url"; import { EmailTransportConfig } from "@/domain/app-settings/app-settings.type.js"; import { ISettingsService } from "@/domain/app-settings/app-settings.service.js"; import { ILogger } from "@/utils/logger.js"; +import { AppError } from "@/utils/AppError.js"; import fs from "node:fs"; import path from "node:path"; import nodemailer from "nodemailer"; @@ -20,7 +21,7 @@ type TemplateCompiler = (template: string) => (context: Record) export interface IEmailService { init(): void; buildEmail(template: string, context: Record): Promise; - sendEmail(to: string, subject: string, html: string, transportConfig?: EmailTransportConfig): Promise; + sendEmail(to: string, subject: string, html: string, transportConfig?: EmailTransportConfig): Promise; } export class EmailService implements IEmailService { @@ -151,7 +152,12 @@ export class EmailService implements IEmailService { method: "verifyTransporter", stack: error instanceof Error ? error.stack : undefined, }); - return false; + throw new AppError({ + message: "Email transporter verification failed", + service: SERVICE_NAME, + method: "sendEmail", + details: { cause: error instanceof Error ? error.message : "Unknown error" }, + }); } const trimmedDisplayName = systemEmailDisplayName?.trim(); @@ -172,6 +178,12 @@ export class EmailService implements IEmailService { method: "sendEmail", stack: error instanceof Error ? error.stack : undefined, }); + throw new AppError({ + message: "Failed to send email", + service: SERVICE_NAME, + method: "sendEmail", + details: { cause: error instanceof Error ? error.message : "Unknown error" }, + }); } }; } diff --git a/server/test/unit/providers/notifications/emailProvider.test.ts b/server/test/unit/providers/notifications/emailProvider.test.ts index fecce8ef6..7f3b34d4e 100644 --- a/server/test/unit/providers/notifications/emailProvider.test.ts +++ b/server/test/unit/providers/notifications/emailProvider.test.ts @@ -50,9 +50,9 @@ describe("EmailProvider", () => { expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Failed to build test email content" })); }); - it("returns false when sendEmail returns falsy", async () => { + it("returns false when sendEmail rejects", async () => { const { provider, emailService, logger } = createProvider(); - (emailService.sendEmail as jest.Mock).mockResolvedValue(false); + (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" })); }); @@ -82,9 +82,11 @@ describe("EmailProvider", () => { expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Failed to build email content" })); }); - it("returns false when sendEmail returns falsy", async () => { + // 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 () => { const { provider, emailService, logger } = createProvider(); - (emailService.sendEmail as jest.Mock).mockResolvedValue(undefined); + (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" })); }); diff --git a/server/test/unit/services/emailService.test.ts b/server/test/unit/services/emailService.test.ts index 971c712da..d18b83917 100644 --- a/server/test/unit/services/emailService.test.ts +++ b/server/test/unit/services/emailService.test.ts @@ -326,15 +326,17 @@ describe("EmailService", () => { expect(nodemailer.createTransport).toHaveBeenCalledWith(expect.objectContaining({ name: "localhost" })); }); - it("returns false when transporter verification fails", async () => { + // Callers wrap sendEmail in .catch()/try-catch to react to delivery failures, so a + // transport error has to reject rather than resolve to a falsy value. + it("throws when transporter verification fails", async () => { const transporter = createMockTransporter(); (transporter.verify as jest.Mock).mockRejectedValue(new Error("SMTP auth failed")); const nodemailer = createMockNodemailer(transporter); const { service, logger } = createService({ nodemailer }); - const result = await service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig()); - - expect(result).toBe(false); + await expect(service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig())).rejects.toThrow( + "Email transporter verification failed" + ); expect(logger.warn).toHaveBeenCalledWith( expect.objectContaining({ message: "Email transporter verification failed", @@ -344,27 +346,38 @@ describe("EmailService", () => { ); }); + it("reports the underlying cause when verification fails", async () => { + const transporter = createMockTransporter(); + (transporter.verify as jest.Mock).mockRejectedValue(new Error("SMTP auth failed")); + const nodemailer = createMockNodemailer(transporter); + const { service } = createService({ nodemailer }); + + await expect(service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig())).rejects.toMatchObject({ + status: 500, + details: { cause: "SMTP auth failed" }, + }); + }); + it("logs stack as undefined when verification fails with non-Error", async () => { const transporter = createMockTransporter(); (transporter.verify as jest.Mock).mockRejectedValue("connection refused"); const nodemailer = createMockNodemailer(transporter); const { service, logger } = createService({ nodemailer }); - const result = await service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig()); - - expect(result).toBe(false); + await expect(service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig())).rejects.toThrow(); expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ stack: undefined })); }); - it("returns undefined and logs error when sendMail throws", async () => { + it("throws and logs error when sendMail throws", async () => { const transporter = createMockTransporter(); (transporter.sendMail as jest.Mock).mockRejectedValue(new Error("Recipient rejected")); const nodemailer = createMockNodemailer(transporter); const { service, logger } = createService({ nodemailer }); - const result = await service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig()); - - expect(result).toBeUndefined(); + await expect(service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig())).rejects.toMatchObject({ + message: "Failed to send email", + details: { cause: "Recipient rejected" }, + }); expect(logger.error).toHaveBeenCalledWith( expect.objectContaining({ message: "Recipient rejected", @@ -380,9 +393,9 @@ describe("EmailService", () => { const nodemailer = createMockNodemailer(transporter); const { service, logger } = createService({ nodemailer }); - const result = await service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig()); - - expect(result).toBeUndefined(); + await expect(service.sendEmail("to@example.com", "Subject", "

body

", makeTransportConfig())).rejects.toMatchObject({ + details: { cause: "Unknown error" }, + }); expect(logger.error).toHaveBeenCalledWith( expect.objectContaining({ message: "Unknown error", diff --git a/server/test/unit/services/inviteService.test.ts b/server/test/unit/services/inviteService.test.ts index 59d472539..d60b0830e 100644 --- a/server/test/unit/services/inviteService.test.ts +++ b/server/test/unit/services/inviteService.test.ts @@ -127,11 +127,11 @@ describe("InviteService", () => { ).rejects.toThrow("Failed to build invite e-mail"); }); - it("throws 500 when sendEmail returns falsy", async () => { + it("throws 500 when sendEmail rejects", async () => { const { service } = createService({ emailService: { buildEmail: jest.fn().mockResolvedValue("ok"), - sendEmail: jest.fn().mockResolvedValue(false), + sendEmail: jest.fn().mockRejectedValue(new Error("SMTP auth failed")), }, }); diff --git a/server/test/unit/services/userService.test.ts b/server/test/unit/services/userService.test.ts index 5187a3c60..ee47ffded 100644 --- a/server/test/unit/services/userService.test.ts +++ b/server/test/unit/services/userService.test.ts @@ -196,6 +196,22 @@ describe("UserService", () => { expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ method: "registerUser" })); }); + // The welcome email is fire-and-forget, but its .catch() only runs if sendEmail + // actually rejects; swallowing the transport error made that handler dead code. + it("logs a warning when the welcome email fails to send", async () => { + const { service, logger } = createService({ + emailService: { + buildEmail: jest.fn().mockResolvedValue("Welcome"), + sendEmail: jest.fn().mockRejectedValue(new Error("SMTP auth failed")), + }, + }); + + const result = await service.registerUser({ password: "pass" }, "invite-token", null); + + expect(result.user).toBeDefined(); + expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ method: "registerUser", message: "SMTP auth failed" })); + }); + it("uses default role when invite has no role", async () => { const { service, usersRepository } = createService({ invitesRepository: { @@ -423,6 +439,19 @@ describe("UserService", () => { await expect(service.requestRecovery("test@example.com")).rejects.toThrow("Failed to build password reset email HTML"); }); + + // Previously the send failure was swallowed and the endpoint still reported + // "Password recovery email sent successfully" while the user got nothing. + it("propagates the failure when the recovery email cannot be sent", async () => { + const { service } = createService({ + emailService: { + buildEmail: jest.fn().mockResolvedValue("Reset"), + sendEmail: jest.fn().mockRejectedValue(new Error("SMTP auth failed")), + }, + }); + + await expect(service.requestRecovery("test@example.com")).rejects.toThrow("SMTP auth failed"); + }); }); // ── validateRecovery ────────────────────────────────────────────────────