Skip to content

Commit db59ef5

Browse files
Refactor error handling: Replace exceptions with WP_Error pattern (#71)
* Refactor Core layer to use WP_Error instead of exceptions Replace exception-based error handling with WordPress-native WP_Error pattern in the Core layer components. Changes: - McpAdapter::create_server() now returns self|WP_Error instead of throwing exceptions for validation failures (invalid error handler, observability handler, timing, and duplicate server IDs) - DefaultServerFactory::create() adds is_wp_error() check and logs errors - McpComponentRegistry removes 4 try-catch blocks from register_tools(), add_tool(), register_resources(), and register_prompts() - McpTransportFactory logs errors and continues processing other transports instead of throwing exceptions - Update McpTransportFactoryTest to expect graceful error handling Error codes introduced: - invalid_error_handler - invalid_observability_handler - invalid_timing - duplicate_server_id All 357 tests passing. Related to #24 * Refactor Domain layer validators to use WP_Error instead of exceptions Convert all domain validators and validate() methods to return WP_Error instead of throwing exceptions, following WordPress conventions. Changes: - All validator methods (validate_tool_data, validate_tool_instance, validate_tool_uniqueness, and equivalents for Resources and Prompts) now return bool|WP_Error instead of void - McpTool::get_ability() returns WP_Ability|WP_Error instead of throwing InvalidArgumentException - McpTool::validate(), McpResource::validate(), and McpPrompt::validate() return self|WP_Error instead of throwing - ToolsHandler::handle_tool_call() adds is_wp_error() check after calling get_ability() - Add assertWPError() and assertNotWPError() helper methods to TestCase - Update 37 validator tests across 3 test files to expect WP_Error instead of exceptions Error codes introduced: - tool_validation_failed, tool_not_unique - resource_validation_failed, resource_not_unique - prompt_validation_failed, prompt_not_unique - ability_not_found All 357 tests passing with 1521 assertions. Related to #24 * Refactor registration classes to propagate WP_Error instead of throwing exceptions Convert all RegisterAbilityAs* classes to return WP_Error instead of throwing exceptions when converting WordPress abilities to MCP components. Changes: - RegisterAbilityAsMcpTool::make() returns McpTool|WP_Error - RegisterAbilityAsMcpResource::make() returns McpResource|WP_Error - RegisterAbilityAsMcpResource::get_uri() returns string|WP_Error instead of throwing InvalidArgumentException - RegisterAbilityAsMcpResource::get_data() propagates WP_Error from get_uri() - RegisterAbilityAsMcpPrompt::make() returns McpPrompt|WP_Error - Remove all @throws annotations from registration classes Error propagation pattern: - Check is_wp_error() at each method in the chain - Return WP_Error immediately if found - Allow from_array() validation errors to bubble up naturally This completes the elimination of all exception throws in the registration layer, with errors now properly propagated as WP_Error objects. All 357 tests passing with 1521 assertions. Related to #24 * Add WP_Error handling in Prompts and Resources handlers Add is_wp_error() checks in handlers after get_ability() calls to properly handle errors propagated from the registration and domain layers. Changes: - PromptsHandler::get_prompt() checks is_wp_error() after get_ability() for non-builder prompts - ResourcesHandler::read_resource() checks is_wp_error() after get_ability() - Both handlers log errors and return proper MCP error responses with metadata including error_code and failure_reason - Error responses include 'ability_retrieval_failed' as failure_reason This completes the refactoring to eliminate all exception-based error handling in favor of WordPress's native WP_Error pattern. The codebase now has: - Zero exception throws in production code (7 eliminated) - Zero try-catch blocks for InvalidArgumentException (4 removed) - Consistent WP_Error propagation from Core → Domain → Registration → Handlers All 357 tests passing with 1521 assertions. Fixes #24 * Fix PHPStan errors by making all get_ability() methods return WP_Error Ensure consistency across all MCP component types by making get_ability() return WP_Ability|WP_Error instead of nullable WP_Ability. Changes: - McpResource::get_ability() now returns WP_Ability|WP_Error instead of ?WP_Ability (null) - McpPrompt::get_ability() now returns WP_Ability|WP_Error instead of ?WP_Ability (null) - McpPromptBuilder anonymous class get_ability() returns WP_Error with 'builder_has_no_ability' error code - Update 2 tests to expect WP_Error instead of null from builder prompts This ensures PHPStan type checking passes and all get_ability() methods across the codebase follow the same error handling pattern. All 357 tests passing with 1523 assertions. PHPStan Level 8 analysis passing with no errors. Related to #24 * Refactor error logging in DefaultServerFactory to use _doing_it_wrong. * Remove unused WP_Ability imports from McpPrompt, McpPromptBuilder, and McpResource files. Update error logging in DefaultServerFactory to escape error messages and codes. * Update error logging in DefaultServerFactory to ensure error codes are cast to string. * Adds unit tests for handlers and component registry Introduces comprehensive unit test suites for the `ToolsHandler` and `ResourcesHandler` to verify their core functionality. These tests cover success paths, error conditions like missing parameters or components not being found, and permission handling. Additionally, enhances the `McpComponentRegistry` tests to ensure that non-string inputs are correctly skipped during registration and that no observability events are recorded when the feature is disabled. * Fix tool error handling to comply with MCP specification Implement proper distinction between protocol errors and tool execution errors according to MCP spec (https://modelcontextprotocol.io/specification/2025-06-18/server/tools#error-handling): Protocol errors (JSON-RPC format with 'error' key): - Tool not found - Missing required parameters - Internal errors (ability retrieval failures) Tool execution errors (isError: true format): - Permission denied - Permission check failures - Execution failures - WP_Error from ability execution Changes: - ToolsHandler::call_tool() now distinguishes error types based on failure_reason metadata - Permission denied, permission check failures, and execution errors now return {content: [{type: 'text', text: '...'}], isError: true} - Protocol errors still return JSON-RPC error format - Update 7 tests to expect isError format for tool execution errors - Remove impossible is_wp_error() check (PHPStan was correct) All 357 tests passing with 1527 assertions. PHPStan Level 8 analysis passing with no errors. Fixes #69 * Fix WP_Error handling in Prompts and Resources handlers (#74) Add proper WP_Error detection after ability execution in PromptsHandler and ResourcesHandler to prevent crashes and return graceful error responses. * Fix permission error tests to align with MCP specification Updates test expectations for permission-related errors in ToolsHandler to use isError format instead of JSON-RPC error format. Per MCP spec: "Any errors that originate from the tool SHOULD be reported inside the result object, with isError set to true, not as an MCP protocol-level error response. Otherwise, the LLM would not be able to see that an error occurred and self-correct." Permission errors originate from the tool's permission check phase, so they should be returned as tool execution errors (isError: true) rather than protocol-level errors. This allows LLMs to observe and potentially handle permission failures. Changes: - test_call_tool_permission_exception_returns_error: Now expects isError - test_call_tool_permission_denied_returns_error: Now expects isError - Added spec-based comments explaining the error handling approach * Fixed backwards conditional logic in TestCase::set_up_before_class() * Removes a comment * Adds assertions for registered abilities Improves test robustness by adding assertions to ensure abilities are registered before being used in tests. This prevents unexpected errors and provides more informative feedback when abilities are not correctly registered. * Guards prompt building against exceptions Improves the stability of prompt registration by wrapping the prompt builder instantiation and build process in a try-catch block. This prevents a single prompt builder exception from halting the entire component registration process. When an exception occurs, it logs the error and records a failure event, allowing the system to continue registering other components. * Adds return type declaration Adds a return type declaration to the `get_ability` method for better type hinting. * Improves developer error handling and testing Enhances developer experience by providing more informative error messages and adding new tests to ensure proper error handling in various scenarios. Adds a mechanism to capture `_doing_it_wrong` calls for more robust testing of developer-facing errors. This allows for assertions that specific functions trigger `_doing_it_wrong` with the expected messages, leading to more reliable detection of incorrect API usage. Specifically, the changes cover: - Creating servers outside of the `mcp_adapter_init` hook. - Using duplicate server IDs. - Using non-existent or invalid transport classes. - Cloning and waking up the Plugin singleton. These improvements help developers identify and fix integration issues more easily, resulting in a more stable and maintainable system. * Improves error handling in prompt builder Ensures that errors during prompt building are caught more broadly by catching `\Throwable` instead of just `\Exception`. This allows the system to handle a wider range of potential issues that might arise when constructing prompts, improving stability and providing more informative error logs. * Registers abilities during the API init hook Ensures abilities are registered within the `wp_abilities_api_init` hook. This is necessary as the WordPress abilities API checks if the hook is currently running. Adds a helper function `register_ability_in_hook` to facilitate registering abilities correctly. * Refactors ability registration in tests Refactors the way abilities are registered in tests to ensure proper cleanup and prevent potential duplicate registrations when the `wp_abilities_api_init` hook is fired multiple times. It achieves this by using a static function for the callback and registers the ability using the `register_ability_in_hook` method. * Enhances ability registration tests Improves the testing framework for ability registration by adding assertions to verify that abilities are correctly registered before use. This change aims to prevent unexpected errors during tests and provides clearer feedback when registration issues occur, contributing to a more reliable testing environment. * Enables MCP component validation during registration Implements validation checks for MCP components (tools and prompts) during registration, ensuring that components meet the required criteria before being added to the system. This change ensures that the validation flag on the McpComponentRegistry is respected by directly calling the validators, and that components are validated during registration and when added individually. This improves the reliability and consistency of the MCP system by preventing the registration of invalid components. It also records component registration failures for observability purposes. Adds the ability to set the MCP server on the McpResource. * Enhances error handling in MCP component tests Implements additional tests for error scenarios in MCP components, including handling WP_Error and exceptions during ability execution, resource reading, and tool calls. This change ensures that the system correctly identifies and reports errors, improving the reliability of the MCP framework. It also verifies that appropriate error messages and metadata are returned, contributing to better observability and debugging capabilities.
1 parent b55221f commit db59ef5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+3107
-593
lines changed

includes/Core/McpAdapter.php

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,18 +102,18 @@ public function init(): void {
102102
* @param array $prompts Prompts to register.
103103
* @param callable|null $transport_permission_callback Optional custom permission callback for transport-level authentication. If null, defaults to is_user_logged_in().
104104
*
105-
* @return \WP\MCP\Core\McpAdapter
106-
* @throws \Exception If the server already exists or if called outside of the mcp_adapter_init action.
105+
* @return \WP\MCP\Core\McpAdapter|\WP_Error McpAdapter instance on success, WP_Error on failure.
107106
*/
108-
public function create_server( string $server_id, string $server_route_namespace, string $server_route, string $server_name, string $server_description, string $server_version, array $mcp_transports, ?string $error_handler, ?string $observability_handler = null, array $tools = array(), array $resources = array(), array $prompts = array(), ?callable $transport_permission_callback = null ): self {
107+
public function create_server( string $server_id, string $server_route_namespace, string $server_route, string $server_name, string $server_description, string $server_version, array $mcp_transports, ?string $error_handler, ?string $observability_handler = null, array $tools = array(), array $resources = array(), array $prompts = array(), ?callable $transport_permission_callback = null ) {
109108
// Use NullMcpErrorHandler if no error handler is provided.
110109
if ( ! $error_handler ) {
111110
$error_handler = NullMcpErrorHandler::class;
112111
}
113112

114113
// Validate error handler class exists and implements McpErrorHandlerInterface.
115114
if ( ! class_exists( $error_handler ) ) {
116-
throw new \Exception(
115+
return new \WP_Error(
116+
'invalid_error_handler',
117117
sprintf(
118118
/* translators: %s: error handler class name */
119119
esc_html__( 'Error handler class "%s" does not exist.', 'mcp-adapter' ),
@@ -123,7 +123,8 @@ public function create_server( string $server_id, string $server_route_namespace
123123
}
124124

125125
if ( ! in_array( McpErrorHandlerInterface::class, class_implements( $error_handler ) ?: array(), true ) ) {
126-
throw new \Exception(
126+
return new \WP_Error(
127+
'invalid_error_handler',
127128
sprintf(
128129
/* translators: %s: error handler class name */
129130
esc_html__( 'Error handler class "%s" must implement the McpErrorHandlerInterface.', 'mcp-adapter' ),
@@ -139,7 +140,8 @@ public function create_server( string $server_id, string $server_route_namespace
139140

140141
// Validate observability handler class exists and implements McpObservabilityHandlerInterface.
141142
if ( ! class_exists( $observability_handler ) ) {
142-
throw new \Exception(
143+
return new \WP_Error(
144+
'invalid_observability_handler',
143145
sprintf(
144146
/* translators: %s: observability handler class name */
145147
esc_html__( 'Observability handler class "%s" does not exist.', 'mcp-adapter' ),
@@ -149,7 +151,8 @@ public function create_server( string $server_id, string $server_route_namespace
149151
}
150152

151153
if ( ! in_array( McpObservabilityHandlerInterface::class, class_implements( $observability_handler ) ?: array(), true ) ) {
152-
throw new \Exception(
154+
return new \WP_Error(
155+
'invalid_observability_handler',
153156
sprintf(
154157
/* translators: %s: observability handler class name */
155158
esc_html__( 'Observability handler class "%s" must implement the McpObservabilityHandlerInterface interface.', 'mcp-adapter' ),
@@ -164,7 +167,8 @@ public function create_server( string $server_id, string $server_route_namespace
164167
esc_html__( 'MCP Servers must be created during the "mcp_adapter_init" action. Hook into "mcp_adapter_init" to register your server.', 'mcp-adapter' ),
165168
'0.1.0'
166169
);
167-
throw new \Exception(
170+
return new \WP_Error(
171+
'invalid_timing',
168172
esc_html__( 'MCP Server creation must be done during mcp_adapter_init action.', 'mcp-adapter' )
169173
);
170174
}
@@ -179,8 +183,9 @@ public function create_server( string $server_id, string $server_route_namespace
179183
),
180184
'0.1.0'
181185
);
182-
throw new \Exception(
183-
// translators: %s: server ID.
186+
return new \WP_Error(
187+
'duplicate_server_id',
188+
// translators: %s: server ID.
184189
sprintf( esc_html__( 'Server with ID "%s" already exists.', 'mcp-adapter' ), esc_html( $server_id ) )
185190
);
186191
}

0 commit comments

Comments
 (0)