From 9ae21c4c150c30edd328a07796a608669664dc96 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Feb 2026 11:08:47 -0600 Subject: [PATCH] fix(#412): wrap BetterAuth handler in try/catch with error logging Refs #412 Co-Authored-By: Claude Opus 4.6 --- apps/api/src/auth/auth.controller.spec.ts | 72 ++++++++++++++++++++++- apps/api/src/auth/auth.controller.ts | 34 ++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/apps/api/src/auth/auth.controller.spec.ts b/apps/api/src/auth/auth.controller.spec.ts index 4ce8095..eb11b52 100644 --- a/apps/api/src/auth/auth.controller.spec.ts +++ b/apps/api/src/auth/auth.controller.spec.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; +import { HttpException, HttpStatus } from "@nestjs/common"; import type { AuthUser, AuthSession } from "@mosaic/shared"; import type { Request as ExpressRequest, Response as ExpressResponse } from "express"; import { AuthController } from "./auth.controller"; @@ -45,13 +46,82 @@ describe("AuthController", () => { socket: { remoteAddress: "127.0.0.1" }, } as unknown as ExpressRequest; - const mockResponse = {} as unknown as ExpressResponse; + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; await controller.handleAuth(mockRequest, mockResponse); expect(mockAuthService.getNodeHandler).toHaveBeenCalled(); expect(mockNodeHandler).toHaveBeenCalledWith(mockRequest, mockResponse); }); + + it("should throw HttpException with 500 when handler throws before headers sent", async () => { + const handlerError = new Error("BetterAuth internal failure"); + mockNodeHandler.mockRejectedValueOnce(handlerError); + + const mockRequest = { + method: "POST", + url: "/auth/sign-in", + headers: {}, + ip: "192.168.1.10", + socket: { remoteAddress: "192.168.1.10" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + try { + await controller.handleAuth(mockRequest, mockResponse); + // Should not reach here + expect.unreachable("Expected HttpException to be thrown"); + } catch (err) { + expect(err).toBeInstanceOf(HttpException); + expect((err as HttpException).getStatus()).toBe(HttpStatus.INTERNAL_SERVER_ERROR); + expect((err as HttpException).getResponse()).toBe("Internal auth error"); + } + }); + + it("should log warning and not throw when handler throws after headers sent", async () => { + const handlerError = new Error("Stream interrupted"); + mockNodeHandler.mockRejectedValueOnce(handlerError); + + const mockRequest = { + method: "POST", + url: "/auth/sign-up", + headers: {}, + ip: "10.0.0.5", + socket: { remoteAddress: "10.0.0.5" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: true, + } as unknown as ExpressResponse; + + // Should not throw when headers already sent + await expect(controller.handleAuth(mockRequest, mockResponse)).resolves.toBeUndefined(); + }); + + it("should handle non-Error thrown values", async () => { + mockNodeHandler.mockRejectedValueOnce("string error"); + + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: {}, + ip: "127.0.0.1", + socket: { remoteAddress: "127.0.0.1" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + await expect(controller.handleAuth(mockRequest, mockResponse)).rejects.toThrow( + HttpException, + ); + }); }); describe("getSession", () => { diff --git a/apps/api/src/auth/auth.controller.ts b/apps/api/src/auth/auth.controller.ts index bb2e2d7..2d38165 100644 --- a/apps/api/src/auth/auth.controller.ts +++ b/apps/api/src/auth/auth.controller.ts @@ -1,4 +1,15 @@ -import { Controller, All, Req, Res, Get, UseGuards, Request, Logger } from "@nestjs/common"; +import { + Controller, + All, + Req, + Res, + Get, + UseGuards, + Request, + Logger, + HttpException, + HttpStatus, +} from "@nestjs/common"; import { Throttle } from "@nestjs/throttler"; import type { Request as ExpressRequest, Response as ExpressResponse } from "express"; import type { AuthUser, AuthSession } from "@mosaic/shared"; @@ -105,7 +116,26 @@ export class AuthController { this.logger.debug(`Auth catch-all: ${req.method} ${req.url} from ${clientIp}`); const handler = this.authService.getNodeHandler(); - return handler(req, res); + + try { + await handler(req, res); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + const stack = error instanceof Error ? error.stack : undefined; + + this.logger.error( + `BetterAuth handler error: ${req.method} ${req.url} from ${clientIp} - ${message}`, + stack + ); + + if (!res.headersSent) { + throw new HttpException("Internal auth error", HttpStatus.INTERNAL_SERVER_ERROR); + } + + this.logger.warn( + `Cannot send error response for ${req.method} ${req.url} - headers already sent` + ); + } } /**