fix: distinguish account not-found (404) from system errors (500) in discovery

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<FastifyInstance> => {
+  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();
+  });
+});

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

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<typeof vi.fn>;
+  findOne?: ReturnType<typeof vi.fn>;
+}) => {
+  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);
+    });
+  });
+});

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;

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;

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;
       }
     },
   );