Skip to content

Commit 7a4d8c2

Browse files
fix: standardize error handling across handlers with MCP specification compliance
1 parent f41ea86 commit 7a4d8c2

File tree

8 files changed

+133
-38
lines changed

8 files changed

+133
-38
lines changed

docs/mcp-elements.md

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -154,31 +154,37 @@ public function getMultipleContent(): array
154154

155155
#### Error Handling
156156

157-
Tools can throw exceptions which are automatically converted to proper JSON-RPC error responses:
157+
**Tools should ONLY throw `ToolCallException` for any error that occurs during tool execution.**
158158

159159
```php
160+
use Mcp\Exception\ToolCallException;
161+
160162
#[McpTool]
161163
public function divideNumbers(float $a, float $b): float
162164
{
163165
if ($b === 0.0) {
164-
throw new \InvalidArgumentException('Division by zero is not allowed');
166+
throw new ToolCallException('Division by zero is not allowed');
165167
}
166-
168+
167169
return $a / $b;
168170
}
169171

170172
#[McpTool]
171173
public function processFile(string $filename): string
172174
{
173175
if (!file_exists($filename)) {
174-
throw new \InvalidArgumentException("File not found: {$filename}");
176+
throw new ToolCallException("File not found: {$filename}");
175177
}
176-
178+
177179
return file_get_contents($filename);
178180
}
179181
```
180182

181-
The SDK will convert these exceptions into appropriate JSON-RPC error responses that MCP clients can understand.
183+
**Critical Rule:** For tool handlers, **ALWAYS** throw `ToolCallException` for any error condition - validation errors, file not found, processing errors, etc. Do not use generic PHP exceptions.
184+
185+
**Error Handling Behavior:**
186+
- **`ToolCallException`**: Converted to `CallToolResult` with `isError: true`, allowing the LLM to see the error message and self-correct
187+
- **Any other exception**: Treated as internal server error and returns a generic error message to client
182188

183189
## Resources
184190

@@ -298,24 +304,33 @@ public function getMultipleResources(): array
298304

299305
#### Error Handling
300306

301-
Resource handlers can throw exceptions for error cases:
307+
**Resource handlers should ONLY throw `ResourceReadException` for any error that occurs during resource reading.**
302308

303309
```php
310+
use Mcp\Exception\ResourceReadException;
311+
304312
#[McpResource(uri: 'file://{path}')]
305313
public function getFile(string $path): string
306314
{
307315
if (!file_exists($path)) {
308-
throw new \InvalidArgumentException("File not found: {$path}");
316+
throw new ResourceReadException("File not found: {$path}");
309317
}
310-
318+
311319
if (!is_readable($path)) {
312-
throw new \RuntimeException("File not readable: {$path}");
320+
throw new ResourceReadException("File not readable: {$path}");
313321
}
314-
322+
315323
return file_get_contents($path);
316324
}
317325
```
318326

327+
**Critical Rule:** For resource handlers, **ALWAYS** throw `ResourceReadException` for any error condition - file not found, permission errors, data format issues, etc. Do not use generic PHP exceptions.
328+
329+
**Error Handling Behavior:**
330+
- **`ResourceReadException`**: Converted to JSON-RPC error response with the actual exception message
331+
- **Any other exception**: Treated as internal server error and returns a generic error message to client
332+
333+
319334
## Resource Templates
320335

321336
Resource templates are **dynamic resources** that use parameterized URIs with variables. They follow all the same rules
@@ -456,26 +471,34 @@ public function explicitMessages(): array
456471

457472
#### Error Handling
458473

459-
Prompt handlers can throw exceptions for invalid inputs:
474+
**Prompt handlers should ONLY throw `PromptGetException` for any error that occurs during prompt generation.**
460475

461476
```php
477+
use Mcp\Exception\PromptGetException;
478+
462479
#[McpPrompt]
463480
public function generatePrompt(string $topic, string $style): array
464481
{
465482
$validStyles = ['casual', 'formal', 'technical'];
466-
483+
467484
if (!in_array($style, $validStyles)) {
468-
throw new \InvalidArgumentException(
485+
throw new PromptGetException(
469486
"Invalid style '{$style}'. Must be one of: " . implode(', ', $validStyles)
470487
);
471488
}
472-
489+
473490
return [
474491
['role' => 'user', 'content' => "Write about {$topic} in a {$style} style"]
475492
];
476493
}
477494
```
478495

496+
**Critical Rule:** For prompt handlers, **ALWAYS** throw `PromptGetException` for any error condition - invalid parameters, data validation errors, template processing issues, etc. Do not use generic PHP exceptions.
497+
498+
**Error Handling Behavior:**
499+
- **`PromptGetException`**: Converted to JSON-RPC error response with the actual exception message
500+
- **Any other exception**: Treated as internal server error and returns a generic error message to client
501+
479502
The SDK automatically validates that all messages have valid roles and converts the result into the appropriate MCP prompt message format.
480503

481504
## Completion Providers

src/Schema/JsonRpc/Error.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ public static function forServerError(string $message, string|int $id = ''): sel
106106
return new self($id, self::SERVER_ERROR, $message);
107107
}
108108

109+
public static function forResourceNotFound(string $message, string|int $id = ''): self
110+
{
111+
return new self($id, self::RESOURCE_NOT_FOUND, $message);
112+
}
113+
109114
public function getId(): string|int
110115
{
111116
return $this->id;

src/Server/Handler/Request/CallToolHandler.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313

1414
use Mcp\Capability\Registry\ReferenceHandlerInterface;
1515
use Mcp\Capability\Registry\ReferenceProviderInterface;
16-
use Mcp\Exception\ExceptionInterface;
1716
use Mcp\Exception\ToolCallException;
1817
use Mcp\Exception\ToolNotFoundException;
18+
use Mcp\Schema\Content\TextContent;
1919
use Mcp\Schema\JsonRpc\Error;
2020
use Mcp\Schema\JsonRpc\Request;
2121
use Mcp\Schema\JsonRpc\Response;
@@ -77,17 +77,19 @@ public function handle(Request $request, SessionInterface $session): Response|Er
7777
]);
7878

7979
return new Response($request->getId(), $result);
80-
} catch (ToolNotFoundException $e) {
81-
$this->logger->error('Tool not found', ['name' => $toolName]);
82-
83-
return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage());
84-
} catch (ToolCallException|ExceptionInterface $e) {
80+
} catch (ToolCallException $e) {
8581
$this->logger->error(\sprintf('Error while executing tool "%s": "%s".', $toolName, $e->getMessage()), [
8682
'tool' => $toolName,
8783
'arguments' => $arguments,
8884
]);
8985

90-
return Error::forInternalError('Error while executing tool', $request->getId());
86+
$errorContent = [new TextContent($e->getMessage())];
87+
88+
return new Response($request->getId(), CallToolResult::error($errorContent));
89+
} catch (ToolNotFoundException $e) {
90+
$this->logger->error('Tool not found', ['name' => $toolName]);
91+
92+
return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage());
9193
} catch (\Throwable $e) {
9294
$this->logger->error('Unhandled error during tool execution', [
9395
'name' => $toolName,

src/Server/Handler/Request/GetPromptHandler.php

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Mcp\Capability\Registry\ReferenceHandlerInterface;
1515
use Mcp\Capability\Registry\ReferenceProviderInterface;
16-
use Mcp\Exception\ExceptionInterface;
1716
use Mcp\Exception\PromptGetException;
1817
use Mcp\Exception\PromptNotFoundException;
1918
use Mcp\Schema\JsonRpc\Error;
@@ -67,18 +66,18 @@ public function handle(Request $request, SessionInterface $session): Response|Er
6766
$formatted = $reference->formatResult($result);
6867

6968
return new Response($request->getId(), new GetPromptResult($formatted));
69+
} catch (PromptGetException $e) {
70+
$this->logger->error(\sprintf('Error while handling prompt "%s": "%s".', $promptName, $e->getMessage()));
71+
72+
return Error::forInternalError($e->getMessage(), $request->getId());
7073
} catch (PromptNotFoundException $e) {
7174
$this->logger->error('Prompt not found', ['prompt_name' => $promptName]);
7275

73-
return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage());
74-
} catch (PromptGetException|ExceptionInterface $e) {
75-
$this->logger->error('Error while handling prompt', ['prompt_name' => $promptName]);
76-
77-
return Error::forInternalError('Error while handling prompt: '.$e->getMessage(), $request->getId());
76+
return Error::forResourceNotFound($e->getMessage(), $request->getId());
7877
} catch (\Throwable $e) {
79-
$this->logger->error('Error while handling prompt', ['prompt_name' => $promptName]);
78+
$this->logger->error(\sprintf('Unexpected error while handling prompt "%s": "%s".', $promptName, $e->getMessage()));
8079

81-
return Error::forInternalError('Error while handling prompt: '.$e->getMessage(), $request->getId());
80+
return Error::forInternalError('Error while handling prompt', $request->getId());
8281
}
8382
}
8483
}

src/Server/Handler/Request/ReadResourceHandler.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Mcp\Capability\Registry\ReferenceProviderInterface;
1616
use Mcp\Capability\Registry\ResourceTemplateReference;
1717
use Mcp\Exception\ResourceNotFoundException;
18+
use Mcp\Exception\ResourceReadException;
1819
use Mcp\Schema\JsonRpc\Error;
1920
use Mcp\Schema\JsonRpc\Request;
2021
use Mcp\Schema\JsonRpc\Response;
@@ -74,12 +75,16 @@ public function handle(Request $request, SessionInterface $session): Response|Er
7475
}
7576

7677
return new Response($request->getId(), new ReadResourceResult($formatted));
78+
} catch (ResourceReadException $e) {
79+
$this->logger->error(\sprintf('Error while reading resource "%s": "%s".', $uri, $e->getMessage()));
80+
81+
return Error::forInternalError($e->getMessage(), $request->getId());
7782
} catch (ResourceNotFoundException $e) {
7883
$this->logger->error('Resource not found', ['uri' => $uri]);
7984

80-
return new Error($request->getId(), Error::RESOURCE_NOT_FOUND, $e->getMessage());
85+
return Error::forResourceNotFound($e->getMessage(), $request->getId());
8186
} catch (\Throwable $e) {
82-
$this->logger->error(\sprintf('Error while reading resource "%s": "%s".', $uri, $e->getMessage()));
87+
$this->logger->error(\sprintf('Unexpected error while reading resource "%s": "%s".', $uri, $e->getMessage()));
8388

8489
return Error::forInternalError('Error while reading resource', $request->getId());
8590
}

tests/Unit/Server/Handler/Request/CallToolHandlerTest.php

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ public function testHandleToolNotFoundExceptionReturnsError(): void
177177
$this->assertEquals(Error::METHOD_NOT_FOUND, $response->code);
178178
}
179179

180-
public function testHandleToolExecutionExceptionReturnsError(): void
180+
public function testHandleToolCallExceptionReturnsResponseWithErrorResult(): void
181181
{
182182
$request = $this->createCallToolRequest('failing_tool', ['param' => 'value']);
183183
$exception = new ToolCallException($request, new \RuntimeException('Tool execution failed'));
@@ -201,9 +201,15 @@ public function testHandleToolExecutionExceptionReturnsError(): void
201201

202202
$response = $this->handler->handle($request, $this->session);
203203

204-
$this->assertInstanceOf(Error::class, $response);
204+
$this->assertInstanceOf(Response::class, $response);
205205
$this->assertEquals($request->getId(), $response->id);
206-
$this->assertEquals(Error::INTERNAL_ERROR, $response->code);
206+
207+
$result = $response->result;
208+
$this->assertInstanceOf(CallToolResult::class, $result);
209+
$this->assertTrue($result->isError);
210+
$this->assertCount(1, $result->content);
211+
$this->assertInstanceOf(TextContent::class, $result->content[0]);
212+
$this->assertEquals('Tool call "failing_tool" failed with error: "Tool execution failed".', $result->content[0]->text);
207213
}
208214

209215
public function testHandleWithNullResult(): void
@@ -274,8 +280,43 @@ public function testHandleLogsErrorWithCorrectParameters(): void
274280

275281
$response = $this->handler->handle($request, $this->session);
276282

283+
// ToolCallException should now return Response with CallToolResult having isError=true
284+
$this->assertInstanceOf(Response::class, $response);
285+
$this->assertEquals($request->getId(), $response->id);
286+
287+
$result = $response->result;
288+
$this->assertInstanceOf(CallToolResult::class, $result);
289+
$this->assertTrue($result->isError);
290+
$this->assertCount(1, $result->content);
291+
$this->assertInstanceOf(TextContent::class, $result->content[0]);
292+
$this->assertEquals('Tool call "test_tool" failed with error: "Custom error message".', $result->content[0]->text);
293+
}
294+
295+
public function testHandleGenericExceptionReturnsError(): void
296+
{
297+
$request = $this->createCallToolRequest('failing_tool', ['param' => 'value']);
298+
$exception = new \RuntimeException('Internal database connection failed');
299+
300+
$toolReference = $this->createMock(ToolReference::class);
301+
$this->referenceProvider
302+
->expects($this->once())
303+
->method('getTool')
304+
->with('failing_tool')
305+
->willReturn($toolReference);
306+
307+
$this->referenceHandler
308+
->expects($this->once())
309+
->method('handle')
310+
->with($toolReference, ['param' => 'value', '_session' => $this->session])
311+
->willThrowException($exception);
312+
313+
$response = $this->handler->handle($request, $this->session);
314+
315+
// Generic exceptions should return Error, not Response
277316
$this->assertInstanceOf(Error::class, $response);
317+
$this->assertEquals($request->getId(), $response->id);
278318
$this->assertEquals(Error::INTERNAL_ERROR, $response->code);
319+
$this->assertEquals('Error while executing tool', $response->message);
279320
}
280321

281322
public function testHandleWithSpecialCharactersInToolName(): void

tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ public function testHandlePromptNotFoundExceptionReturnsError(): void
243243

244244
$this->assertInstanceOf(Error::class, $response);
245245
$this->assertEquals($request->getId(), $response->id);
246-
$this->assertEquals(Error::METHOD_NOT_FOUND, $response->code);
246+
$this->assertEquals(Error::RESOURCE_NOT_FOUND, $response->code);
247247
$this->assertEquals('Prompt not found for name: "nonexistent_prompt".', $response->message);
248248
}
249249

@@ -263,7 +263,7 @@ public function testHandlePromptGetExceptionReturnsError(): void
263263
$this->assertInstanceOf(Error::class, $response);
264264
$this->assertEquals($request->getId(), $response->id);
265265
$this->assertEquals(Error::INTERNAL_ERROR, $response->code);
266-
$this->assertEquals('Error while handling prompt: Handling prompt "failing_prompt" failed with error: "Failed to get prompt".', $response->message);
266+
$this->assertEquals('Handling prompt "failing_prompt" failed with error: "Failed to get prompt".', $response->message);
267267
}
268268

269269
public function testHandlePromptGetWithComplexArguments(): void

tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public function testHandleResourceNotFoundExceptionReturnsSpecificError(): void
194194
$this->assertEquals('Resource not found for uri: "'.$uri.'".', $response->message);
195195
}
196196

197-
public function testHandleResourceReadExceptionReturnsGenericError(): void
197+
public function testHandleResourceReadExceptionReturnsActualErrorMessage(): void
198198
{
199199
$uri = 'file://corrupted/file.txt';
200200
$request = $this->createReadResourceRequest($uri);
@@ -211,6 +211,26 @@ public function testHandleResourceReadExceptionReturnsGenericError(): void
211211

212212
$response = $this->handler->handle($request, $this->session);
213213

214+
$this->assertInstanceOf(Error::class, $response);
215+
$this->assertEquals($request->getId(), $response->id);
216+
$this->assertEquals(Error::INTERNAL_ERROR, $response->code);
217+
$this->assertEquals('Reading resource "file://corrupted/file.txt" failed with error: "Failed to read resource: corrupted data".', $response->message);
218+
}
219+
220+
public function testHandleGenericExceptionReturnsGenericError(): void
221+
{
222+
$uri = 'file://problematic/file.txt';
223+
$request = $this->createReadResourceRequest($uri);
224+
$exception = new \RuntimeException('Internal database connection failed');
225+
226+
$this->referenceProvider
227+
->expects($this->once())
228+
->method('getResource')
229+
->with($uri)
230+
->willThrowException($exception);
231+
232+
$response = $this->handler->handle($request, $this->session);
233+
214234
$this->assertInstanceOf(Error::class, $response);
215235
$this->assertEquals($request->getId(), $response->id);
216236
$this->assertEquals(Error::INTERNAL_ERROR, $response->code);

0 commit comments

Comments
 (0)