Improve Server Log DX #1085
Improve Server Log DX #1085
Conversation
dosubot
bot
added
the
size:XL
WalkthroughThis pull request introduces a centralized logging infrastructure across the codebase. A new logger module integrates with Sentry for error reporting while supporting environment-based conditional console output. Verbose logging support is added via Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
server/routes/mcp/servers.ts (1)
109-112: Replace console.debug with logger.debug.This console.debug usage is inconsistent with the centralized logging migration. Per AGENTS.md guidelines, server code should use the logger utility.
Apply this diff:
// Ignore disconnect errors for already disconnected servers - console.debug( - `Failed to disconnect MCP server ${serverId} during removal`, - error, - ); + logger.debug( + `Failed to disconnect MCP server ${serverId} during removal`, + error, + );server/routes/mcp/resources.ts (1)
36-55: Fix out-of-scope variables in catch blocks (will not compile).
serverId/uriare declared in thetryscope but referenced incatch(Line 47, Line 88). Hoist them.// List resources endpoint resources.post("/list", async (c) => { + let serverId: string | undefined; try { - const { serverId } = (await c.req.json()) as { serverId?: string }; + ({ serverId } = (await c.req.json()) as { serverId?: string }); if (!serverId) { return c.json({ success: false, error: "serverId is required" }, 400); } @@ } catch (error) { logger.error("Error fetching resources", error, { serverId }); return c.json( @@ }); // Read resource endpoint resources.post("/read", async (c) => { + let serverId: string | undefined; + let uri: string | undefined; try { - const { serverId, uri } = (await c.req.json()) as { - serverId?: string; - uri?: string; - }; + ({ serverId, uri } = (await c.req.json()) as { + serverId?: string; + uri?: string; + }); if (!serverId) { return c.json({ success: false, error: "serverId is required" }, 400); } @@ } catch (error) { logger.error("Error reading resource", error, { serverId, uri }); return c.json(Also applies to: 59-96
server/routes/mcp/tokenizer.ts (1)
104-128: Downgrade expected fallback warnings to avoid Sentry flooding.
logger.warn()unconditionally sends to Sentry viacaptureMessage()(not subject totracesSampleRatesampling). The three paths at lines 104–128 and analogous locations (145–152, 230–253, 267–274) are expected failure modes with graceful fallbacks. When the tokenizer backend is unavailable or a model lacks mapping, each server/request triggers a warn message. Over time, this can accumulate unnecessarily in Sentry.Use
logger.debug()for these expected recovery paths instead. If you need visibility during issues, implement a targeted Sentry filter viabeforeSendrather than logging all expected failures as warnings.bin/start.js (1)
357-366:--portappears broken: parsed value is ignored and 6274 is always used.
You setenvVars.PORT = portbut later always proberequestedPort = 6274, even in the “explicit port” branch.- // Port configuration (fixed default to 6274) - const requestedPort = 6274; + // Port configuration (default 6274; allow --port override) + const defaultPort = 6274; + const requestedPort = envVars.PORT != null ? Number(envVars.PORT) : defaultPort; + if (!Number.isInteger(requestedPort) || requestedPort < 1 || requestedPort > 65535) { + logError(`Invalid port: ${envVars.PORT}`); + process.exit(1); + } let PORT; try { // Check if user explicitly set a port via --port flag const hasExplicitPort = envVars.PORT !== undefined; if (hasExplicitPort) { if (await isPortAvailable(requestedPort)) { PORT = requestedPort.toString(); } else { - logError(`Explicitly requested port ${requestedPort} is not available`); + logError(`Requested port ${requestedPort} is not available`); logInfo( "Use a different port with --port <number> or let the system find one automatically", ); throw new Error(`Port ${requestedPort} is already in use`); } } else { - // Fixed port policy: use default port 6274 and fail fast if unavailable + // Default port policy: use 6274 and fail fast if unavailable if (await isPortAvailable(requestedPort)) { PORT = requestedPort.toString(); } else { logError( `Default port ${requestedPort} is already in use. Please free the port`, ); throw new Error(`Port ${requestedPort} is already in use`); } }Also applies to: 626-667
🧹 Nitpick comments (9)
server/routes/mcp/apps.ts (1)
176-177: Consider log level for security-relevant configuration.While the debug level aligns with verbose logging patterns, CSP configuration may warrant
logger.infoto ensure security-relevant data is captured in production environments, not just verbose mode.If CSP configuration should be auditable in production, apply this diff:
- logger.debug("[MCP Apps] CSP configuration", { + logger.info("[MCP Apps] CSP configuration", { resourceUri, effectiveCspMode,server/routes/mcp/tunnels.ts (2)
195-201: Consider adding request context on “Error creating tunnel”.If available, include
credentialId/domainId/domain(when fetched) to cut triage time.
246-252: Consider adding request context on “Error closing tunnel”.E.g., include
credentialId/domainId(already read above) as logger context.server/routes/mcp/prompts.ts (1)
52-65: Make the “Unknown MCP server” suppression less brittle.Substring matching is easy to break; if the SDK can expose a typed error/code, prefer that. Otherwise, consider a narrow regex or a dedicated helper to centralize this heuristic.
server/index.ts (1)
247-269: Static serving simplification looks good; consider cwd-sensitivity.
UsingserveStatic({ root: "./dist/client" })and separatelyprocess.cwd()forindex.htmlassumes a stable working directory—worth sanity-checking for Electron/resources launches.server/services/evals-runner.ts (2)
131-136: Use logger context to preserve the most actionable metadata (iterationId/runId/status/url).
Severallogger.error(...)calls drop useful structured fields that you already have in-scope; passingcontextwill make Sentry + local debugging far faster.-logger.error("[evals] backend fetch failed", error); +logger.error("[evals] backend fetch failed", error, { + runId, + iterationId, + convexHttpUrl, + steps, +});(Apply similarly to the other error sites: create/finish iteration failures, invalid payload, stream non-OK, test-case failures.)
Also applies to: 395-403, 651-679, 900-933
584-601: Backend stream error: log the real failure, not juststatusText.
Right now Sentry getsnew Error(res.statusText)but the actualerrorText(often the important part) is only in a local string.-logger.error("[evals] backend stream error", new Error(res.statusText)); +logger.error("[evals] backend stream error", new Error(errorText || res.statusText), { + status: res.status, +});server/utils/logger.ts (1)
25-35: Consider whether “warn always goes to Sentry” is too chatty outside prod.
If warnings are expected during local/dev, you may want to gatecaptureMessagesimilarly to avoid noisy projects.bin/start.js (1)
331-343: Verbose flag propagation is wired correctly; small simplification opportunity.
You detect--verbose/-vtwice (pre-pass + main parse); not harmful, but you could drop one to reduce cognitive load.Also applies to: 715-725
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
AGENTS.md(1 hunks)bin/start.js(14 hunks)client/CLAUDE.md(0 hunks)docs/installation.mdx(1 hunks)lib/launcher/index.ts(2 hunks)lib/vite/index.ts(2 hunks)sdk/src/mcp-client-manager/index.ts(1 hunks)server/CLAUDE.md(0 hunks)server/app.ts(3 hunks)server/index.ts(6 hunks)server/routes/apps/chatgpt.ts(4 hunks)server/routes/mcp/apps.ts(5 hunks)server/routes/mcp/chat-v2.ts(3 hunks)server/routes/mcp/evals.ts(7 hunks)server/routes/mcp/list-tools.ts(2 hunks)server/routes/mcp/log-level.ts(2 hunks)server/routes/mcp/models.ts(3 hunks)server/routes/mcp/oauth.ts(6 hunks)server/routes/mcp/prompts.ts(5 hunks)server/routes/mcp/registry.ts(4 hunks)server/routes/mcp/resource-templates.ts(2 hunks)server/routes/mcp/resources.ts(3 hunks)server/routes/mcp/servers.ts(6 hunks)server/routes/mcp/tokenizer.ts(6 hunks)server/routes/mcp/tools.ts(2 hunks)server/routes/mcp/tunnels.ts(8 hunks)server/services/eval-agent.ts(2 hunks)server/services/evals-runner.ts(10 hunks)server/services/evals/recorder.ts(6 hunks)server/services/tunnel-manager.ts(3 hunks)server/utils/logger.ts(1 hunks)
💤 Files with no reviewable changes (2)
- server/CLAUDE.md
- client/CLAUDE.md
🧰 Additional context used
📓 Path-based instructions (3)
server/**/*.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Implement TypeScript for type safety throughout the codebase
Files:
server/services/eval-agent.tsserver/routes/apps/chatgpt.tsserver/routes/mcp/registry.tsserver/routes/mcp/list-tools.tsserver/routes/mcp/chat-v2.tsserver/routes/mcp/resource-templates.tsserver/app.tsserver/routes/mcp/tokenizer.tsserver/utils/logger.tsserver/routes/mcp/models.tsserver/services/tunnel-manager.tsserver/services/evals/recorder.tsserver/routes/mcp/evals.tsserver/services/evals-runner.tsserver/routes/mcp/oauth.tsserver/routes/mcp/prompts.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.tsserver/routes/mcp/tunnels.tsserver/routes/mcp/log-level.tsserver/index.ts
server/**/{app,index,routes/**}.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Use Hono.js for API routing and middleware in the backend
Files:
server/app.tsserver/index.ts
server/**/routes/**/*auth*.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Implement OAuth 2.0 token validation flow with scope verification and refresh mechanisms
Files:
server/routes/mcp/oauth.ts
🧠 Learnings (25)
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/llm/openai/**/*.ts : Implement OpenAI API client setup with model management, stream processing, and error handling
Applied to files:
server/services/eval-agent.tsserver/routes/apps/chatgpt.tsAGENTS.mdserver/routes/mcp/chat-v2.tsserver/routes/mcp/resource-templates.tsserver/app.tsserver/routes/mcp/models.tsserver/services/evals/recorder.tsserver/routes/mcp/evals.tsserver/services/evals-runner.tsserver/routes/mcp/oauth.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tunnels.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/utils/*response*.ts : Implement LLM response handling with stream processing, token counting, format validation, and safety checks
Applied to files:
server/services/eval-agent.tsserver/routes/mcp/tokenizer.tsserver/routes/mcp/tools.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/llm.ts : Define LLM integration types in TypeScript type definition files
Applied to files:
server/services/eval-agent.tsserver/services/evals-runner.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{stores/chat,hooks/llm}/**/*.{ts,tsx} : Manage context with window size tracking, token counting, context pruning, and state persistence for chat sessions
Applied to files:
server/routes/apps/chatgpt.tsserver/routes/mcp/chat-v2.tsserver/routes/mcp/tokenizer.tsserver/services/evals-runner.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/chat/**/*.{ts,tsx} : Implement AI model state handling including model selection state, generation parameters, stream management, and history persistence
Applied to files:
server/routes/apps/chatgpt.tsserver/routes/mcp/chat-v2.tsserver/services/evals/recorder.tsserver/services/evals-runner.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/servers/**/*.{ts,tsx} : Synchronize MCP connection state, request tracking, response handling, and error management with global state
Applied to files:
server/routes/mcp/registry.tsserver/routes/mcp/list-tools.tsserver/routes/mcp/chat-v2.tsserver/routes/mcp/resource-templates.tsserver/app.tsserver/routes/mcp/models.tsserver/routes/mcp/evals.tsserver/services/evals-runner.tsserver/routes/mcp/oauth.tsserver/routes/mcp/prompts.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.tsserver/routes/mcp/tunnels.tsserver/routes/mcp/log-level.tsserver/index.tssdk/src/mcp-client-manager/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/mcp/resources/**/*.ts : Implement resource schema compliance checking and type validation in resource handlers
Applied to files:
server/routes/mcp/registry.tsserver/routes/mcp/resource-templates.tsserver/routes/mcp/models.tsserver/routes/mcp/evals.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/mcp.ts : Define MCP protocol types in TypeScript type definition files
Applied to files:
server/routes/mcp/registry.tsserver/routes/mcp/list-tools.tsserver/routes/mcp/chat-v2.tsserver/routes/mcp/resource-templates.tsserver/app.tsserver/routes/mcp/models.tsserver/routes/mcp/evals.tsserver/services/evals-runner.tsserver/routes/mcp/oauth.tsserver/routes/mcp/prompts.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.tsserver/routes/mcp/tunnels.tsserver/routes/mcp/log-level.tsserver/index.tssdk/src/mcp-client-manager/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/{app,index,routes/**}.ts : Use Hono.js for API routing and middleware in the backend
Applied to files:
server/routes/mcp/registry.tsserver/routes/mcp/list-tools.tsserver/routes/mcp/resource-templates.tsserver/app.tsserver/routes/mcp/models.tsserver/routes/mcp/oauth.tsserver/routes/mcp/prompts.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tunnels.tsserver/routes/mcp/log-level.tsserver/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/mcp/tools/**/*.ts : Validate tool schemas according to MCP spec including parameters and response formats
Applied to files:
server/routes/mcp/list-tools.tsserver/routes/mcp/resource-templates.tsserver/routes/mcp/models.tsserver/routes/mcp/evals.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.tsserver/routes/mcp/tunnels.tsserver/index.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Implement MCP server management UI with connection list, status indicators, quick actions, and group management
Applied to files:
server/routes/mcp/list-tools.tsserver/routes/mcp/resource-templates.tsserver/services/evals-runner.tsserver/routes/mcp/prompts.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/index.tssdk/src/mcp-client-manager/index.tsbin/start.js
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/lib/api/**/*.{ts,tsx} : Implement OpenAI API integration with API client setup, model configuration, response handling, and error recovery
Applied to files:
AGENTS.mdserver/routes/mcp/models.tsserver/services/evals/recorder.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{hooks/llm,components/chat}/**/*.{ts,tsx} : Implement response streaming with token processing, UI updates, cancel handling, and error states
Applied to files:
server/routes/mcp/chat-v2.tsserver/routes/mcp/tokenizer.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Build chat interface with message components, input handling, stream rendering, and history management
Applied to files:
server/routes/mcp/chat-v2.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/mcp/prompts/**/*.ts : Validate prompt context, token limits, and format in prompt processing handlers
Applied to files:
server/routes/mcp/resource-templates.tsserver/routes/mcp/tokenizer.tsserver/routes/mcp/evals.tsserver/routes/mcp/prompts.tsserver/routes/mcp/resources.tsserver/routes/mcp/apps.tsserver/routes/mcp/tools.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/utils/validation.ts : Use schema validation for MCP protocol compliance in validation utility files
Applied to files:
server/routes/mcp/resource-templates.tsserver/routes/mcp/evals.tsserver/routes/mcp/resources.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/utils/*{connection,pool,circuit}*.ts : Implement connection management with health checks, load balancing, circuit breakers, and auto-recovery
Applied to files:
server/routes/mcp/resource-templates.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/tunnels.tssdk/src/mcp-client-manager/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/*.ts : Implement TypeScript for type safety throughout the codebase
Applied to files:
server/app.tsserver/utils/logger.tsserver/routes/mcp/evals.tsserver/routes/mcp/servers.tsserver/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/llm/claude/**/*.ts : Implement Claude integration with authentication, request handling, response streaming, and rate limiting
Applied to files:
server/app.tsserver/routes/mcp/models.tsserver/routes/mcp/oauth.tsserver/routes/mcp/servers.tsserver/routes/mcp/resources.tsserver/routes/mcp/tunnels.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/lib/api/**/*.{ts,tsx} : Implement Ollama local model support with local setup, model management, inference options, and resource control
Applied to files:
server/services/evals-runner.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/**/*auth*.ts : Implement OAuth 2.0 token validation flow with scope verification and refresh mechanisms
Applied to files:
server/routes/mcp/oauth.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{components/servers,stores/servers}/**/*.{ts,tsx} : Implement authentication setup with OAuth configuration, token management, scope selection, and refresh handling
Applied to files:
server/routes/mcp/oauth.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/transport/sse/**/*.ts : Implement Server-Sent Events (SSE) with event streaming, connection handling, and reconnection logic
Applied to files:
server/routes/mcp/servers.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{stores/servers,lib/api}/**/*.{ts,tsx} : Persist server configurations with local storage, export/import functionality, sync options, and backup/restore capabilities
Applied to files:
server/routes/mcp/servers.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Use Vite + React for fast development and optimized builds
Applied to files:
server/index.ts
🧬 Code graph analysis (21)
server/services/eval-agent.ts (1)
server/utils/logger.ts (1)
logger(11-53)
server/routes/apps/chatgpt.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/registry.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/list-tools.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/resource-templates.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/app.ts (1)
server/utils/logger.ts (1)
logger(11-53)
server/routes/mcp/tokenizer.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/utils/logger.ts (2)
client/src/hooks/use-logger.ts (1)
shouldLog(83-85)bin/start.js (1)
args(312-312)
server/routes/mcp/models.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/services/tunnel-manager.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/services/evals/recorder.ts (1)
server/utils/logger.ts (1)
logger(11-53)
server/routes/mcp/evals.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/oauth.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/prompts.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/servers.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/resources.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/apps.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/tools.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/tunnels.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/routes/mcp/log-level.ts (1)
server/utils/logger.ts (2)
logger(11-53)error(15-23)
server/index.ts (2)
server/utils/logger.ts (2)
error(15-23)logger(11-53)server/config.ts (1)
SERVER_HOSTNAME(11-12)
🔍 Remote MCP
I'll search for relevant information to help contextualize this PR's logging improvements and best practices.## Summary of Relevant Context for PR #1085 Review
Sentry Integration Best Practices
The PR's logger implementation aligns with Sentry best practices by using Sentry.logger with structured methods (info, warn, error) and including contextual data in log statements. Additionally, Node.js best practices emphasize always using Error instances when logging to Sentry and including contextual information through extra parameters to help diagnose error causes.
Centralized Logging Architecture
The PR's approach of creating a centralized logger module (server/utils/logger.ts) is well-founded. Centralized logging simplifies management, enables more efficient analysis of logs from multiple sources to identify patterns, and makes troubleshooting faster by pinpointing root causes more quickly.
Verbose Mode Implementation
The verbose logging feature follows established CLI conventions. POSIX-compliant argument syntax (such as the --verbose / -v pattern implemented in this PR) ensures CLIs align with familiar standards and reduces the learning curve for users. A verbose log level is particularly helpful for debugging purposes or when deeper insights into the process are needed.
MCP Protocol Logging Considerations
MCP protocol documentation specifically notes that local MCP servers should not log messages to stdout, as this will interfere with protocol operation. This is relevant context for the PR's implementation, particularly for error handling in the SDK's MCP client manager.
Log Level Classification
Log levels allow categorization of logs based on severity, enabling filtering and appropriate management. The PR's implementation with error, warn, info, and debug levels aligns with standard logging level hierarchy (error, warn, info, verbose, debug).
Error Context and Structured Logging
For effective debug logging in servers, focus should be on capturing connection/authentication events, request/response handling, and capturing errors at each processing stage with contextual data like stack traces or error codes.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (42)
server/services/evals/recorder.ts (1)
4-4: Clean migration to centralized logger.The integration follows best practices: errors are properly wrapped in Error instances for Sentry, context is included where available, and debug messages appropriately use logger.debug. This aligns with the Sentry integration patterns noted in the external tools context.
Also applies to: 111-119, 143-146, 172-175, 213-216, 246-249
server/routes/mcp/chat-v2.ts (1)
17-17: LGTM!Error logging properly migrated to centralized logger with clear route prefixes.
Also applies to: 252-252, 268-268
AGENTS.md (1)
1-49: Excellent documentation for logging patterns.Clear guidance with practical examples. The distinction between server, CLI, and client logging contexts is well-articulated.
server/routes/apps/chatgpt.ts (1)
7-7: LGTM!Widget-related error paths now properly route through the centralized logger.
Also applies to: 395-395, 517-517, 652-652
lib/vite/index.ts (1)
44-50: Clean verbose mode integration.The optional verbose flag properly extends the plugin API and propagates to the launcher.
Also applies to: 90-90
server/app.ts (1)
5-5: Well-structured conditional logging.The aliased import avoids conflicts, debug-level logging is appropriate for the flag check, and HTTP request logging is now environment-aware. This elegantly addresses the PR's objective of improving log UX.
Also applies to: 82-82, 91-97
server/routes/mcp/oauth.ts (5)
155-155: Centralized error logging adopted correctly.The migration from console.error to logger.error aligns with the PR's logging infrastructure improvements.
174-174: LGTM!Error logging properly captures SSE parsing failures with the centralized logger.
202-202: Proper error handling with centralized logging.The descriptive prefix aids in log filtering and debugging.
312-312: Centralized logging applied consistently.Error handling maintains clarity while adopting the new logging infrastructure.
372-372: Consistent error logging pattern.The OAuth routes now uniformly adopt the centralized logger, improving observability.
server/services/eval-agent.ts (1)
264-266: Exemplary structured logging with contextual data.Including
assistantResponsein the context enables effective debugging of LLM parsing failures, aligning with Sentry best practices for diagnostic information.lib/launcher/index.ts (2)
39-44: Well-designed API extension for verbose mode.The optional property with clear documentation follows established CLI conventions and integrates seamlessly with the launcher interface.
142-144: Clean propagation of verbose mode to child process.The environment variable approach correctly enables verbose logging in the spawned inspector instance.
server/routes/mcp/tools.ts (1)
51-53: Appropriate log level for non-critical error.Using
logger.warncorrectly reflects that token counting failures have a safe fallback (returning 0), making this a warning rather than an error-level event.server/routes/mcp/models.ts (2)
48-50: Exemplary error logging with structured context.Creating an Error instance from the response text and including the HTTP status aligns with Sentry best practices for effective error tracking and debugging.
63-63: Consistent error logging pattern.The descriptive prefix and centralized logger usage maintain observability standards across the codebase.
server/routes/mcp/registry.ts (3)
27-27: LGTM!Error logging appropriately captures registry server fetch failures.
54-54: Structured logging with appropriate context.Including
serverNameaids in diagnosing version fetch failures for specific registry servers.
83-86: Comprehensive contextual logging.Including both
serverNameandversionprovides complete diagnostic information for specific version fetch failures.server/routes/mcp/apps.ts (1)
156-158: Appropriate warning level for SEP-1865 compliance.Using
logger.warncorrectly reflects that mimetype validation failures are non-blocking compliance issues rather than critical errors.server/routes/mcp/tunnels.ts (6)
6-6: Logger import looks fine.
76-78: Good: skip-tunnel-recording warning is now centralized.
95-97: Good:recordTunnelerror now includesserverId/urlcontext.
125-126: Good: closure reporting failure is captured with server context.
155-159: Good: credential cleanup failures now capture identifiers.
264-270: Good: orphaned cleanup errors now go through logger.server/routes/mcp/list-tools.ts (3)
4-5: Logger adoption is consistent with the new pattern.
44-48: Good: warning includes structuredserverId+ error string.
53-59: Good: top-level failure reports exception to Sentry vialogger.error.server/services/tunnel-manager.ts (1)
3-3: Import is fine.server/routes/mcp/resources.ts (1)
3-4: Logger import is fine.server/routes/mcp/prompts.ts (3)
3-4: Logger import is consistent.
19-27: Good: centralized error reporting for /list.
125-132: Good: /get failures now go through logger.server/routes/mcp/tokenizer.ts (1)
7-8: Import is fine.server/routes/mcp/evals.ts (3)
12-13: Logger import is consistent with the rest of the PR.
82-87: Good: tool-discovery failures now include structured context.
388-405: Good: error paths now reliably capture exceptions (Sentry) vialogger.error.Also applies to: 537-539, 573-584, 648-654
server/index.ts (1)
202-207: Conditional HTTP request logging is a nice UX win; verify stdout constraints.
Gatinghono/loggerbehind dev/VERBOSE_LOGS should reduce noise; just ensure enabling it can’t interfere with any stdout-sensitive modes in your packaging/runtime.server/services/evals-runner.ts (1)
15-16: Good consolidation onto the centralized logger.
Replacing console logging here should make verbosity consistent across eval runs.Also applies to: 101-104
bin/start.js (1)
13-21: Banner + boxed output improvements look clean and readable.
The split-color banner and themed box drawing are straightforward and should improve startup UX.Also applies to: 51-59, 97-117, 306-310
| let verboseLogs = false; | ||
|
|
||
| // First pass: check for --verbose flag before processing other args | ||
| for (const arg of args) { | ||
| if (arg === "--verbose" || arg === "-v") { | ||
| verboseLogs = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Conditional logging functions (only log when verbose) | ||
| const verboseInfo = (message) => verboseLogs && logInfo(message); | ||
| const verboseSuccess = (message) => verboseLogs && logSuccess(message); | ||
| const verboseStep = (step, message) => verboseLogs && logStep(step, message); | ||
|
|
There was a problem hiding this comment.
Verbose logging may leak secrets via custom header values; redact by key.
Even in verbose mode, printing raw header values (especially cookies/tokens) is risky in shared terminals/log captures.
+ const redactHeaderValue = (key, value) =>
+ /authorization|cookie|token|secret|api[-_]?key/i.test(String(key))
+ ? "<redacted>"
+ : value;
+
for (const { key, value } of customHeaders) {
headers[key] = value;
- verboseInfo(`Header: ${key}=${value}`);
+ verboseInfo(`Header: ${key}=${redactHeaderValue(key, value)}`);
}Also applies to: 421-425, 549-552
|
|
||
| ## Start with Verbose Logging | ||
|
|
||
| Enable verbose HTTP request logging for debugging purposes. This is useful when troubleshooting connection issues with MCP servers. | ||
|
|
||
| ```bash | ||
| npx @mcpjam/inspector@latest --verbose | ||
| ``` | ||
|
|
||
| Or use the short form: | ||
|
|
||
| ```bash | ||
| npx @mcpjam/inspector@latest -v | ||
| ``` | ||
|
|
||
| <Info> | ||
| Verbose mode logs all HTTP requests made by the inspector, which can help | ||
| diagnose connectivity problems or inspect server communication. | ||
| </Info> |
There was a problem hiding this comment.
Doc addition is clear; align scope with actual verbose behavior.
Wording says “logs all HTTP requests”; if verbose also affects other logs (or only a subset), consider tightening to “enables verbose logging (including HTTP requests)” to avoid over-promising.
🤖 Prompt for AI Agents
In docs/installation.mdx around lines 144 to 162, the copy currently claims
“Verbose mode logs all HTTP requests,” which overstates behavior; update the
wording to accurately reflect scope (e.g., “enables verbose logging, including
HTTP requests” or specify what verbose actually logs). Edit the paragraph and
Info block to use the tightened phrasing and, if helpful, add a short
parenthetical listing of other log types affected (or explicitly state it only
affects HTTP request logging) so the docs match the implementation.
| // Set logging level if supported (ignore errors if server doesn't support it) | ||
| this.setLoggingLevel(serverId, "debug").catch(() => { | ||
| // Silently ignore - server may not support logging/setLevel | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file to understand the context and current implementation
head -550 sdk/src/mcp-client-manager/index.ts | tail -50Repository: MCPJam/inspector
Length of output: 1368
🏁 Script executed:
# Look at the setLoggingLevel implementation
sed -n '537,547p' sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 471
🏁 Script executed:
# Check what type 'client' is and what methods it has
sed -n '300,330p' sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 906
🌐 Web query:
MCP spec logging/setLevel method JSON-RPC error response @modelcontextprotocol/sdk v1.20.2
💡 Result:
Short answer — per the MCP spec, logging/setLevel is a JSON‑RPC request that must either return an empty result or a standard JSON‑RPC error when it fails. Common error codes defined by the spec are:
- Invalid params (code -32602) — e.g., unknown/unsupported level value.
- Internal error (code -32603) — e.g., configuration or runtime failure.
Example error response (invalid level):
{
"jsonrpc": "2.0",
"id": 1,
"error": {
"code": -32602,
"message": "Invalid params: unknown logging level",
"data": { "accepted": ["debug","info","notice","warning","error","critical","alert","emergency"] }
}
}
The spec’s logging page and schema for logging/setLevel describe the request shape and the recommended error codes. The official SDKs implement/expect this behavior (see SDK logging docs). [1][2][3]
Sources:
[1] MCP Logging page — error handling and setLevel examples.
[2] MCP Schema — logging/setLevel request definition.
[3] MCP C# SDK logging docs (client SetLoggingLevel behavior / examples).
🏁 Script executed:
# Check the type of client and its setLoggingLevel method
rg -A 5 "class MCPClientManager" sdk/src/mcp-client-manager/index.ts | head -20Repository: MCPJam/inspector
Length of output: 288
🏁 Script executed:
# Look for the client type and imports at top of file
head -50 sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 2099
🏁 Script executed:
# Search for how errors from client are handled elsewhere
rg "error\.code|JsonRpcError|Method not found" sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 42
🏁 Script executed:
# Check package.json for MCP SDK version
cat package.json | grep -A 2 -B 2 "modelcontextprotocol"Repository: MCPJam/inspector
Length of output: 248
🏁 Script executed:
# Search for how client errors are handled elsewhere in the codebase
rg "client\." sdk/src/mcp-client-manager/index.ts | grep -i "catch\|error" | head -10Repository: MCPJam/inspector
Length of output: 97
🏁 Script executed:
# Look for any error checking or error type usage
rg "message.*not found\|method.*not found\|32601" sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 42
🏁 Script executed:
# Check if there are any other fire-and-forget or error suppression patterns
rg "\.catch\(" sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 147
🌐 Web query:
@modelcontextprotocol/sdk Client.setLoggingLevel() error response format JSON-RPC code property
💡 Result:
Client.setLoggingLevel() errors follow MCP’s JSON‑RPC 2.0 error shape: an "error" object with numeric code (integer), message (string) and optional data. Typical codes for setLoggingLevel are:
- -32602 (Invalid params) — e.g. invalid log level value. [1][2]
- -32603 (Internal error) — e.g. configuration/server failure. [3][2]
Example MCP error response shape:
{
"jsonrpc": "2.0",
"id": "",
"error": { "code": , "message": "", "data": }
} [2][4]
Sources:
[1] MCP — Logging (server utilities) (shows error cases for setLoggingLevel).
[2] MCP Specification — Messages / JSON‑RPC error response (schema and requirements).
[3] McpErrorCode enum (lists -32602 and -32603 meanings).
[4] MCP Schema Reference — JSONRPC error type.
🌐 Web query:
MCP SDK client error structure -32601 Method not found code how to check
💡 Result:
Short answer — -32601 is the standard JSON‑RPC "Method not found" code. To check it from an MCP client:
-
Inspect the JSON‑RPC response for an error object and check error.code === -32601 and error.message === "Method not found". The error object may include error.data with the missing method name. [1][2]
-
If you get -32601 from an MCP server, verify the server advertised capabilities (during handshake / mcp.describe); call/list the server methods to confirm which methods are implemented. [2][7]
-
Troubleshooting checklist:
- Confirm your client sent the correct method name and params (case/namespace exact). [1]
- Ensure McpServer.connect() returned a valid connection (not null/undefined) before calling methods — otherwise calls will return -32601. Check server logs for connection errors. [7]
- If applicable, enable debug logging on the server to see why the method wasn’t registered. [8]
References:
[1] JSON‑RPC 2.0 error structure and codes (includes -32601).
[2] MCP‑specific guide explaining -32601 and that servers advertise capabilities/methods.
[7] modelcontextprotocol/typescript‑sdk issue showing connect() null can produce "Method not found".
🏁 Script executed:
# Verify if there's already a utility function for checking MCP error codes
rg "32601\|isMethod\|Unavailable" sdk/src/Repository: MCPJam/inspector
Length of output: 42
🏁 Script executed:
# Check if error structure is documented in any types file
rg -A 3 "error.*code\|code.*error" sdk/src/ | head -30Repository: MCPJam/inspector
Length of output: 42
🏁 Script executed:
# Check the full connectToServer method to see config.onError usage
sed -n '280,340p' sdk/src/mcp-client-manager/index.tsRepository: MCPJam/inspector
Length of output: 1694
🏁 Script executed:
# Verify config.onError is available in this scope
rg "config.onError" sdk/src/mcp-client-manager/index.ts | head -5Repository: MCPJam/inspector
Length of output: 122
Don't silently swallow all setLoggingLevel failures; filter only "method not found" and propagate the rest.
Right now any auth, transport, or protocol failure when enabling logging is hidden, making diagnostics harder. Since client is already in-scope, you can call it directly and preserve error structure for filtering:
- // Set logging level if supported (ignore errors if server doesn't support it)
- this.setLoggingLevel(serverId, "debug").catch(() => {
- // Silently ignore - server may not support logging/setLevel
- });
+ // Best-effort: enable server-side logging when supported.
+ void client.setLoggingLevel("debug").catch((error) => {
+ // Ignore servers that don't support logging (JSON-RPC code -32601).
+ if (
+ error instanceof Error &&
+ error.message?.includes("Method not found")
+ ) {
+ return;
+ }
+ config.onError?.(error);
+ });This surfaces real errors (invalid level, server failure) while gracefully ignoring genuinely unsupported servers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set logging level if supported (ignore errors if server doesn't support it) | |
| this.setLoggingLevel(serverId, "debug").catch(() => { | |
| // Silently ignore - server may not support logging/setLevel | |
| }); | |
| // Best-effort: enable server-side logging when supported. | |
| void client.setLoggingLevel("debug").catch((error) => { | |
| // Ignore servers that don't support logging (JSON-RPC code -32601). | |
| if ( | |
| error instanceof Error && | |
| error.message?.includes("Method not found") | |
| ) { | |
| return; | |
| } | |
| config.onError?.(error); | |
| }); |
🤖 Prompt for AI Agents
In sdk/src/mcp-client-manager/index.ts around lines 325-328, don't swallow all
errors from setLoggingLevel; call the in-scope client directly
(client.setLoggingLevel(serverId, "debug")) and catch the error only to filter
the RPC "method not found" case, otherwise rethrow or propagate it. Implement
the catch to detect the specific RPC error (e.g., error.code === -32601 or
error.message/name contains "Method not found") and return silently only for
that condition; for any other error (auth, transport, invalid level, server
failure) rethrow or pass the error along so it surfaces to diagnostics.
| appLogger.error("Failed to parse MCP_CONFIG_DATA:", error); | ||
| } |
There was a problem hiding this comment.
Harden SPA config injection against </script>-style XSS.
Injecting JSON.stringify(mcpConfig) directly into a <script> can be broken out of if any string contains </script> (or < generally). Escaping is cheap and removes a sharp edge.
+function safeJsonForInlineScript(value: unknown) {
+ // Prevent closing the script tag / HTML parsing quirks.
+ return JSON.stringify(value)
+ .replace(/</g, "\\u003c")
+ .replace(/>/g, "\\u003e")
+ .replace(/&/g, "\\u0026");
+}
+
// Inject MCP server config if provided via CLI
const mcpConfig = getMCPConfigFromEnv();
if (mcpConfig) {
- const configScript = `<script>window.MCP_CLI_CONFIG = ${JSON.stringify(mcpConfig)};</script>`;
+ const configScript = `<script>window.MCP_CLI_CONFIG = ${safeJsonForInlineScript(mcpConfig)};</script>`;
htmlContent = htmlContent.replace("</head>", `${configScript}</head>`);
}Also applies to: 258-267
🤖 Prompt for AI Agents
In server/index.ts around lines 97-98 (and also apply the same change at
258-267), the code logs or injects JSON.stringify(mcpConfig) directly into a
<script> which can be broken out of by strings containing "</script>" or raw
"<"; update the injection to escape problematic characters before embedding by
replacing "<" with its Unicode escape (e.g.
JSON.stringify(mcpConfig).replace(/</g, '\\u003c')) (or equivalently escape
"</script>" to "<\/script>") so the injected JSON cannot prematurely close the
script tag; apply this change to both places mentioned and keep existing error
handling/logging untouched.
| logger.error("Error setting MCP server logging level", error, { | ||
| serverId, | ||
| level, | ||
| }); |
There was a problem hiding this comment.
Potential ReferenceError when accessing destructured variables.
The serverId and level variables are destructured within the try block (Line 26). If JSON parsing fails or an error occurs before the destructuring completes, these variables won't exist, causing the context object at lines 66-67 to throw a ReferenceError.
Apply this diff to safely access the variables:
} catch (error) {
- logger.error("Error setting MCP server logging level", error, {
- serverId,
- level,
- });
+ const body = await c.req.json().catch(() => ({}));
+ logger.error("Error setting MCP server logging level", error, {
+ serverId: body.serverId,
+ level: body.level,
+ });Or more simply, remove the context if it's not guaranteed to be available:
} catch (error) {
- logger.error("Error setting MCP server logging level", error, {
- serverId,
- level,
- });
+ logger.error("Error setting MCP server logging level", error);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.error("Error setting MCP server logging level", error, { | |
| serverId, | |
| level, | |
| }); | |
| logger.error("Error setting MCP server logging level", error); |
🤖 Prompt for AI Agents
In server/routes/mcp/log-level.ts around lines 65 to 68, the catch block logs a
context object using serverId and level that are destructured inside the try
block; if parsing or destructuring fails those variables will be undefined and
referencing them can throw a ReferenceError. Fix by ensuring safe access to
those values: declare serverId and level in the outer scope before the try and
assign inside the try, then in the catch pass a context object using safe
defaults (e.g. serverId ?? null, level ?? null) or only include the context if
the variables are defined; alternatively remove the context entirely from the
logger.error call if the values cannot be guaranteed.
| return c.json({ resourceTemplates: templates }); | ||
| } catch (error) { | ||
| console.error("Error fetching resource templates:", error); | ||
| logger.error("Error fetching resource templates", error, { serverId }); |
There was a problem hiding this comment.
Context parameter references potentially undefined variable.
If c.req.json() fails to parse, serverId will be undefined when passed to the logger context. Consider omitting the context when the variable is unavailable, or using { serverId: c.req.param("serverId") } if serverId can be reliably extracted from the route.
🤖 Prompt for AI Agents
In server/routes/mcp/resource-templates.ts around line 20, the logger call
passes a potentially undefined variable serverId into the context if
c.req.json() fails; change the logging to only include serverId when it's known
(e.g. extract a safe fallback with const serverId = c.req.param("serverId") ||
undefined) and then call logger.error with either logger.error("Error fetching
resource templates", error) when serverId is undefined or logger.error("Error
fetching resource templates", error, { serverId }) when it is defined, so the
context object is omitted unless serverId is available.
| }); | ||
| } catch (error) { | ||
| console.error("Error reconnecting server:", error); | ||
| logger.error("Error reconnecting server", error, { serverId }); |
There was a problem hiding this comment.
Context parameter references potentially undefined variable.
If c.req.json() fails to parse, serverId will be undefined when passed to the logger context. While the logger should handle this gracefully, consider extracting serverId from params as a fallback or omitting the context when unavailable.
🤖 Prompt for AI Agents
In server/routes/mcp/servers.ts around line 193, the logger call passes serverId
which may be undefined if c.req.json() fails; update the code to derive serverId
from the parsed body with a fallback to c.req.params.serverId (or another
request source) before logging, and only include the context object when
serverId is defined (or omit the context entirely on failure); ensure any
parsing is guarded so the logger receives a valid value or no context.
| logger.info(`✓ Created tunnel: ${this.baseUrl} -> ${addr}`); | ||
| return this.baseUrl; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if logger.ts exists and examine the shouldLog implementation
if [ -f "server/utils/logger.ts" ]; then
echo "=== Examining logger.ts ==="
head -100 server/utils/logger.ts
echo ""
echo "=== Looking for shouldLog and logger.info ==="
rg -n "shouldLog|logger\.info" server/utils/logger.ts -A 2 -B 2
else
echo "logger.ts not found at server/utils/logger.ts"
# Search for logger file
find . -name "logger*" -type f 2>/dev/null | head -20
fiRepository: MCPJam/inspector
Length of output: 2272
Consider making tunnel lifecycle messages user-visible by default, not gated by dev/verbose mode.
The shouldLog() guard restricts logger.info() output to dev mode or explicit --verbose flag. In typical production CLI usage (standard npx flow with NODE_ENV=production and no VERBOSE_LOGS), tunnel creation and closure messages won't be printed—users receive no feedback that tunnels were created/closed. This matters for UX feedback.
Options: Use console.log() directly for these messages, introduce a separate user-output channel, or explicitly document that tunnel logs require --verbose.
Applies to lines 55–56, 58–61, 73–76.
| import * as Sentry from "@sentry/node"; | ||
|
|
||
| const isVerbose = () => process.env.VERBOSE_LOGS === "true"; | ||
| const isDev = () => process.env.NODE_ENV !== "production"; | ||
| const shouldLog = () => isVerbose() || isDev(); | ||
|
|
||
| /** | ||
| * Centralized logger that sends errors to Sentry and only logs to console | ||
| * in dev mode or when verbose mode is enabled (--verbose flag or VERBOSE_LOGS=true). | ||
| */ | ||
| export const logger = { | ||
| /** | ||
| * Log an error. Always sends to Sentry, only prints to console in dev/verbose mode. | ||
| */ | ||
| error(message: string, error?: unknown, context?: Record<string, unknown>) { | ||
| Sentry.captureException(error ?? new Error(message), { | ||
| extra: { message, ...context }, | ||
| }); | ||
|
|
||
| if (shouldLog()) { | ||
| console.error(message, error); | ||
| } | ||
| }, | ||
|
|
||
| /** | ||
| * Log a warning. Always sends to Sentry, only prints to console in dev/verbose mode. | ||
| */ | ||
| warn(message: string, context?: Record<string, unknown>) { | ||
| Sentry.captureMessage(message, { level: "warning", extra: context }); | ||
|
|
||
| if (shouldLog()) { | ||
| console.warn(message); | ||
| } | ||
| }, | ||
|
|
||
| /** | ||
| * Log info. Only prints to console in dev/verbose mode. Does not send to Sentry. | ||
| */ | ||
| info(message: string) { | ||
| if (shouldLog()) { | ||
| console.log(message); | ||
| } | ||
| }, | ||
|
|
||
| /** | ||
| * Log debug info. Only prints to console in dev/verbose mode. Does not send to Sentry. | ||
| */ | ||
| debug(message: string, ...args: unknown[]) { | ||
| if (shouldLog()) { | ||
| console.log(message, ...args); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Sentry captureException Error vs string object normalization best practices
💡 Result:
Short answer — prefer passing real Error (or Error subclasses) to captureException; only use strings/objects for simple messages or when you also call captureMessage. That gives the SDK a real stacktrace and richer grouping/frames; the SDK will try to serialize arbitrary values but stack/metadata will be less accurate if you pass a string or arbitrary object.
Best-practice checklist
- Pass Error instances (or platform-native error types) to captureException so Sentry captures the original stack, type, and frames. This is the recommended approach in Sentry docs. [1][9]
- Don’t throw or capture raw literals (strings/numbers/null). Throwing/using Error objects preserves stack traces and debugging context. If you currently throw literals, convert them to Error or wrap them (e.g., new Error(msg)). [7]
- Use captureMessage for plain text messages (alerts/info) rather than captureException when there is no Error object. captureException is intended for exceptions. [4]
- If you must send non-Error values, attach useful context via hint / extra / tags / scope (don’t put everything into the exception payload). The hint parameter is an object — supply structured data there rather than a string. [10][1]
- Let the SDK normalize complex objects but avoid sending huge/deep objects; use SDK normalization utilities or trim objects first (depth/max properties) to avoid oversized events and loss of detail. See SDK normalization helpers (normalizeToSize, normalization options). [4]
- Use beforeSend to enrich, filter, or normalize events client-side (drop PII, convert non-standard objects into something Sentry can group, add fingerprints). [8]
- For third-party/network errors (e.g., browser network failures), keep in mind some native info isn’t exposed to JS; capture additional context where available (breadcrumbs, custom messages). [6]
Relevant references
- Sentry captureException / SDK guidance (prefer Error objects; accepts unknown but serializes best when Error passed). [1]
- Sentry SDK normalization utilities and normalizeToSize docs. [4]
- Sentry filtering / beforeSend (attach/modify events). [8]
- Note on throwing literals / best-practice to throw Error objects. [7]
- Hint parameter is structured (use object) — example and discussion. [10]
If you want, tell me your platform (browser/Node/React Native/Swift/etc.) and I’ll give a tiny code example showing the recommended pattern for captureException + attaching context.
🌐 Web query:
Sentry.captureException documentation error handling stack traces grouping
💡 Result:
Summary of how Sentry.captureException affects stack traces and grouping — key points and fixes:
- captureException behavior
- Best practice: pass an actual Error (or language-native exception) so the SDK captures the original stack trace. If you pass arbitrary values (string/object), the SDK will serialize it and may generate a synthetic stack trace (less accurate). [1]
- Exception payload / mechanism
- The Exception Interface supports a mechanism object. For user-invoked captures (e.g., captureException), mechanism.type should be "generic". SDKs may set mechanism.handled/ synthetic flags; marking synthetic helps Sentry avoid incorrect grouping for artificially-added stacks. [2]
- How grouping is determined
- Sentry’s default grouping algorithm orders signals as: stack trace first, exception second, message last. That means whether an event has a stacktrace (or how it looks) is the primary factor for grouping. Changing whether a stacktrace is attached can create new groups. [3]
- SDK options that affect stack traces and grouping
- attach_stacktrace (or attachStacktrace): when enabled the SDK attaches a stacktrace to message events (and can change grouping results). Turning this on/off may split or merge existing groups. Some SDKs also offer add_full_stack / max_stack_frames to include more frames which also affects grouping. [4][5]
- Practical recommendations
- Always pass Error objects to captureException when possible.
- If you must capture non-Error values, consider adding a clear fingerprint or explicit fingerprint array to control grouping.
- Use the mechanism object (where supported) to mark captures appropriately (and allow SDKs to set synthetic when they generated stacks).
- If grouping is wrong for your project, use project Issue Grouping rules or SDK fingerprinting to customize grouping (or adjust attach_stacktrace/add_full_stack carefully). [3][4]
Sources:
- Sentry SDK captureException docs (JS) — captureException accepts any value; Error objects give best results. [1]
- Sentry data model: Exception Interface (mechanism, synthetic, ids). [2]
- Sentry Help: Why events are grouped or separated; grouping precedence (stacktrace → exception → message). [3]
- Sentry SDK options: attach_stacktrace / add_full_stack (Node/Python docs). [4][5]
If you want, tell me which SDK/language you use (JavaScript, Python, Node, etc.) and I’ll give exact code examples and recommended config.
🏁 Script executed:
cat -n server/utils/logger.ts | head -52Repository: MCPJam/inspector
Length of output: 1887
Normalize non-Error throwables before sending to Sentry for accurate stack traces and grouping.
captureException with strings or objects degrades Sentry's grouping signal; convert to Error instances to preserve real stack traces. Attach the original value to extra for context.
import * as Sentry from "@sentry/node";
const isVerbose = () => process.env.VERBOSE_LOGS === "true";
const isDev = () => process.env.NODE_ENV !== "production";
const shouldLog = () => isVerbose() || isDev();
+function toError(error: unknown, fallbackMessage: string): Error {
+ if (error instanceof Error) return error;
+ if (typeof error === "string") return new Error(error);
+ try {
+ return new Error(JSON.stringify(error));
+ } catch {
+ return new Error(fallbackMessage);
+ }
+}
+
export const logger = {
error(message: string, error?: unknown, context?: Record<string, unknown>) {
- Sentry.captureException(error ?? new Error(message), {
- extra: { message, ...context },
+ const err = toError(error, message);
+ Sentry.captureException(err, {
+ extra: { message, originalError: error, ...context },
});
if (shouldLog()) {
- console.error(message, error);
+ console.error(message, err);
}
},🤖 Prompt for AI Agents
In server/utils/logger.ts around lines 1 to 52, the logger.error currently
passes non-Error throwables (strings/objects) directly to Sentry which harms
grouping and stack traces; update it to normalize the throwable into an Error
instance before calling Sentry.captureException (e.g., if the provided error is
not instanceof Error, create a new Error(String(error)) and attach the original
value to the extra/context payload such as extra.originalError), then call
captureException with that Error and keep console.error printing both the
normalized Error and the original value for context.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
server/index.ts (1)
244-267: Inline<script>config injection remains XSS-prone (</script>breakout). Escape</>/&(or at least</script>) before embedding JSON. This was already raised previously for this same pattern.+function safeJsonForInlineScript(value: unknown) { + return JSON.stringify(value) + .replace(/</g, "\\u003c") + .replace(/>/g, "\\u003e") + .replace(/&/g, "\\u0026"); +} + const mcpConfig = getMCPConfigFromEnv(); if (mcpConfig) { - const configScript = `<script>window.MCP_CLI_CONFIG = ${JSON.stringify(mcpConfig)};</script>`; + const configScript = `<script>window.MCP_CLI_CONFIG = ${safeJsonForInlineScript(mcpConfig)};</script>`; htmlContent = htmlContent.replace("</head>", `${configScript}</head>`); }
🧹 Nitpick comments (3)
server/index.ts (3)
96-98: Prefer structured context over “message with colon”.appLogger.error("Failed to parse MCP_CONFIG_DATA", error, { envVar: "MCP_CONFIG_DATA" })is easier to search/aggregate.- } catch (error) { - appLogger.error("Failed to parse MCP_CONFIG_DATA:", error); - } + } catch (error) { + appLogger.error("Failed to parse MCP_CONFIG_DATA", error, { + envVar: "MCP_CONFIG_DATA", + }); + }
135-144: Confirm whether 4xxHTTPExceptions should page Sentry. Right now, anyHTTPExceptionwill go throughappLogger.error(...)→ Sentry. If 401/403/404 are expected, consider only capturing 5xx (or non-HTTPException).const app = new Hono().onError((err, c) => { - appLogger.error("Unhandled error:", err); - // Return appropriate response if (err instanceof HTTPException) { + // Avoid noisy Sentry events for expected 4xx (tune as desired). + if (err.status >= 500) appLogger.error("Unhandled HTTPException", err); + else appLogger.debug("HTTPException", { status: err.status, message: err.message }); return err.getResponse(); } + appLogger.error("Unhandled error", err); return c.json({ error: "Internal server error" }, 500); });
278-287: Nice UX touch on the banner title. One thought:logBoxusesconsole.logdirectly, so it won’t honor “quiet by default” if that’s intended for server output too—worth aligning with the new logging policy if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/index.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
server/**/{app,index,routes/**}.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Use Hono.js for API routing and middleware in the backend
Files:
server/index.ts
server/**/*.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Implement TypeScript for type safety throughout the codebase
Files:
server/index.ts
🧠 Learnings (7)
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/servers/**/*.{ts,tsx} : Synchronize MCP connection state, request tracking, response handling, and error management with global state
Applied to files:
server/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/mcp.ts : Define MCP protocol types in TypeScript type definition files
Applied to files:
server/index.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Implement MCP server management UI with connection list, status indicators, quick actions, and group management
Applied to files:
server/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/*.ts : Implement TypeScript for type safety throughout the codebase
Applied to files:
server/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/routes/mcp/prompts/**/*.ts : Validate prompt context, token limits, and format in prompt processing handlers
Applied to files:
server/index.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/{app,index,routes/**}.ts : Use Hono.js for API routing and middleware in the backend
Applied to files:
server/index.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Use Vite + React for fast development and optimized builds
Applied to files:
server/index.ts
🧬 Code graph analysis (1)
server/index.ts (2)
server/utils/logger.ts (2)
error(15-23)logger(11-53)server/config.ts (1)
SERVER_HOSTNAME(11-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
server/index.ts (2)
10-12: Centralized logger wiring looks clean.
199-205: No action needed—Hono'slogger()is safe by default. The middleware logs only method, path, status code, and request timing; it does not log headers, cookies, or request/response bodies. Your current implementation poses no risk of leaking secrets or PII.Likely an incorrect or invalid review comment.
Note
Introduce a centralized logger with opt-in verbose output, propagate via CLI/launcher/Vite, replace console.* across server routes/services, and document usage.
server/utils/loggerthat sends errors/warnings to Sentry and respectsVERBOSE_LOGS.console.*withlogger.*across server routes/services (server/routes/**,server/services/**,server/index.ts,server/app.ts).logger()only in dev or whenVERBOSE_LOGS=true.bin/start.js)--verbose/-vflag andverbose*helpers; passVERBOSE_LOGSto server env.setLoggingLevel('debug')and ignore unsupported errors.verbose?: booleanoption inlib/launcherandlib/vite; propagate to env.docs/installation.mdx.AGENTS.mdwith logging guidance; removeclient/CLAUDE.mdandserver/CLAUDE.md.🎵 MCPJam; minor log message tweaks.Written by Cursor Bugbot for commit a679901. This will update automatically on new commits. Configure here.