Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fuzzy-pandas-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@croco/protocols-trpc": patch
"@croco/problems-core": patch
---

Reject duplicate tRPC domain and procedure registrations with diagnostics for both source routes.
43 changes: 34 additions & 9 deletions docs/problem-code-registry.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"version": "croco.problem-code-registry.v1",
"problemCount": 581,
"problemCount": 582,
"problems": [
{
"code": "ACCESS_DENIED",
Expand Down Expand Up @@ -11762,6 +11762,36 @@
}
]
},
{
"code": "protocols-trpc/duplicate-procedure-name",
"category": "InternalServerError",
"status": 500,
"title": "Internal Server Error",
"cookbookPath": "/reference/problem-recovery-cookbook/#protocols-trpc-duplicate-procedure-name",
"recovery": {
"cause": "Two controller routes resolve to the same tRPC domain and procedure name.",
"userAction": "Use an application build where every tRPC procedure has a unique domain and controller method name combination.",
"operatorAction": "Inspect the duplicate-procedure diagnostic for the existing and conflicting controller routes and their decorator source locations, then change one domain or controller method name.",
"retryability": "not-retryable",
"redactionPolicy": "operator-only",
"telemetry": {
"eventName": "croco.problem.error",
"severity": "error",
"attributes": ["problem.code", "problem.category", "problem.status"]
}
},
"lifecycle": {
"status": "active"
},
"sources": [
{
"file": "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
"line": 93,
"column": 3,
"kind": "problem-class"
}
]
},
{
"code": "protocols-trpc/provider-container-required",
"category": "InternalServerError",
Expand All @@ -11786,7 +11816,7 @@
"sources": [
{
"file": "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
"line": 70,
"line": 77,
"column": 3,
"kind": "problem-class"
}
Expand Down Expand Up @@ -11876,7 +11906,7 @@
"sources": [
{
"file": "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
"line": 57,
"line": 64,
"column": 3,
"kind": "problem-class"
}
Expand Down Expand Up @@ -14301,12 +14331,7 @@
}
},
"lifecycle": {
"status": "deprecated",
"deprecation": {
"reason": "The telemetry SDK no longer exposes non-executable metrics or logs configuration.",
"migrationNote": "Stop branching on TELEMETRY_SIGNAL_UNSUPPORTED and remove metrics or logs options from TelemetryConfig consumers.",
"noReplacementReason": "The trace-only runtime has no unsupported signal configuration path to replace this code."
}
"status": "active"
},
"sources": [
{
Expand Down
1,199 changes: 606 additions & 593 deletions packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md

Large diffs are not rendered by default.

48 changes: 36 additions & 12 deletions packages/problems-core/src/generated/problem-code-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ProblemCodeRegistry } from "../libs/ProblemRegistry";

export const CROCO_PROBLEM_CODE_REGISTRY = {
version: "croco.problem-code-registry.v1",
problemCount: 581,
problemCount: 582,
problems: [
{
code: "ACCESS_DENIED",
Expand Down Expand Up @@ -12231,6 +12231,38 @@ export const CROCO_PROBLEM_CODE_REGISTRY = {
},
],
},
{
code: "protocols-trpc/duplicate-procedure-name",
category: "InternalServerError",
status: 500,
title: "Internal Server Error",
cookbookPath: "/reference/problem-recovery-cookbook/#protocols-trpc-duplicate-procedure-name",
recovery: {
cause: "Two controller routes resolve to the same tRPC domain and procedure name.",
userAction:
"Use an application build where every tRPC procedure has a unique domain and controller method name combination.",
operatorAction:
"Inspect the duplicate-procedure diagnostic for the existing and conflicting controller routes and their decorator source locations, then change one domain or controller method name.",
retryability: "not-retryable",
redactionPolicy: "operator-only",
telemetry: {
eventName: "croco.problem.error",
severity: "error",
attributes: ["problem.code", "problem.category", "problem.status"],
},
},
lifecycle: {
status: "active",
},
sources: [
{
file: "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
line: 93,
column: 3,
kind: "problem-class",
},
],
},
{
code: "protocols-trpc/provider-container-required",
category: "InternalServerError",
Expand Down Expand Up @@ -12258,7 +12290,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = {
sources: [
{
file: "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
line: 70,
line: 77,
column: 3,
kind: "problem-class",
},
Expand Down Expand Up @@ -12356,7 +12388,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = {
sources: [
{
file: "packages/protocols-trpc/src/libs/createTrpcRouter.ts",
line: 57,
line: 64,
column: 3,
kind: "problem-class",
},
Expand Down Expand Up @@ -14903,15 +14935,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = {
},
},
lifecycle: {
status: "deprecated",
deprecation: {
reason:
"The telemetry SDK no longer exposes non-executable metrics or logs configuration.",
migrationNote:
"Stop branching on TELEMETRY_SIGNAL_UNSUPPORTED and remove metrics or logs options from TelemetryConfig consumers.",
noReplacementReason:
"The trace-only runtime has no unsupported signal configuration path to replace this code.",
},
status: "active",
},
sources: [
{
Expand Down
92 changes: 91 additions & 1 deletion packages/protocols-trpc/src/libs/createTrpcRouter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Container, Context } from "@croco/framework-context";
import type { RequestContext } from "@croco/framework-context";
import { Problem, ProblemCategory } from "@croco/problems-core";
import type { RouteIR } from "@croco/protocols-core";
import type { RouteContractSourceLocation, RouteIR } from "@croco/protocols-core";
import { extractRouteIR } from "@croco/protocols-core";
import { getFilters, getGuards, getInterceptors, type Constructor } from "@croco/protocols-rest";
import {
Expand All @@ -17,6 +17,13 @@ import { TrpcExecutionPipeline, type TrpcPipelineConfig } from "./TrpcExecutionP

type ControllerConstructor = (new () => object) & Function;
type RouteHandler = (...args: unknown[]) => unknown;
type TrpcRouteDiagnostic = {
readonly controllerName: string;
readonly methodName: string;
readonly httpMethod: string;
readonly path: string;
readonly sourceLocation?: RouteContractSourceLocation;
};

export type TrpcRouterOptions = {
readonly container?: {
Expand Down Expand Up @@ -82,6 +89,27 @@ class TrpcProviderContainerProblem extends Problem {
}
}

class TrpcDuplicateProcedureProblem extends Problem {
readonly code = "protocols-trpc/duplicate-procedure-name";
readonly category = ProblemCategory.InternalServerError;

constructor(
domain: string,
procedureName: string,
existingRoute: TrpcRouteDiagnostic,
conflictingRoute: TrpcRouteDiagnostic,
) {
super(
undefined,
undefined,
formatDuplicateProcedureDetail(domain, procedureName, existingRoute, conflictingRoute),
{
extensions: { domain, procedureName, existingRoute, conflictingRoute },
},
);
}
}

/**
* Creates a tRPC router whose procedures run Croco guards before input parsing, then interceptors around handlers.
*
Expand All @@ -94,12 +122,28 @@ export function createTrpcRouter(
options: TrpcRouterOptions = {},
): AnyRouter {
const domains: Record<string, TRPCCreateRouterOptions> = {};
const procedureSources = new Map<string, Map<string, TrpcRouteDiagnostic>>();

for (const controller of controllers) {
const controllerCtor = controller as ControllerConstructor;

for (const route of extractRouteIR(controllerCtor)) {
const domain = getDomainName(route);
const routeDiagnostic = toRouteDiagnostic(route);
const domainSources = procedureSources.get(domain) ?? new Map();
const existingRoute = domainSources.get(route.methodName);

if (existingRoute) {
throw new TrpcDuplicateProcedureProblem(
domain,
route.methodName,
existingRoute,
routeDiagnostic,
);
}

domainSources.set(route.methodName, routeDiagnostic);
procedureSources.set(domain, domainSources);
domains[domain] ??= {};
domains[domain][route.methodName] = createProcedure(controllerCtor, route, options);
}
Expand Down Expand Up @@ -200,6 +244,52 @@ function getDomainName(route: RouteIR): string {
return domain.charAt(0).toLowerCase() + domain.slice(1);
}

function toRouteDiagnostic(route: RouteIR): TrpcRouteDiagnostic {
return {
controllerName: route.controllerName,
methodName: route.methodName,
httpMethod: route.httpMethod,
path: route.path,
...(route.sourceLocation ? { sourceLocation: route.sourceLocation } : {}),
};
}

function formatDuplicateProcedureDetail(
domain: string,
procedureName: string,
existingRoute: TrpcRouteDiagnostic,
conflictingRoute: TrpcRouteDiagnostic,
): string {
return [
`Duplicate tRPC procedure detected for ${domain}.${procedureName}.`,
`Existing route: ${formatRouteDiagnostic(existingRoute)}.`,
`Conflicting route: ${formatRouteDiagnostic(conflictingRoute)}.`,
"Recovery: give one route a unique tRPC domain or controller method name before constructing the router.",
].join(" ");
}

function formatRouteDiagnostic(route: TrpcRouteDiagnostic): string {
const routeLabel = `${route.controllerName}.${route.methodName} (${route.httpMethod} ${route.path})`;
const sourceLocation = formatSourceLocation(route.sourceLocation);

return sourceLocation
? `${routeLabel} at ${sourceLocation}`
: `${routeLabel} (route decorator source unavailable)`;
}

function formatSourceLocation(
sourceLocation: RouteContractSourceLocation | undefined,
): string | null {
if (!sourceLocation) {
return null;
}

const line = sourceLocation.line === undefined ? "" : `:${sourceLocation.line}`;
const column = sourceLocation.column === undefined ? "" : `:${sourceLocation.column}`;

return `${sourceLocation.path}${line}${column}`;
}

function isRouteHandler(value: unknown): value is RouteHandler {
return typeof value === "function";
}
Expand Down
75 changes: 75 additions & 0 deletions packages/protocols-trpc/src/tests/createTrpcRouter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,71 @@ describe("createTrpcRouter", () => {
expect(router._def.record).toHaveProperty("order");
});

it("should reject duplicate domain and procedure names with both source routes", () => {
class PublicUserController {
listUsers(): string[] {
return ["public-user"];
}
}

class AdminUserController {
listUsers(): string[] {
return ["admin-user"];
}
}

mocked.extractRouteIR = (controller) => [
{
controllerName: controller.name,
methodName: "listUsers",
httpMethod: controller === PublicUserController ? "GET" : "POST",
path: controller === PublicUserController ? "/public/users" : "/admin/users",
sourceLocation:
controller === PublicUserController
? { path: "src/PublicUserController.ts", line: 12, column: 3 }
: { path: "src/AdminUserController.ts", line: 24, column: 5 },
routeContract: null,
params: [],
inputSchema: null,
inputSchemas: { body: null, path: null, query: null, headers: null },
outputSchema: null,
domain: "user",
},
];

const error = captureThrownValue(() =>
createTrpcRouter([PublicUserController, AdminUserController]),
) as { readonly toJSON: () => Record<string, unknown> };
const problem = error.toJSON();

expect(problem).toMatchObject({
code: "protocols-trpc/duplicate-procedure-name",
status: 500,
domain: "user",
procedureName: "listUsers",
existingRoute: {
controllerName: "PublicUserController",
methodName: "listUsers",
httpMethod: "GET",
path: "/public/users",
sourceLocation: { path: "src/PublicUserController.ts", line: 12, column: 3 },
},
conflictingRoute: {
controllerName: "AdminUserController",
methodName: "listUsers",
httpMethod: "POST",
path: "/admin/users",
sourceLocation: { path: "src/AdminUserController.ts", line: 24, column: 5 },
},
});
expect(problem.detail).toContain(
"Existing route: PublicUserController.listUsers (GET /public/users) at src/PublicUserController.ts:12:3.",
);
expect(problem.detail).toContain(
"Conflicting route: AdminUserController.listUsers (POST /admin/users) at src/AdminUserController.ts:24:5.",
);
});

it("should expose a coded error when route metadata points at a non-callable handler", async () => {
class UserController {
readonly listUsers = "not callable";
Expand Down Expand Up @@ -697,6 +762,16 @@ async function closeServer(server: ReturnType<typeof createHTTPServer>): Promise
});
}

function captureThrownValue(callback: () => unknown): unknown {
try {
callback();
} catch (error) {
return error;
}

expect.fail("Expected callback to throw.");
}

function extractTestRouteIR(controllerCtor: Function): RouteIR[] {
const controllerMeta = Reflect.getMetadata(REST_CONTROLLER_KEY, controllerCtor) as
| ControllerMetadata
Expand Down
Loading
Loading