Skip to content

Commit 084686e

Browse files
Vu-Johnclaude
andauthored
[logging] inspector: add deprecation guards, LOGGING.md, and ESLint enforcement (#1934)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 740089b commit 084686e

7 files changed

Lines changed: 227 additions & 310 deletions

File tree

mcpjam-inspector/eslint.config.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// @ts-check
2+
import tsParser from "@typescript-eslint/parser";
3+
4+
export default [
5+
{
6+
files: ["server/routes/web/**/*.ts"],
7+
languageOptions: {
8+
parser: tsParser,
9+
},
10+
rules: {
11+
"no-restricted-syntax": [
12+
"warn",
13+
{
14+
selector:
15+
"CallExpression[callee.type='MemberExpression'][callee.object.name='logger'][callee.property.name=/^(warn|error|info)$/]",
16+
message:
17+
"Use logger.event() for production diagnostics in server/routes/web/. " +
18+
"Free-form logger.warn/error/info calls are not queryable in Axiom. " +
19+
"See server/utils/LOGGING.md for the typed event catalog.",
20+
},
21+
],
22+
},
23+
},
24+
];

mcpjam-inspector/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@
191191
"@types/node": "^24",
192192
"@types/react": "^19",
193193
"@types/react-dom": "^19",
194+
"@typescript-eslint/parser": "^8.53.1",
194195
"@vitejs/plugin-react": "^4.3.4",
195196
"@vitest/coverage-v8": "^3.2.4",
196197
"autoprefixer": "^10.4.21",

mcpjam-inspector/server/routes/web/__tests__/helpers/test-app.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Hono } from "hono";
22
import { vi } from "vitest";
33
import webRoutes from "../../index.js";
4+
import { requestLogContextMiddleware } from "../../../../middleware/request-log-context.js";
45

56
/**
67
* Minimal mock for hosted-mode MCPClientManager.
@@ -15,6 +16,11 @@ export function createWebTestApp(options?: {
1516
}) {
1617
const app = new Hono();
1718

19+
// Mirror production wiring: requestLogContextMiddleware must run before any
20+
// route that calls getRequestLogger (which throws when context is missing).
21+
// Mounted at /api/* in production via server/app.ts.
22+
app.use("/api/*", requestLogContextMiddleware);
23+
1824
// Inject a no-op mcpClientManager (web routes create their own)
1925
app.use("*", async (c, next) => {
2026
(c as any).mcpClientManager = {};
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Server Logging Conventions
2+
3+
## Quick rule
4+
5+
- **New production diagnostics in `server/routes/`** → use `logger.event()` via `getRequestLogger`
6+
- **Ad-hoc debugging anywhere**`logger.debug()` is fine
7+
- **Legacy code outside converted routes**`logger.warn/error` remain; migrate opportunistically
8+
9+
---
10+
11+
## Why typed events?
12+
13+
`logger.warn("something failed", { ... })` produces unstructured Axiom rows that are hard to query.
14+
`logger.event("tunnel.created", ...)` produces a row you can filter by `event`, `workspaceId`, `orgId`, `route`, etc. without grepping prose.
15+
16+
---
17+
18+
## How to emit a typed event from a route handler
19+
20+
```ts
21+
import { getRequestLogger } from "../../utils/request-logger.js";
22+
import { classifyError } from "../../utils/error-classify.js";
23+
24+
// inside a Hono handler that has `c: Context`
25+
getRequestLogger(c, "routes.web.tools").event("mcp.tool.execution.failed", {
26+
toolName: body.toolName,
27+
serverId: body.serverId,
28+
errorCode: classifyError(error),
29+
});
30+
```
31+
32+
The request context (`workspaceId`, `orgId`, `workspaceRole`, etc.) is automatically
33+
picked up from `c.var.requestLogContext`, which is populated by:
34+
1. `requestLogContextMiddleware` at request start (sets `requestId`, `route`, `method`)
35+
2. `authorizeServer` / `createAuthorizedManager` after Convex auth succeeds (sets workspace + user fields)
36+
37+
---
38+
39+
## Event catalog
40+
41+
| Event | Emitted from | Key payload fields |
42+
|---|---|---|
43+
| `http.request.completed` | middleware | `statusCode` |
44+
| `http.request.failed` | middleware | `statusCode`, `errorCode` |
45+
| `mcp.oauth.proxy.failed` | `routes/mcp/oauth.ts`, `routes/web/oauth.ts` | `targetUrlHost`, `oauthPhase`, `errorCode`, `statusCode?` |
46+
| `mcp.tool.execution.failed` | `routes/web/tools.ts` | `toolName`, `serverId?`, `errorCode` |
47+
| `tunnel.created` | `routes/mcp/tunnels.ts` | `tunnelKind`, `tunnelDomain`, `existed`, `credentialIdPresent?` |
48+
| `tunnel.creation_failed` | `routes/mcp/tunnels.ts` | `tunnelKind`, `errorCode` |
49+
| `tunnel.record_failed` | `routes/mcp/tunnels.ts` | `tunnelKind`, `tunnelDomain?`, `errorCode` |
50+
| `chat.session.persist.failed` | `utils/chat-ingestion.ts` | `failureKind`, `statusCode?`, `sourceType?` |
51+
| `widget.resource.served` | `routes/apps/mcp-apps/index.ts` | `widgetType`, `resourceUri`, `cspMode`, `mimeTypeValid?` |
52+
| `widget.resource.failed` | `routes/apps/mcp-apps/index.ts` | `widgetType`, `resourceUri?`, `errorCode` |
53+
| `mcp.connection.closed_with_pending_requests` | `index.ts` (system event) | `errorCode` |
54+
55+
All events live in `server/utils/log-events.ts`. Add new events there before emitting them.
56+
57+
---
58+
59+
## Adding a new event
60+
61+
1. Add the event name and payload type to `RequestEventMap` (or `SystemEventMap`) in `log-events.ts`.
62+
2. Emit it with `getRequestLogger(c, "routes.your.component").event("your.event.name", payload)`.
63+
3. Add a row to the catalog table above.
64+
65+
---
66+
67+
## Scrubbing
68+
69+
All payloads are scrubbed by `scrubLogPayload` before reaching Axiom. Forbidden key substrings
70+
(token, secret, email, authorization, cookie, apikey, stripe\*, pkce\*) are replaced with `"[redacted]"`.
71+
String values are scanned for Bearer tokens, JWTs, email addresses, and `sk-` keys.
72+
73+
Never put raw error messages containing user input directly into event payloads without first
74+
verifying they don't carry secrets.
75+
76+
---
77+
78+
## ESLint enforcement
79+
80+
`eslint.config.js` warns on new `logger.warn|error|info` calls in `server/routes/web/`.
81+
Run `npx eslint server/routes/web/` to check before committing route changes.

mcpjam-inspector/server/utils/__tests__/chat-ingestion.test.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import type { Context } from "hono";
33
import { persistChatSessionToConvex } from "../chat-ingestion";
4+
import type { RequestLogContext } from "../log-events";
45

56
const mockLogger = vi.hoisted(() => ({
67
warn: vi.fn(),
@@ -11,6 +12,31 @@ vi.mock("../logger", () => ({
1112
logger: mockLogger,
1213
}));
1314

15+
// Mirror the production envelope populated by requestLogContextMiddleware.
16+
// Without this, getRequestLogger throws — the strict-throw was added in the
17+
// typed-event foundation to surface wiring bugs, so test fixtures must reflect
18+
// real production wiring.
19+
function makeTestContext(): Context {
20+
const baseContext: RequestLogContext = {
21+
event: "http.request.completed",
22+
timestamp: "2024-01-01T00:00:00.000Z",
23+
environment: "test",
24+
release: null,
25+
component: "http",
26+
requestId: "test-req",
27+
route: "/api/web/test",
28+
method: "POST",
29+
authType: "unknown",
30+
};
31+
const vars: Record<string, unknown> = { requestLogContext: baseContext };
32+
return {
33+
var: new Proxy(vars, { get: (t, p) => t[p as string] }),
34+
set: vi.fn((key: string, value: unknown) => {
35+
vars[key] = value;
36+
}),
37+
} as unknown as Context;
38+
}
39+
1440
describe("chat-ingestion", () => {
1541
const originalFetch = global.fetch;
1642

@@ -211,7 +237,7 @@ describe("chat-ingestion", () => {
211237
headers: { "Content-Type": "application/json" },
212238
}),
213239
);
214-
const c = { var: {}, set: vi.fn() } as unknown as Context;
240+
const c = makeTestContext();
215241

216242
await persistChatSessionToConvex(
217243
{
@@ -238,7 +264,7 @@ describe("chat-ingestion", () => {
238264
global.fetch = vi.fn().mockResolvedValue(
239265
new Response("Server Error", { status: 503 }),
240266
);
241-
const c = { var: {}, set: vi.fn() } as unknown as Context;
267+
const c = makeTestContext();
242268

243269
await persistChatSessionToConvex(
244270
{
@@ -274,7 +300,7 @@ describe("chat-ingestion", () => {
274300
}
275301
}),
276302
) as typeof fetch;
277-
const c = { var: {}, set: vi.fn() } as unknown as Context;
303+
const c = makeTestContext();
278304

279305
const p = persistChatSessionToConvex(
280306
{
@@ -301,7 +327,7 @@ describe("chat-ingestion", () => {
301327

302328
it("emits chat.session.persist.failed(exception) via typed event when c is provided", async () => {
303329
global.fetch = vi.fn().mockRejectedValue(new Error("network failure"));
304-
const c = { var: {}, set: vi.fn() } as unknown as Context;
330+
const c = makeTestContext();
305331

306332
await persistChatSessionToConvex(
307333
{

mcpjam-inspector/server/utils/logger.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ function ingestToAxiom(
4747
export const logger = {
4848
/**
4949
* Log an error. Always sends to Sentry and Axiom, only prints to console in dev/verbose mode.
50+
*
51+
* NOTE: For new production diagnostics in `server/routes/web/`, prefer `logger.event()` —
52+
* free-form messages are not queryable in Axiom. Legacy callers (CLI, system code,
53+
* non-route utilities) remain on this API. See `server/utils/LOGGING.md`.
5054
*/
5155
error(message: string, error?: unknown, context?: Record<string, unknown>) {
5256
Sentry.captureException(error ?? new Error(message), {
@@ -65,6 +69,10 @@ export const logger = {
6569

6670
/**
6771
* Log a warning. Always sends to Sentry and Axiom, only prints to console in dev/verbose mode.
72+
*
73+
* NOTE: For new production diagnostics in `server/routes/web/`, prefer `logger.event()` —
74+
* free-form messages are not queryable in Axiom. Legacy callers (CLI, system code,
75+
* non-route utilities) remain on this API. See `server/utils/LOGGING.md`.
6876
*/
6977
warn(message: string, context?: Record<string, unknown>) {
7078
Sentry.captureMessage(message, { level: "warning", extra: context });
@@ -78,6 +86,10 @@ export const logger = {
7886

7987
/**
8088
* Log info. Always sends to Axiom. Only prints to console in dev/verbose mode.
89+
*
90+
* NOTE: For new production diagnostics in `server/routes/web/`, prefer `logger.event()` —
91+
* free-form messages are not queryable in Axiom. Legacy callers (CLI, system code,
92+
* non-route utilities) remain on this API. See `server/utils/LOGGING.md`.
8193
*/
8294
info(message: string, context?: Record<string, unknown>) {
8395
ingestToAxiom("info", message, context);

0 commit comments

Comments
 (0)