diff --git a/packages/fastify/__test__/accountDiscoveryPlugin.test.ts b/packages/fastify/__test__/accountDiscoveryPlugin.test.ts new file mode 100644 index 0000000..bb7cd52 --- /dev/null +++ b/packages/fastify/__test__/accountDiscoveryPlugin.test.ts @@ -0,0 +1,160 @@ +/* eslint-disable unicorn/no-null */ +import type { ApiConfig } from "@prefabs.tech/fastify-config"; + +import errorHandlerPlugin from "@prefabs.tech/fastify-error-handler"; +import Fastify, { FastifyInstance } from "fastify"; +import { Writable } from "node:stream"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { Account } from "../src/types"; + +import AccountNotFoundError from "../src/lib/accountNotFoundError"; +import discoverAccount from "../src/lib/discoverAccount"; +import accountDiscoveryPlugin from "../src/plugins/accountDiscoveryPlugin"; + +// Mock discovery so we can drive each outcome (found / not-found / DB failure) +// without a database. A factory keeps the heavy real import graph out of the +// test (it transitively pulls in an ESM-incompatible dependency). +vi.mock("../src/lib/discoverAccount", () => ({ default: vi.fn() })); + +const mockedDiscoverAccount = vi.mocked(discoverAccount); + +const account = { + database: null, + id: "acc_1", + slug: "acme", +} as unknown as Account; + +const testConfig = { + saas: { + apps: [{ domain: "admin.example.test", name: "admin", subdomain: "admin" }], + rootDomain: "example.test", + subdomains: "required", + }, +} as unknown as ApiConfig; + +// Captured pino log lines. Levels: debug=20, info=30, warn=40, error=50. +let logLines: Array<{ level: number }> = []; + +const buildApp = async (): Promise => { + const stream = new Writable({ + write(chunk, _encoding, callback) { + logLines.push(JSON.parse(chunk.toString())); + + callback(); + }, + }); + + const app = Fastify({ logger: { level: "debug", stream } }); + + // Provide the request decorations the hook reads. `slonik` is only forwarded + // to the (mocked) discoverAccount, so its value is irrelevant here. + app.addHook("onRequest", async (request) => { + request.config = testConfig; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (request as any).slonik = {}; + }); + + await app.register(errorHandlerPlugin, {}); + await app.register(accountDiscoveryPlugin); + + app.get("/resource", async (request) => ({ + account: request.account?.id ?? null, + })); + + await app.ready(); + + return app; +}; + +const maxLevel = () => Math.max(0, ...logLines.map((line) => line.level)); + +describe("accountDiscoveryPlugin", () => { + beforeEach(() => { + vi.clearAllMocks(); + logLines = []; + }); + + it("decorates the request and responds 200 when an account is found", async () => { + mockedDiscoverAccount.mockResolvedValue(account); + + const app = await buildApp(); + + const response = await app.inject({ + headers: { host: "acme.example.test" }, + method: "GET", + url: "/resource", + }); + + expect(response.statusCode).toBe(200); + expect(response.json()).toEqual({ account: "acc_1" }); + }); + + it("genuine not-found → 404 with the legacy body and no error/warn log", async () => { + mockedDiscoverAccount.mockRejectedValue(new AccountNotFoundError()); + + const app = await buildApp(); + + const response = await app.inject({ + headers: { host: "ghost.example.test" }, + method: "GET", + url: "/resource", + }); + + expect(response.statusCode).toBe(404); + expect(response.json()).toEqual({ + error: { message: "Account not found" }, + }); + // Operational, expected — never logged loud enough to alert. + expect(maxLevel()).toBeLessThan(40); + }); + + it("unexpected DB failure → 500 and an error-level log (not a 404)", async () => { + mockedDiscoverAccount.mockRejectedValue( + new Error("connection terminated unexpectedly"), + ); + + const app = await buildApp(); + + const response = await app.inject({ + headers: { host: "acme.example.test" }, + method: "GET", + url: "/resource", + }); + + expect(response.statusCode).toBe(500); + expect(response.json()).not.toEqual({ + error: { message: "Account not found" }, + }); + // The central error handler must log system failures at error level. + expect(maxLevel()).toBe(50); + }); + + it("does not 404 when discovery returns null (excluded route)", async () => { + mockedDiscoverAccount.mockResolvedValue(null); + + const app = await buildApp(); + + const response = await app.inject({ + headers: { host: "app.example.test" }, + method: "GET", + url: "/resource", + }); + + expect(response.statusCode).toBe(200); + expect(response.json()).toEqual({ account: null }); + }); + + it("skips discovery entirely for configured app domains", async () => { + const app = await buildApp(); + + const response = await app.inject({ + headers: { host: "admin.example.test" }, + method: "GET", + url: "/resource", + }); + + expect(response.statusCode).toBe(200); + expect(mockedDiscoverAccount).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/fastify/src/index.ts b/packages/fastify/src/index.ts index 838d2d8..6a84ea6 100644 --- a/packages/fastify/src/index.ts +++ b/packages/fastify/src/index.ts @@ -37,6 +37,7 @@ declare module "@prefabs.tech/fastify-config" { export * from "./constants"; +export { default as AccountNotFoundError } from "./lib/accountNotFoundError"; export { default as accountInvitationRoutes } from "./model/accountInvitations/controller"; export { default as AccountInvitationService } from "./model/accountInvitations/service"; export { default as accountRoutes } from "./model/accounts/controller"; diff --git a/packages/fastify/src/lib/__test__/discoverAccount.test.ts b/packages/fastify/src/lib/__test__/discoverAccount.test.ts new file mode 100644 index 0000000..0ae09c0 --- /dev/null +++ b/packages/fastify/src/lib/__test__/discoverAccount.test.ts @@ -0,0 +1,161 @@ +/* eslint-disable unicorn/no-null, unicorn/no-useless-undefined */ +import type { ApiConfig } from "@prefabs.tech/fastify-config"; +import type { Database } from "@prefabs.tech/fastify-slonik"; + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { Account } from "../../types"; + +import AccountService from "../../model/accounts/service"; +import AccountNotFoundError from "../accountNotFoundError"; +import discoverAccount from "../discoverAccount"; +import createConfig from "./helpers/createConfig"; + +vi.mock("../../model/accounts/service", () => ({ default: vi.fn() })); + +const database = {} as unknown as Database; + +const account = { id: "acc_1", slug: "acme" } as unknown as Account; + +const mockService = (overrides: { + findByHostname?: ReturnType; + findOne?: ReturnType; +}) => { + const findByHostname = overrides.findByHostname ?? vi.fn(); + const findOne = overrides.findOne ?? vi.fn(); + + vi.mocked(AccountService).mockImplementation( + () => ({ findByHostname, findOne }) as unknown as AccountService, + ); + + return { findByHostname, findOne }; +}; + +// subdomains !== "disabled" keeps mainApp.skipHostnameCheck false, so a +// non-mainApp hostname is resolved through the hostname branch. +const hostnameConfig = (): ApiConfig => + createConfig({ rootDomain: "example.test", subdomains: "required" }); + +// subdomains === "disabled" forces mainApp.skipHostnameCheck true, so +// resolution goes through the account-id header branch. +const mainAppConfig = (): ApiConfig => + createConfig({ rootDomain: "example.test", subdomains: "disabled" }); + +describe("discoverAccount", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("hostname branch", () => { + it("returns the account when the hostname matches", async () => { + const { findByHostname } = mockService({ + findByHostname: vi.fn().mockResolvedValue(account), + }); + + await expect( + discoverAccount( + hostnameConfig(), + database, + "acme.example.test", + undefined, + ), + ).resolves.toBe(account); + + expect(findByHostname).toHaveBeenCalledWith("acme.example.test"); + }); + + it("throws AccountNotFoundError when no account matches the hostname", async () => { + mockService({ findByHostname: vi.fn().mockResolvedValue(null) }); + + await expect( + discoverAccount( + hostnameConfig(), + database, + "ghost.example.test", + undefined, + ), + ).rejects.toBeInstanceOf(AccountNotFoundError); + }); + + it("propagates the original error when the query fails (DB outage)", async () => { + const dbError = new Error("connection terminated unexpectedly"); + + mockService({ findByHostname: vi.fn().mockRejectedValue(dbError) }); + + const promise = discoverAccount( + hostnameConfig(), + database, + "acme.example.test", + undefined, + ); + + await expect(promise).rejects.toBe(dbError); + await expect(promise).rejects.not.toBeInstanceOf(AccountNotFoundError); + }); + }); + + describe("account-id header branch", () => { + it("returns null for excluded routes without touching the database", async () => { + const { findOne } = mockService({}); + + await expect( + discoverAccount( + mainAppConfig(), + database, + "app.example.test", + "acc_1", + true, + ), + ).resolves.toBeNull(); + + expect(findOne).not.toHaveBeenCalled(); + }); + + it("returns the account when the id resolves", async () => { + mockService({ findOne: vi.fn().mockResolvedValue(account) }); + + await expect( + discoverAccount(mainAppConfig(), database, "app.example.test", "acc_1"), + ).resolves.toBe(account); + }); + + it("throws AccountNotFoundError when the id does not resolve", async () => { + mockService({ findOne: vi.fn().mockResolvedValue(null) }); + + await expect( + discoverAccount(mainAppConfig(), database, "app.example.test", "acc_1"), + ).rejects.toBeInstanceOf(AccountNotFoundError); + }); + + it("throws AccountNotFoundError when no account id is supplied", async () => { + const { findOne } = mockService({}); + + await expect( + discoverAccount( + mainAppConfig(), + database, + "app.example.test", + undefined, + ), + ).rejects.toBeInstanceOf(AccountNotFoundError); + + expect(findOne).not.toHaveBeenCalled(); + }); + + it("propagates the original error when the query fails (DB outage)", async () => { + const dbError = new Error("statement timeout"); + + mockService({ findOne: vi.fn().mockRejectedValue(dbError) }); + + const promise = discoverAccount( + mainAppConfig(), + database, + "app.example.test", + "acc_1", + ); + + await expect(promise).rejects.toBe(dbError); + await expect(promise).rejects.not.toBeInstanceOf(AccountNotFoundError); + }); + }); +}); diff --git a/packages/fastify/src/lib/accountNotFoundError.ts b/packages/fastify/src/lib/accountNotFoundError.ts new file mode 100644 index 0000000..164cdff --- /dev/null +++ b/packages/fastify/src/lib/accountNotFoundError.ts @@ -0,0 +1,16 @@ +/** + * Thrown by `discoverAccount` when no account matches the request. Mapped + * to a 404 and not logged as an error, while other errors propagate as 500s. + */ +class AccountNotFoundError extends Error { + constructor(message = "Account not found") { + super(message); + + this.name = "AccountNotFoundError"; + + // Preserve `instanceof` across transpilation targets that downlevel classes. + Object.setPrototypeOf(this, new.target.prototype); + } +} + +export default AccountNotFoundError; diff --git a/packages/fastify/src/lib/discoverAccount.ts b/packages/fastify/src/lib/discoverAccount.ts index 618d844..24dd20c 100644 --- a/packages/fastify/src/lib/discoverAccount.ts +++ b/packages/fastify/src/lib/discoverAccount.ts @@ -6,6 +6,7 @@ import type { Account } from "../types"; import getSaasConfig from "../config"; import AccountService from "../model/accounts/service"; +import AccountNotFoundError from "./accountNotFoundError"; const getAccountByHostname = async ( config: ApiConfig, @@ -66,7 +67,8 @@ const discoverAccount = async ( return account; } - throw new Error("Account not found"); + // Signal not-found with a typed error to distinguish it from unexpected failures. + throw new AccountNotFoundError(); }; export default discoverAccount; diff --git a/packages/fastify/src/plugins/accountDiscoveryPlugin.ts b/packages/fastify/src/plugins/accountDiscoveryPlugin.ts index bd4730b..22bd5bf 100644 --- a/packages/fastify/src/plugins/accountDiscoveryPlugin.ts +++ b/packages/fastify/src/plugins/accountDiscoveryPlugin.ts @@ -4,6 +4,7 @@ import FastifyPlugin from "fastify-plugin"; import getSaasConfig from "../config"; import { ACCOUNT_HEADER_NAME } from "../constants"; +import AccountNotFoundError from "../lib/accountNotFoundError"; import discoverAccount from "../lib/discoverAccount"; import getHost from "../lib/getHost"; @@ -60,11 +61,17 @@ const plugin = async (fastify: FastifyInstance) => { } } } catch (error) { - fastify.log.error(error); + // Expected not-found (e.g. unknown subdomain): return 404 without error logging. + if (error instanceof AccountNotFoundError) { + log.debug({ hostname }, "Account discovery: no account found"); - return reply - .status(404) - .send({ error: { message: "Account not found" } }); + return reply + .status(404) + .send({ error: { message: "Account not found" } }); + } + + // Rethrow unexpected system failures to let the central error handler report them as 500s. + throw error; } }, );