Skip to content
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.

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

Add support for passing a WP_Error object to $context in log methods #7

Closed
DrewAPicture opened this issue Apr 29, 2021 · 2 comments
Closed
Labels
enhancement New feature or request

Comments

@DrewAPicture
Copy link

DrewAPicture commented Apr 29, 2021

Describe the problem you want to solve

Problem: The $context parameter in the log methods only supports passing an array of data.

Background:
It is not currently possible to (sanely) pass a WP_Error object to the $context parameter in any of the log methods outside of passing it inside an array in order to adhere to the typehinting rules.

A central feature of error logging in WordPress is the ability to leverage the WP_Error object to log errors against, and I would think a WordPress logger utility should handle for that eventuality as elegantly as it can.

Describe the solution you'd like

Ideally, $context would be allowed to accept mixed types: either an array or WP_Error object. Naturally, such a change would mean the typehinting on the $context parameter in all of the logging method signatures would have to be dropped.

Describe alternatives you've considered

I've been using a sort of simplified version of a logger similar to this one in private and commercial WordPress products for years. When our $data parameter (similar in use to $context) was initially introduced, it was blind to the input, I think it just supported any kind of input and was output as a string similarly to how the new dump() method introduced in #6 works.

At a certain point, DRY principles necessitated adding WP_Error support to handle for first- and third-party integrations that returned WP_Error object. This feature was initially implemented to support WP_Error objects containing just a single error, though it was later expanded to handle for WP_Error objects containing multiple different errors that could be "collated" together into a cohesive string of output.

Additional context

Changing $context to accept something other than an array (and dropping the array typehints) would constitute a break in backward compatibility for anyone extending the Log or Logger classes.

@DrewAPicture DrewAPicture added the enhancement New feature or request label Apr 29, 2021
@DrewAPicture
Copy link
Author

To give you some perspective, this is a barebones example for how my simplified logger operates:

/**
 * Log message to file.
 *
 * @since x.x.x
 *
 * @param string      $message Message to write to the debug log.
 * @param array|mixed $data    Optional. Array of data or other output to send to the log.
 *                             Default empty array.
 * @return void
 */
public function log( $message, $data = array() ) {
	$message = date( 'Y-n-d H:i:s' ) . ' - ' . $message . "\r\n";

	if ( ! empty( $data ) ) {
		if ( is_array( $data ) ) {
			$data = var_export( $data, true );
		} elseif ( is_wp_error( $data ) ) {
			$data = $this->collate_errors( $data );
		} else {
			ob_start();

			var_dump( $data );

			$data = ob_get_clean();
		}

		$message .= $data;
	}


	$this->write_to_log( $message );
}

This version of a log() method handles for passing an array, a WP_Error object, or any other random thing that ends up simply getting dumped out to the output buffer.

The collate_errors() method handles for the eventuality that somebody has potentially leveraged a single WP_Error instance to log multiple errors along the point of execution, i.e.

$errors = new \WP_Error();

if ( true === false ) {
    $errors->add_error( 'some_error_code', 'Some message', [ 'some' => 'data' ] );
}

if ( false === true ) {
    $errors->add_error( 'some_other_error_code', 'Some other message', [ 'some' => 'other_data' ] );
}

// Pre-WP 5.1 support for has_errors().
$has_errors = method_exists( $errors, 'has_errors' ) ? $errors->has_errors() : ! empty( $errors->errors );

if ( true === $has_errors ) {
    Logger::log( 'There was an error in executing foo.', $errors );
}

This is how collate_errors() works:

/**
 * Collates errors stored in a WP_Error object for output to the debug log.
 *
 * @since 2.3
 *
 * @param \WP_Error $wp_error WP_Error object.
 * @return string Error log output. Empty if not a WP_Error object or if there are no errors to collate.
 */
public function collate_errors( WP_Error $wp_error ) {
	$output = '';

	// Pre-WP 5.1 support for has_errors().
	$has_errors = method_exists( $wp_error, 'has_errors' ) ? $wp_error->has_errors() : ! empty( $wp_error->errors );

	if ( false === $has_errors ) {
		return $output;
	}

	foreach ( $wp_error->errors as $code => $messages ) {
		$message = implode( ' ', $messages );

		if ( isset( $wp_error->error_data[ $code ] ) ) {
			$data = $wp_error->error_data[ $code ];
		} else {
			$data = '';
		}

		$output .= sprintf( '- (%1$s): %2$s', $code, $message ) . "\r\n";

		if ( ! empty( $data ) ) {
			$output .= var_export( $data, true ) . "\r\n";
		}
	}

	return $output;
}

And finally debug log output that looks something like this:

2021-4-28 01:46:51 - There was an error in executing foo.
- (some_error_code): Some message.
array (
  'some' => 'data',
)
- (some_other_error_code): Some other message.
array (
  'some' => 'other_data',
)

@andrewwoods
Copy link
Owner

You're right that WP_Error support is needed. I will add that. I'll definitely add the collate_errors function, though I'll adapt it for this project. The problem with dropping the typehints approach I see, is that it will break PSR-3 support - which is the basis for this project. I think what I want to do is create a separate method typehinted to WP_Error.

Log::wp_error( string $level, string $messgae, WP_Error $data );

In this, the collate_errors() would get called. That's the high level view of how I want to adapt it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants