`Keyring_Error` is sometimes instantiated in a way that is incompatible with usage of `WP_Error`

[jorgeatorres]

In the implementation for most Keyring services (maybe all?) a request that results in an error returns an instance of Keyring_Error.

This is all good until you see that Keyring_Error is a subclass of WP_Error with no specific implementation, and as such, you would expect that it'd be compatible with how WP_Error is used in WP. For example, the following standard error-checking pattern from the WP world can result in a PHP notice/warning/error if $result is of type Keyring_Error:

if ( is_wp_error( $result ) ) {
    echo $result->get_error_message();
}

In particular, because there are a bunch of places in the codebase where Keyring_Error is instantiated passing an error code as first argument and the HTTP response object as second argument, the "error message" stored in the object is frequently not a string. This can happen, for example, if the server returns something other than a 2xx code.

For comparison, this is WP_Error's constructor signature:

public function __construct( $code = '', $message = '', $data = '' )

The second argument should be a string, which would also enable Keyring_Error::get_error_message() to return something that can be printed on screen or logged to a log file.

Most likely, the response was meant to go into the 3rd parameter ($data), which can in fact be of any type.

---

Full example:

add_action(
	'keyring_load_services',
	function() {

		class Keyring_Dummy_Service extends Keyring_Service_OAuth2 {

			public function get_name() {
				return 'dummy';
			}

			public function requires_token($does_it = null) {
				return false;
			}

		}

		Keyring_Dummy_Service::init();
	}
);

add_action( 'init', function() {

	Keyring::init();

	$k = Keyring::get_service_by_name( 'dummy' );
	$result = $k->request( 'https://amazon.com/nothinghere');

	if ( is_wp_error( $result ) ) {
		// Results in "Array to string conversion" PHP notice.
		echo $result->get_error_message();
	}

} );