Skip to content

Refactor error handling in REST API run controller #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 21, 2025
Merged

Refactor error handling in REST API run controller #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a88e04
Refactor error handling in REST API run controller
gziolo Aug 19, 2025
ac69406
Improve placement for translation comments
gziolo Aug 19, 2025
6562eb3
Refactor unit tests to include more details in assertions
gziolo Aug 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions includes/abilities-api/class-wp-ability.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,20 @@ protected function validate_input( array $input = array() ) {
return true;
}

$valid_input = rest_validate_value_from_schema( $input, $input_schema );
$valid_input = rest_validate_value_from_schema( $input, $input_schema, 'input' );
if ( is_wp_error( $valid_input ) ) {
return new \WP_Error(
'ability_invalid_input',
sprintf(
/* translators: %1$s ability name, %2$s error message. */
__( 'Ability "%1$s" has invalid input. Reason: %2$s' ),
$this->name,
$valid_input->get_error_message()
)
);
}

return is_wp_error( $valid_input ) ? $valid_input : true;
return true;
}

/**
Expand Down Expand Up @@ -260,9 +271,20 @@ protected function validate_output( $output ) {
return true;
}

$valid_output = rest_validate_value_from_schema( $output, $output_schema );
$valid_output = rest_validate_value_from_schema( $output, $output_schema, 'output' );
if ( is_wp_error( $valid_output ) ) {
return new \WP_Error(
'ability_invalid_output',
sprintf(
/* translators: %1$s ability name, %2$s error message. */
__( 'Ability "%1$s" has invalid output. Reason: %2$s' ),
$this->name,
$valid_output->get_error_message()
)
);
}

return is_wp_error( $valid_output ) ? $valid_output : true;
return true;
}

/**
Expand All @@ -276,10 +298,12 @@ protected function validate_output( $output ) {
*/
public function execute( array $input = array() ) {
$has_permissions = $this->has_permission( $input );

if ( true !== $has_permissions ) {
if ( is_wp_error( $has_permissions ) ) {
// Don't leak the error to someone without the correct perms.
if ( 'ability_invalid_input' === $has_permissions->get_error_code() ) {
return $has_permissions;
}
// Don't leak the permission check error to someone without the correct perms.
_doing_it_wrong(
__METHOD__,
esc_html( $has_permissions->get_error_message() ),
Expand Down
94 changes: 7 additions & 87 deletions includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ public function run_ability_with_method_check( $request ) {

if ( 'resource' === $type && 'GET' !== $method ) {
return new \WP_Error(
'rest_invalid_method',
'rest_ability_invalid_method',
__( 'Resource abilities require GET method.' ),
array( 'status' => 405 )
);
}

if ( 'tool' === $type && 'POST' !== $method ) {
return new \WP_Error(
'rest_invalid_method',
'rest_ability_invalid_method',
__( 'Tool abilities require POST method.' ),
array( 'status' => 405 )
);
Expand All @@ -123,7 +123,6 @@ public function run_ability_with_method_check( $request ) {
*/
public function run_ability( $request ) {
$ability = wp_get_ability( $request->get_param( 'name' ) );

if ( ! $ability ) {
return new \WP_Error(
'rest_ability_not_found',
Expand All @@ -132,28 +131,15 @@ public function run_ability( $request ) {
);
}

$input = $this->get_input_from_request( $request );

// REST API needs detailed error messages with HTTP status codes.
// While WP_Ability::execute() validates internally, it only returns false
// and logs with _doing_it_wrong, which doesn't provide capturable error messages.
// TODO: Consider updating WP_Ability to return WP_Error for better error handling.
$input_validation = $this->validate_input( $ability, $input );
if ( is_wp_error( $input_validation ) ) {
return $input_validation;
}

$input = $this->get_input_from_request( $request );
$result = $ability->execute( $input );

if ( is_wp_error( $result ) ) {
if ( 'ability_invalid_input' === $result->get_error_code() ) {
$result->add_data( array( 'status' => 400 ) );
}
return $result;
}

$output_validation = $this->validate_output( $ability, $result );
if ( is_wp_error( $output_validation ) ) {
return $output_validation;
}

return rest_ensure_response( $result );
}

Expand All @@ -167,7 +153,6 @@ public function run_ability( $request ) {
*/
public function run_ability_permissions_check( $request ) {
$ability = wp_get_ability( $request->get_param( 'name' ) );

if ( ! $ability ) {
return new \WP_Error(
'rest_ability_not_found',
Expand All @@ -177,10 +162,9 @@ public function run_ability_permissions_check( $request ) {
}

$input = $this->get_input_from_request( $request );

if ( ! $ability->has_permission( $input ) ) {
return new \WP_Error(
'rest_cannot_execute',
'rest_ability_cannot_execute',
__( 'Sorry, you are not allowed to execute this ability.' ),
array( 'status' => rest_authorization_required_code() )
);
Expand All @@ -189,70 +173,6 @@ public function run_ability_permissions_check( $request ) {
return true;
}

/**
* Validates input data against the ability's input schema.
*
* @since 0.1.0
*
* @param \WP_Ability $ability The ability object.
* @param array<string, mixed> $input The input data to validate.
* @return true|\WP_Error True if validation passes, WP_Error object on failure.
*/
private function validate_input( $ability, $input ) {
$input_schema = $ability->get_input_schema();

if ( empty( $input_schema ) ) {
return true;
}

$validation_result = rest_validate_value_from_schema( $input, $input_schema );
if ( is_wp_error( $validation_result ) ) {
return new \WP_Error(
'rest_invalid_param',
sprintf(
/* translators: %s: error message */
__( 'Invalid input parameters: %s' ),
$validation_result->get_error_message()
),
array( 'status' => 400 )
);
}

return true;
}

/**
* Validates output data against the ability's output schema.
*
* @since 0.1.0
*
* @param \WP_Ability $ability The ability object.
* @param mixed $output The output data to validate.
* @return true|\WP_Error True if validation passes, WP_Error object on failure.
*/
private function validate_output( $ability, $output ) {
$output_schema = $ability->get_output_schema();

if ( empty( $output_schema ) ) {
return true;
}

$validation_result = rest_validate_value_from_schema( $output, $output_schema );
if ( is_wp_error( $validation_result ) ) {
return new \WP_Error(
'rest_invalid_response',
sprintf(
/* translators: %s: error message */
__( 'Invalid response from ability: %s' ),
$validation_result->get_error_message()
),
array( 'status' => 500 )
);
}

return true;
}

/**
* Extracts input parameters from the request.
*
Expand Down
26 changes: 18 additions & 8 deletions tests/unit/abilities-api/wpRegisterAbility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ public function test_execute_ability_no_input_schema_match(): void {

$result = wp_register_ability( self::$test_ability_name, self::$test_ability_properties );

$this->setExpectedIncorrectUsage( 'WP_Ability::execute' );

$actual = $result->execute(
array(
'a' => 2,
Expand All @@ -194,9 +192,13 @@ public function test_execute_ability_no_input_schema_match(): void {

$this->assertWPError(
$actual,
'Execution should fail due to input not matching schema'
'Execution should fail due to input not matching schema.'
);
$this->assertSame( 'ability_invalid_input', $actual->get_error_code() );
$this->assertSame(
'Ability "test/add-numbers" has invalid input. Reason: unknown is not a valid property of Object.',
$actual->get_error_message()
);
$this->assertEquals( 'ability_invalid_permissions', $actual->get_error_code() );
}

/**
Expand All @@ -218,9 +220,13 @@ public function test_execute_ability_no_output_schema_match(): void {
);
$this->assertWPError(
$actual,
'Execution should fail due to output not matching schema',
'Execution should fail due to output not matching schema.',
);
$this->assertSame( 'ability_invalid_output', $actual->get_error_code() );
$this->assertSame(
'Ability "test/add-numbers" has invalid output. Reason: output is not of type number.',
$actual->get_error_message()
);
$this->assertEquals( 'rest_invalid_type', $actual->get_error_code() );
}

/**
Expand All @@ -241,9 +247,13 @@ public function test_permission_callback_no_input_schema_match(): void {

$this->assertWPError(
$actual,
'Permission check should fail due to input not matching schema'
'Permission check should fail due to input not matching schema.'
);
$this->assertSame( 'ability_invalid_input', $actual->get_error_code() );
$this->assertSame(
'Ability "test/add-numbers" has invalid input. Reason: unknown is not a valid property of Object.',
$actual->get_error_message()
);
$this->assertEquals( 'rest_additional_properties_forbidden', $actual->get_error_code() );
}

/**
Expand Down
52 changes: 32 additions & 20 deletions tests/unit/rest-api/wpRestAbilitiesRunController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,10 @@ public function test_tool_ability_requires_post(): void {
$request = new WP_REST_Request( 'GET', '/wp/v2/abilities/test/open-tool/run' );
$response = $this->server->dispatch( $request );

$this->assertEquals( 405, $response->get_status() );
$this->assertSame( 405, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_invalid_method', $data['code'] );
$this->assertStringContainsString( 'Tool abilities require POST', $data['message'] );
$this->assertSame( 'rest_ability_invalid_method', $data['code'] );
$this->assertSame( 'Tool abilities require POST method.', $data['message'] );
}

/**
Expand All @@ -354,10 +354,10 @@ public function test_resource_ability_requires_get(): void {

$response = $this->server->dispatch( $request );

$this->assertEquals( 405, $response->get_status() );
$this->assertSame( 405, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_invalid_method', $data['code'] );
$this->assertStringContainsString( 'Resource abilities require GET', $data['message'] );
$this->assertSame( 'rest_ability_invalid_method', $data['code'] );
$this->assertSame( 'Resource abilities require GET method.', $data['message'] );
}


Expand All @@ -373,11 +373,13 @@ public function test_output_validation(): void {

$response = $this->server->dispatch( $request );

$this->assertEquals( 500, $response->get_status() );
$this->assertSame( 500, $response->get_status() );
$data = $response->get_data();

$this->assertEquals( 'rest_invalid_type', $data['code'] );
$this->assertStringContainsString( 'is not of type number.', $data['message'] );
$this->assertSame( 'ability_invalid_output', $data['code'] );
$this->assertSame(
'Ability "test/invalid-output" has invalid output. Reason: output is not of type number.',
$data['message']
);
}

/**
Expand All @@ -401,9 +403,10 @@ public function test_execution_permission_denied(): void {

$response = $this->server->dispatch( $request );

$this->assertEquals( 403, $response->get_status() );
$this->assertSame( 403, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_cannot_execute', $data['code'] );
$this->assertSame( 'rest_ability_cannot_execute', $data['code'] );
$this->assertSame( 'Sorry, you are not allowed to execute this ability.', $data['message'] );
}

/**
Expand Down Expand Up @@ -618,10 +621,14 @@ public function test_output_validation_failure_returns_error(): void {

$response = $this->server->dispatch( $request );

// Should return error when output validation fails
$this->assertEquals( 500, $response->get_status() );
// Should return error when output validation fails.
$this->assertSame( 500, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_property_required', $data['code'] );
$this->assertSame( 'ability_invalid_output', $data['code'] );
$this->assertSame(
'Ability "test/strict-output" has invalid output. Reason: status is a required property of output.',
$data['message']
);
}

/**
Expand Down Expand Up @@ -659,9 +666,13 @@ public function test_input_validation_failure_returns_error(): void {
$response = $this->server->dispatch( $request );

// Should return error when input validation fails.
$this->assertEquals( 400, $response->get_status() );
$this->assertSame( 400, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_invalid_param', $data['code'] );
$this->assertSame( 'ability_invalid_input', $data['code'] );
$this->assertSame(
'Ability "test/strict-input" has invalid input. Reason: required_field is a required property of input.',
$data['message']
);
}

/**
Expand Down Expand Up @@ -934,10 +945,11 @@ public function test_invalid_http_methods( string $method ): void {
$request = new WP_REST_Request( $method, '/wp/v2/abilities/test/method-test/run' );
$response = $this->server->dispatch( $request );

// Tool abilities should only accept POST, so these should return 405
$this->assertEquals( 405, $response->get_status() );
// Tool abilities should only accept POST, so these should return 405.
$this->assertSame( 405, $response->get_status() );
$data = $response->get_data();
$this->assertEquals( 'rest_invalid_method', $data['code'] );
$this->assertSame( 'rest_ability_invalid_method', $data['code'] );
$this->assertSame( 'Tool abilities require POST method.', $data['message'] );
}

/**
Expand Down
Loading