feat: provide more informative validate error messages for tool calls

CodeWithKyrian · CodeWithKyrian · commit ca6d25a46d5e · 2025-05-10T12:18:57.000+01:00
- Enhanced error handling in Processor to provide more informative validation error messages.
- Changed server listening address from '0.0.0.0' to '127.0.0.1' in example server scripts for improved clarity and compatibility.
- Refined logging messages in HttpServerTransport for consistency and clarity.
diff --git a/examples/04-combined-registration-http/server.php b/examples/04-combined-registration-http/server.php
@@ -19,7 +19,7 @@
     |
     | To Use:
     | 1. Run this script from your CLI: `php server.php`
-    |    The server will listen on http://0.0.0.0:8081 by default.
+    |    The server will listen on http://127.0.0.1:8081 by default.
     | 2. Configure your MCP Client (e.g., Cursor):
     |
     | {
@@ -80,7 +80,7 @@ public function log($level, \Stringable|string $message, array $context = []): v
     // If 'config://priority' was discovered, the manual one takes precedence.
     $server->discover(__DIR__, scanDirs: ['.']);
 
-    $transport = new HttpServerTransport('0.0.0.0', 8081, 'mcp_combined');
+    $transport = new HttpServerTransport('127.0.0.1', 8081, 'mcp_combined');
 
     $server->listen($transport);
 
diff --git a/examples/07-complex-tool-schema-http/server.php b/examples/07-complex-tool-schema-http/server.php
@@ -82,7 +82,7 @@ public function log($level, \Stringable|string $message, array $context = []): v
 
     $server->discover(__DIR__, ['.']);
 
-    $transport = new HttpServerTransport('120.0.0.1', 8082, 'mcp_scheduler');
+    $transport = new HttpServerTransport('127.0.0.1', 8082, 'mcp_scheduler');
     $server->listen($transport);
 
     $logger->info('Server listener stopped gracefully.');
diff --git a/src/Processor.php b/src/Processor.php
@@ -299,18 +299,33 @@ private function handleToolCall(array $params): CallToolResult
 
         $definition = $this->registry->findTool($toolName);
         if (! $definition) {
-            throw McpServerException::methodNotFound("Tool '{$toolName}' not found."); // Method not found seems appropriate
+            throw McpServerException::methodNotFound("Tool '{$toolName}' not found.");
         }
 
         $inputSchema = $definition->getInputSchema();
         $argumentsForValidation = is_object($argumentsRaw) ? (array) $argumentsRaw : $argumentsRaw;
 
         $validationErrors = $this->schemaValidator->validateAgainstJsonSchema($argumentsForValidation, $inputSchema);
+
         if (! empty($validationErrors)) {
-            throw McpServerException::invalidParams(data: ['validation_errors' => $validationErrors]);
+            $errorMessages = [];
+
+            foreach ($validationErrors as $errorDetail) {
+                $pointer = $errorDetail['pointer'] ?? '';
+                $message = $errorDetail['message'] ?? 'Unknown validation error';
+                $errorMessages[] = ($pointer !== '/' && $pointer !== '' ? "Property '{$pointer}': " : '').$message;
+            }
+
+            $summaryMessage = "Invalid parameters for tool '{$toolName}': ".implode('; ', array_slice($errorMessages, 0, 3));
+
+            if (count($errorMessages) > 3) {
+                $summaryMessage .= '; ...and more errors.';
+            }
+
+            throw McpServerException::invalidParams($summaryMessage, data: ['validation_errors' => $validationErrors]);
         }
 
-        $argumentsForPhpCall = (array) $argumentsRaw; // Need array for ArgumentPreparer
+        $argumentsForPhpCall = (array) $argumentsRaw;
 
         try {
             $instance = $this->container->get($definition->getClassName());
diff --git a/src/Support/SchemaValidator.php b/src/Support/SchemaValidator.php
@@ -2,19 +2,20 @@
 
 namespace PhpMcp\Server\Support;
 
+use InvalidArgumentException;
 use JsonException;
 use Opis\JsonSchema\Errors\ValidationError;
 use Opis\JsonSchema\Validator;
 use Psr\Log\LoggerInterface;
 use Throwable;
-use InvalidArgumentException;
 
 /**
  * Validates data against JSON Schema definitions using opis/json-schema.
  */
 class SchemaValidator
 {
     private ?Validator $jsonSchemaValidator = null;
+
     private LoggerInterface $logger;
 
     public function __construct(LoggerInterface $logger)
@@ -50,12 +51,15 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar
 
         } catch (JsonException $e) {
             $this->logger->error('MCP SDK: Invalid schema structure provided for validation (JSON conversion failed).', ['exception' => $e]);
+
             return [['pointer' => '', 'keyword' => 'internal', 'message' => 'Invalid schema definition provided (JSON error).']];
         } catch (InvalidArgumentException $e) {
             $this->logger->error('MCP SDK: Invalid schema structure provided for validation.', ['exception' => $e]);
+
             return [['pointer' => '', 'keyword' => 'internal', 'message' => $e->getMessage()]];
         } catch (Throwable $e) {
             $this->logger->error('MCP SDK: Error preparing data/schema for validation.', ['exception' => $e]);
+
             return [['pointer' => '', 'keyword' => 'internal', 'message' => 'Internal validation preparation error.']];
         }
 
@@ -70,6 +74,7 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar
                 'data' => json_encode($dataToValidate),
                 'schema' => json_encode($schemaObject),
             ]);
+
             return [['pointer' => '', 'keyword' => 'internal', 'message' => 'Schema validation process failed: '.$e->getMessage()]];
         }
 
@@ -101,9 +106,10 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar
     private function getJsonSchemaValidator(): Validator
     {
         if ($this->jsonSchemaValidator === null) {
-            $this->jsonSchemaValidator = new Validator();
+            $this->jsonSchemaValidator = new Validator;
             // Potentially configure resolver here if needed later
         }
+
         return $this->jsonSchemaValidator;
     }
 
@@ -114,24 +120,27 @@ private function convertDataForValidator(mixed $data): mixed
     {
         if (is_array($data)) {
             // Check if it's an associative array (keys are not sequential numbers 0..N-1)
-            if (!empty($data) && array_keys($data) !== range(0, count($data) - 1)) {
-                $obj = new \stdClass();
+            if (! empty($data) && array_keys($data) !== range(0, count($data) - 1)) {
+                $obj = new \stdClass;
                 foreach ($data as $key => $value) {
                     $obj->{$key} = $this->convertDataForValidator($value);
                 }
+
                 return $obj;
             } else {
                 // It's a list (sequential array), convert items recursively
                 return array_map([$this, 'convertDataForValidator'], $data);
             }
         } elseif (is_object($data) && $data instanceof \stdClass) {
             // Deep copy/convert stdClass objects as well
-            $obj = new \stdClass();
+            $obj = new \stdClass;
             foreach (get_object_vars($data) as $key => $value) {
                 $obj->{$key} = $this->convertDataForValidator($value);
             }
+
             return $obj;
         }
+
         // Leave other objects and scalar types as they are
         return $data;
     }
@@ -165,8 +174,10 @@ private function formatJsonPointerPath(?array $pathComponents): string
         }
         $escapedComponents = array_map(function ($component) {
             $componentStr = (string) $component;
+
             return str_replace(['~', '/'], ['~0', '~1'], $componentStr);
         }, $pathComponents);
+
         return '/'.implode('/', $escapedComponents);
     }
 
@@ -213,6 +224,7 @@ private function formatValidationError(ValidationError $error): string
                         if ($v === null) {
                             return 'null';
                         }
+
                         return (string) $v;
                     }, $allowedValues);
                     $message = 'Value must be one of the allowed values: '.implode(', ', $formattedAllowed).'.';
@@ -289,12 +301,14 @@ private function formatValidationError(ValidationError $error): string
                     $builtInMessage = preg_replace_callback('/\{(\w+)\}/', function ($match) use ($placeholders) {
                         $key = $match[1];
                         $value = $placeholders[$key] ?? '{'.$key.'}';
+
                         return is_array($value) ? json_encode($value) : (string) $value;
                     }, $builtInMessage);
                     $message = $builtInMessage;
                 }
                 break;
         }
+
         return $message;
     }
 }
diff --git a/src/Transports/HttpServerTransport.php b/src/Transports/HttpServerTransport.php
@@ -65,7 +65,7 @@ public function __construct(
         private readonly string $mcpPathPrefix = 'mcp', // e.g., /mcp/sse, /mcp/message
         private readonly ?array $sslContext = null // For enabling HTTPS
     ) {
-        $this->logger = new NullLogger();
+        $this->logger = new NullLogger;
         $this->loop = Loop::get();
         $this->ssePath = '/'.trim($mcpPathPrefix, '/').'/sse';
         $this->messagePath = '/'.trim($mcpPathPrefix, '/').'/message';
@@ -109,7 +109,7 @@ public function listen(): void
             $this->http->listen($this->socket);
 
             $this->socket->on('error', function (Throwable $error) {
-                $this->logger->error('HttpTransport: Socket server error.', ['error' => $error->getMessage()]);
+                $this->logger->error('Socket server error.', ['error' => $error->getMessage()]);
                 $this->emit('error', [new TransportException("Socket server error: {$error->getMessage()}", 0, $error)]);
                 $this->close();
             });
@@ -159,7 +159,7 @@ private function handleSseRequest(ServerRequestInterface $request): Response
         $clientId = 'sse_'.bin2hex(random_bytes(16));
         $this->logger->info('New SSE connection', ['clientId' => $clientId]);
 
-        $sseStream = new ThroughStream();
+        $sseStream = new ThroughStream;
 
         $sseStream->on('close', function () use ($clientId) {
             $this->logger->info('SSE stream closed', ['clientId' => $clientId]);
@@ -216,13 +216,13 @@ private function handleMessagePostRequest(ServerRequestInterface $request): Resp
         $clientId = $queryParams['clientId'] ?? null;
 
         if (! $clientId || ! is_string($clientId)) {
-            $this->logger->warning('HttpTransport: Received POST without valid clientId query parameter.');
+            $this->logger->warning('Received POST without valid clientId query parameter.');
 
             return new Response(400, ['Content-Type' => 'text/plain'], 'Missing or invalid clientId query parameter');
         }
 
         if (! isset($this->activeSseStreams[$clientId])) {
-            $this->logger->warning('HttpTransport: Received POST for unknown or disconnected clientId.', ['clientId' => $clientId]);
+            $this->logger->warning('Received POST for unknown or disconnected clientId.', ['clientId' => $clientId]);
 
             return new Response(404, ['Content-Type' => 'text/plain'], 'Client ID not found or disconnected');
         }
@@ -234,7 +234,7 @@ private function handleMessagePostRequest(ServerRequestInterface $request): Resp
         $body = $request->getBody()->getContents();
 
         if (empty($body)) {
-            $this->logger->warning('HttpTransport: Received empty POST body', ['clientId' => $clientId]);
+            $this->logger->warning('Received empty POST body', ['clientId' => $clientId]);
 
             return new Response(400, ['Content-Type' => 'text/plain'], 'Empty request body');
         }
@@ -265,16 +265,15 @@ public function sendToClientAsync(string $clientId, string $rawFramedMessage): P
             return \React\Promise\resolve(null);
         }
 
-        $deferred = new Deferred();
+        $deferred = new Deferred;
         $written = $this->sendSseEvent($stream, 'message', $jsonData);
 
         if ($written) {
-            $this->logger->debug('HttpTransport: Message sent via SSE.', ['clientId' => $clientId, 'data' => $jsonData]);
             $deferred->resolve(null);
         } else {
-            $this->logger->debug('HttpTransport: SSE stream buffer full, waiting for drain.', ['clientId' => $clientId]);
+            $this->logger->debug('SSE stream buffer full, waiting for drain.', ['clientId' => $clientId]);
             $stream->once('drain', function () use ($deferred, $clientId) {
-                $this->logger->debug('HttpTransport: SSE stream drained.', ['clientId' => $clientId]);
+                $this->logger->debug('SSE stream drained.', ['clientId' => $clientId]);
                 $deferred->resolve(null);
             });
             // Add a timeout?
@@ -316,7 +315,7 @@ public function close(): void
         }
         $this->closing = true;
         $this->listening = false;
-        $this->logger->info('HttpTransport: Closing...');
+        $this->logger->info('Closing transport...');
 
         if ($this->socket) {
             $this->socket->close();
@@ -326,7 +325,7 @@ public function close(): void
         $activeStreams = $this->activeSseStreams;
         $this->activeSseStreams = [];
         foreach ($activeStreams as $clientId => $stream) {
-            $this->logger->debug('HttpTransport: Closing active SSE stream', ['clientId' => $clientId]);
+            $this->logger->debug('Closing active SSE stream', ['clientId' => $clientId]);
             unset($this->activeSseStreams[$clientId]);
             $stream->close();
         }

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@`
`19`	`19`	`\|`
`20`	`20`	`\| To Use:`
`21`	`21`	\| 1. Run this script from your CLI: `php server.php`
`22`		`- \| The server will listen on http://0.0.0.0:8081 by default.`
	`22`	`+ \| The server will listen on http://127.0.0.1:8081 by default.`
`23`	`23`	`\| 2. Configure your MCP Client (e.g., Cursor):`
`24`	`24`	`\|`
`25`	`25`	`\| {`
`@@ -80,7 +80,7 @@ public function log($level, \Stringable\|string $message, array $context = []): v`
`80`	`80`	`// If 'config://priority' was discovered, the manual one takes precedence.`
`81`	`81`	`$server->discover(__DIR__, scanDirs: ['.']);`
`82`	`82`
`83`		`- $transport = new HttpServerTransport('0.0.0.0', 8081, 'mcp_combined');`
	`83`	`+ $transport = new HttpServerTransport('127.0.0.1', 8081, 'mcp_combined');`
`84`	`84`
`85`	`85`	`$server->listen($transport);`
`86`	`86`