Conversation
2 tasks
Shikha-code36
added a commit
to Shikha-code36/monitor
that referenced
this pull request
Aug 21, 2026
Address review: mapMcpError collapsed the per-call log message and the client-facing fallback into one string, so restoring the real status code for BetterDB-inc#155 lost the connection id from every log line. Give mapMcpError an optional logMessage param (defaults to fallback, so the other 8 existing callers are unaffected) and pass id-bearing log messages from all 19 mcp.controller.ts call sites. Also ports BetterDB-inc#204's tests, which exercise the actual registry.get() NotFoundException path instead of a mocked service method, plus a new test asserting the id survives in the log line.
KIvanow
pushed a commit
that referenced
this pull request
Aug 30, 2026
) * fix(mcp): propagate real HTTP status instead of generic 500 (#155) mcp.controller.ts caught every error and rethrew a hardcoded 500, discarding the NotFoundException that ConnectionRegistry.get() already throws for a non-existent instanceId override. Switch to mapMcpError, the helper already used by the sibling mcp-analytics/mcp-ai/vector-search controllers, so a bad instanceId now surfaces as 404 as expected. * fix(mcp): preserve error id in logs, keep client message generic Address review: mapMcpError collapsed the per-call log message and the client-facing fallback into one string, so restoring the real status code for #155 lost the connection id from every log line. Give mapMcpError an optional logMessage param (defaults to fallback, so the other 8 existing callers are unaffected) and pass id-bearing log messages from all 19 mcp.controller.ts call sites. Also ports #204's tests, which exercise the actual registry.get() NotFoundException path instead of a mocked service method, plus a new test asserting the id survives in the log line. * fix(mcp): stop leaking internal error text to MCP clients mapMcpError appended ': \' to the client-facing HttpException for any non-HTTP error, so an arbitrary internal failure (connection string, query detail, etc.) could reach an authenticated MCP caller verbatim. Return only the generic fallback to the client; the real error still goes to logger.error via logMessage/stack. Shared by mcp.controller, mcp-analytics, mcp-ai, and vector-search — verified no other test asserted on the old suffix text. * fix(client-analytics): include connectionId in snapshot fetch failure log getActivityTimeline already receives connectionId but the catch block around getClientSnapshots logged without it, so a snapshot-fetch failure on this one path was unattributable on a multi-connection deployment. Same gap flagged by jamby77 in #405 review.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HttpExceptions thrown inside MCP controller handlers instead of wrapping them as generic 500sFixes #155.
Verification
git diff --checkpasses with only Windows LF-to-CRLF warningsSKIP_DOCKER_SETUP=true corepack pnpm --filter api test -- src/mcp/__tests__/mcp.controller.spec.tsis blocked in this local checkout before this test runs because Jest/ts-jest resolves../../proprietary/licenses/license.module.tsand cannot resolve@nestjs/common/@nestjs/configcorepack pnpm --filter api buildis blocked by the same proprietary workspace dependency resolution problem before reaching this change