fix(mcp): structured unknown-tool errors, successful empty search, and test coverage (#414, #415, #421)

.changeset/mcp-error-handling-and-tests.md

@@ -0,0 +1,8 @@
+---
+'lens': patch
+---
+
+Structured error handling and comprehensive test coverage for MCP server:
+- Handle empty search results as a success returning `{ count: 0, resources: [] }` rather than an error (resolves #414)
+- Return structured `ERR_UNKNOWN_TOOL` with valid tool suggestions and bounded/sanitized names instead of throwing raw exceptions (resolves #415)
+- Add comprehensive Vitest test suite exercising ListTools schema validation, bazaar_search execution paths (query, default, empty, non-2xx, malformed), and realistic upstream SDK errors (resolves #421)

src/mcp/codes.ts

@@ -6,14 +6,10 @@ export const MCP_ERROR_CODES = {
   ERR_SERVICE_UNREACHABLE: 'ERR_SERVICE_UNREACHABLE',
   ERR_NO_RESULTS: 'ERR_NO_RESULTS',
   ERR_BAD_REQUEST: 'ERR_BAD_REQUEST',
-  // Refused before any request was made, for two different reasons:
-  //   ERR_URL_BLOCKED      the target resolves somewhere we will never fetch
-  //                        (private, loopback, link-local, metadata) — #488
-  //   ERR_HOST_NOT_ALLOWED the host is reachable but is not on the operator's
-  //                        paid_call allow-list — #487
   ERR_URL_BLOCKED: 'ERR_URL_BLOCKED',
   ERR_HOST_NOT_ALLOWED: 'ERR_HOST_NOT_ALLOWED',
-  ERR_INTERNAL_FAILURE: 'ERR_INTERNAL_FAILURE'
+  ERR_INTERNAL_FAILURE: 'ERR_INTERNAL_FAILURE',
+  ERR_UNKNOWN_TOOL: 'ERR_UNKNOWN_TOOL'
 } as const;
 
 export type McpErrorCode = keyof typeof MCP_ERROR_CODES;

src/mcp/server.ts

@@ -7,6 +7,9 @@ import { fetchWithTimeout } from '../fetch.js';
 import { config } from '../config.js';
 import { ssrfSafeFetch, SsrfError, type SsrfFetchOptions } from '../net/ssrf.js';
 
+export const VALID_MCP_TOOLS = ['bazaar_search', 'paid_call'] as const;
+export type ValidMcpToolName = typeof VALID_MCP_TOOLS[number];
+
 export interface McpServerConfig {
   fetchWithPayment: typeof fetch;
   /**
@@ -111,79 +114,104 @@ export class BazaarMcpServer {
     }));
 
     this.server.setRequestHandler(CallToolRequestSchema, async (request) => {
-      if (request.params.name === 'bazaar_search') {
-        const url = String(request.params.arguments?.url);
-        const query = request.params.arguments?.query ? String(request.params.arguments?.query) : undefined;
-
-        try {
-          const endpoint = query 
-            ? `${url.replace(/\/$/, '')}/discovery/search?q=${encodeURIComponent(query)}`
-            : `${url.replace(/\/$/, '')}/discovery/resources`;
-
-          const res = await ssrfSafeFetch(endpoint, {}, this.bazaarFetchOptions);
-
-          if (!res.ok) {
-            return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', `Service returned ${res.status}`);
-          }
-
-          const data = await res.json();
-
-          const hasResources = (d: unknown): d is { resources: unknown[] } => 
-            typeof d === 'object' && d !== null && 'resources' in d && Array.isArray((d as Record<string, unknown>).resources);
-
-          if ((Array.isArray(data) && data.length === 0) || (hasResources(data) && data.resources.length === 0)) {
-            return this.createErrorResponse('ERR_NO_RESULTS', 'No resources found');
-          }
-
-          return {
-            content: [{ type: 'text', text: JSON.stringify(data, null, 2) }],
-          };
-        } catch (e: any) {
-          if (e instanceof SsrfError) {
-            return this.createErrorResponse('ERR_URL_BLOCKED', `Blocked request: ${e.message}`);
-          }
-          return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', e.message);
-        }
-      }
+      try {
+        if (request.params.name === 'bazaar_search') {
+          const url = String(request.params.arguments?.url);
+          const query = request.params.arguments?.query ? String(request.params.arguments?.query) : undefined;
+
+          try {
+            const endpoint = query
+              ? `${url.replace(/\/$/, '')}/discovery/search?q=${encodeURIComponent(query)}`
+              : `${url.replace(/\/$/, '')}/discovery/resources`;
 
-      if (request.params.name === 'paid_call') {
-        const url = String(request.params.arguments?.url);
-        const method = request.params.arguments?.method ? String(request.params.arguments?.method) : 'GET';
+            const res = await ssrfSafeFetch(endpoint, {}, this.bazaarFetchOptions);
 
-        // Gate before the fetch: fetchWithPayment answers a 402 by paying, so
-        // by the time a request leaves this process the money is already spent.
-        const target = checkPaidCallTarget(url, this.paidCallRules);
-        if (!target.allowed) {
-          return this.createErrorResponse(target.code, target.reason);
-        }
+            if (!res.ok) {
+              return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', `Service returned ${res.status}`);
+            }
+
+            const data = await res.json();
+
+            const hasResources = (d: unknown): d is { resources: unknown[] } =>
+              typeof d === 'object' && d !== null && 'resources' in d && Array.isArray((d as Record<string, unknown>).resources);
+
+            // Empty results return success with empty array instead of error (Issue #414)
+            if (Array.isArray(data) && data.length === 0) {
+              return {
+                content: [{ type: 'text', text: JSON.stringify({ count: 0, resources: [] }, null, 2) }],
+              };
+            }
 
-        try {
-          const res = await this.fetchWithPayment(url, { method });
-
-          if (!res.ok) {
-            if (res.status === 402) {
-              return this.createErrorResponse('ERR_PAYMENT_REJECTED', 'Payment required but was rejected or failed.');
+            if (hasResources(data) && data.resources.length === 0) {
+              const emptyResult = {
+                count: 0,
+                resources: [],
+                ...(typeof data === 'object' && data !== null ? data : {}),
+              };
+              emptyResult.count = 0;
+              emptyResult.resources = [];
+              return {
+                content: [{ type: 'text', text: JSON.stringify(emptyResult, null, 2) }],
+              };
             }
-            return this.createErrorResponse('ERR_BAD_REQUEST', `Request failed with status ${res.status}`);
+
+            return {
+              content: [{ type: 'text', text: JSON.stringify(data, null, 2) }],
+            };
+          } catch (e: any) {
+            if (e instanceof SsrfError) {
+              return this.createErrorResponse('ERR_URL_BLOCKED', `Blocked request: ${e.message}`);
+            }
+            return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', e.message);
           }
-
-          const text = await res.text();
-          let parsed;
+        }
+
+        if (request.params.name === 'paid_call') {
+          const url = String(request.params.arguments?.url);
+          const method = request.params.arguments?.method ? String(request.params.arguments?.method) : 'GET';
+
+          // Gate before the fetch: check allowlist
+          const target = checkPaidCallTarget(url, this.paidCallRules);
+          if (!target.allowed) {
+            return this.createErrorResponse(target.code, target.reason);
+          }
+
           try {
-            parsed = JSON.parse(text);
-          } catch {
-            parsed = text;
+            const res = await this.fetchWithPayment(url, { method });
+
+            if (!res.ok) {
+              if (res.status === 402) {
+                return this.createErrorResponse('ERR_PAYMENT_REJECTED', 'Payment required but was rejected or failed.');
+              }
+              return this.createErrorResponse('ERR_BAD_REQUEST', `Request failed with status ${res.status}`);
+            }
+
+            const text = await res.text();
+            let parsed;
+            try {
+              parsed = JSON.parse(text);
+            } catch {
+              parsed = text;
+            }
+
+            return {
+              content: [{ type: 'text', text: typeof parsed === 'string' ? parsed : JSON.stringify(parsed, null, 2) }],
+            };
+          } catch (e: any) {
+            return this.handlePaidCallError(e);
           }
-
-          return {
-            content: [{ type: 'text', text: typeof parsed === 'string' ? parsed : JSON.stringify(parsed, null, 2) }],
-          };
-        } catch (e: any) {
-          return this.handlePaidCallError(e);
         }
-      }
 
-      throw new Error(`Unknown tool: ${request.params.name}`);
+        // Return structured schema error for unknown tool (Issue #415)
+        const rawName = String(request.params?.name ?? '');
+        const sanitizedName = rawName.replace(/[\x00-\x1F\x7F]/g, '').slice(0, 64);
+        return this.createErrorResponse(
+          'ERR_UNKNOWN_TOOL',
+          `Unknown tool "${sanitizedName}". Valid tools are: ${VALID_MCP_TOOLS.join(', ')}`
+        );
+      } catch (err: any) {
+        return this.createErrorResponse('ERR_INTERNAL_FAILURE', err?.message || 'An internal error occurred during tool execution.');
+      }
     });
   }