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

What should the logging behavior be when run from WP CLI? #15

Closed
salcode opened this issue Feb 16, 2018 · 3 comments
Closed

What should the logging behavior be when run from WP CLI? #15

salcode opened this issue Feb 16, 2018 · 3 comments

Comments

@salcode
Copy link
Owner

salcode commented Feb 16, 2018

As @mitchelldmiller bring up in this comment

When WP CLI is used to run code that tries to send an email

The screen fills with error_log output.

He suggests

You might want to replace the error_log() if WP-CLI is active.

@mitchelldmiller
Copy link

mitchelldmiller commented Feb 16, 2018

Suggestion:

 public function log_to_php_error_log( $mock_email ) {
      if ( $this->should_emails_be_logged_to_php_error_log() ) {
         if ( defined( 'WP_CLI' ) && WP_CLI ) {
           WP_CLI::warning( __( 'Stop Emails: suppressing error log.', 'stop-emails' ) );
         } else {
            $text = $this->mock_email_to_text( $mock_email );
            error_log( $text );
         } // end if WP-CLI
      }
   } 

salcode added a commit that referenced this issue Feb 18, 2018
Add check when logging outgoing messages to the error log, for WP CLI.
If WP CLI is being used, displaying a single warning rather than dumping
the entire email to the error log. Writing to the error log populates on
the screen when using WP CLI.

See #15
@salcode
Copy link
Owner Author

salcode commented May 2, 2018

@mitchelldmiller Thanks for your patience on this issue.

After giving this a lot of thought, I'm against removing the logging behavior when WP CLI is used. Not logging the email (in some form) when logging is turned on, would be an unexpected behavior.

I can imagine running the Stop Emails plugin with logging turned on and then performing a WP CLI task which generates an email. In that situation, I would expect the email to be logged. While WP CLI logs the email to the command line (rather than the log file), I still think this is preferable to not logging in this situation.

Additionally, I can imagine instances of other code (e.g. a plugin) wanting to prevent this logging. In #14 we discussed the option of deactivating the plugin but as you pointed out, that isn't a good solution.

Since whether or not Stop Emails tries to log an email is based on a value (log-email) in the serialized option fe_stop_emails_options, we can modify this with a filter such as the following.

if ( class_exists( 'WP_CLI') ) {
	add_filter( 'option_fe_stop_emails_options', function( $fe_stop_emails_options ) {
		if ( isset( $fe_stop_emails_options['log-email'] ) && 1 === $fe_stop_emails_options['log-email'] ) {
			WP_CLI::log( 'My Plugin is suspending the Stop Emails Plugin from Logging Emails when run from WP CLI' );
			$fe_stop_emails_options['log-email'] = 0;
		}
		return $fe_stop_emails_options;
	} );
}

I've successfully tested this solution in my proof of concept plugin at https://gist.github.com/salcode/52d6f88efc175ddfa965a90c50350747.

I realize this solution is probably not optimal for your situation and I'm sorry for that. I hope this is helpful in some way.

@salcode
Copy link
Owner Author

salcode commented Mar 26, 2021

Since ultimately I decided not to introduce any changes based on this issue, I'm going to mark this as closed.

@salcode salcode closed this as completed Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants