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

Fatal Error on activation #35

Closed
danieliser opened this issue Jun 21, 2019 · 7 comments
Closed

Fatal Error on activation #35

danieliser opened this issue Jun 21, 2019 · 7 comments
Assignees
Labels

Comments

@danieliser
Copy link

danieliser commented Jun 21, 2019

An error of type E_ERROR was caused in line 35 of the file /wp-content/plugins/laps/src/Record/Collector/Plugin_Load_Collector.php. Error message: Uncaught TypeError: Argument 1 passed to Rarst\Laps\Record\Collector\Plugin_Load_Collector::plugin_loaded() must be of the type string, object given in /wp-content/plugins/laps/src/Record/Collector/Plugin_Load_Collector.php:35 Stack trace: #0 [internal function]: Rarst\Laps\Record\Collector\Plugin_Load_Collector->plugin_loaded(Object(EDD_Clean_Unused_Discounts))

 #1 /wp-includes/class-wp-hook.php(286): call_user_func_array(Array, Array)
 #2 /wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters('', Array)
 #3 /wp-includes/plugin.php(465): WP_Hook->do_action(Array)
 #4 /wp-settings.php(371): do_action('plugin_loaded', Object(EDD_Clean_Unused_Discounts))

In this case the EDD_Clean_Unused_Discounts class init calls which is totally valid.

add_action( 'plugins_loaded', array( $this, 'init' ) );

@Rarst
Copy link
Owner

Rarst commented Jun 24, 2019

Was travelling, just seeing this! Thank you for reporting!

I am a bit confused on how this happens, because plugin_loaded is explicitly documented to be passed a string in core and that's how it is called. Any other action attached to the hook should be irrelevant since it's action hook and all callbacks should be receiving same string input.

The snippet you suspect is a reason looks unrelated because pluginS_loaded is a different (old) hook from the (new) plugin_loaded, which error refers to.

Is the source code available for me to take a look?

@Rarst Rarst self-assigned this Jun 24, 2019
@Rarst Rarst added the bug label Jun 24, 2019
@danieliser
Copy link
Author

@Rarst You are correct, I misread that. I've never used plugin_loaded before so no idea.

Its a simple plugin so this is the entirity of it:

<?php
/*
Plugin Name: Easy Digital Downloads - Clean unused & expired discounts
Version: 1.0
Description: Clears expired & unused discounts in EDD daily.
Author: Daniel Iser
Author URI: http://presspowered.com

Original Author: Danny van Kooten
Original Author URI: http://dvk.co/
*/

class EDD_Clean_Unused_Discounts {

	const EVENT_NAME = 'edd_clear_unused_discounts';

	/**
	 * Constructor
	 */
	public function __construct() {}

	/**
	 * Add hooks
	 */
	public function add_hooks() {
		register_activation_hook( __FILE__, array( $this, 'schedule_event' ) );
		register_deactivation_hook( __FILE__, array( $this, 'clear_scheduled_event' ) );

		add_action( self::EVENT_NAME, array( $this, 'clear_discounts' ) );

		add_action( 'plugins_loaded', array( $this, 'init' ) );
	}

	public function init() {
		if ( current_user_can( 'manage_shop_settings' ) && isset( $_GET['clear_discounts'] ) ) {
			$this->clear_discounts();
		}
	}

	/**
	 * Schedule clean event
	 */
	public function schedule_event() {
		wp_schedule_event( time(), 'daily', self::EVENT_NAME );
	}

	/**
	 * Clear event when plugin is deactivated
	 */
	public function clear_scheduled_event() {
		wp_clear_scheduled_hook( self::EVENT_NAME );
	}

	/**
	 * Clear expired discounts with 0 uses
	 *
	 *
	 * To test run this debug query directly.
	 * SELECT a.ID, b.meta_value as expires, c.meta_value as uses FROM wp_postmeta b
	 * LEFT JOIN wp_posts a ON a.ID = b.post_id
	 * LEFT JOIN wp_postmeta c ON a.ID = c.post_id
	 * WHERE b.meta_key = '_edd_discount_expiration' AND b.meta_value != '' AND STR_TO_DATE(b.meta_value, '%m/%d/%Y %T') < NOW()
	 * AND c.meta_key = '_edd_discount_uses' AND c.meta_value IN ('', 0, '0')
	 * ORDER BY a.ID ASC
	 * LIMIT 500
	 */
	public function clear_discounts() {
		global $wpdb;

		$limit = 30;

		if ( current_user_can( 'manage_shop_settings' ) && isset( $_GET['clear_discounts'] ) ) {
			$limit = is_numeric( $_GET['clear_discounts'] ) ? $_GET['clear_discounts'] : 500;
		}

		$expired_and_unused_discounts = $wpdb->get_col("
SELECT a.ID, b.meta_value as expires, c.meta_value as uses FROM $wpdb->postmeta b 
LEFT JOIN $wpdb->posts a ON a.ID = b.post_id 
LEFT JOIN $wpdb->postmeta c ON a.ID = c.post_id
WHERE b.meta_key = '_edd_discount_expiration' AND b.meta_value != '' AND STR_TO_DATE(b.meta_value, '%m/%d/%Y %T') < NOW()
AND c.meta_key = '_edd_discount_uses' AND c.meta_value IN ('', 0, '0')
ORDER BY a.ID ASC
LIMIT $limit");

		if ( ! $expired_and_unused_discounts ) {
			return;
		}

		if( is_array( $expired_and_unused_discounts ) ) {
			foreach( $expired_and_unused_discounts as $discount_id ) {
				wp_delete_post( $discount_id, true );
			}
		}
	}
}

$plugin = new EDD_Clean_Unused_Discounts();
$plugin->add_hooks();

@Rarst
Copy link
Owner

Rarst commented Jun 25, 2019

Ah, I get what happens now. This plugin overwrites global $plugin variable, which holds the path to plugin file, being passed to hook after plugin is loaded.

That is way too generic name to mess with in global scope and I would strongly advise to change it to something prefixed/unique.

I am not sure there is a practical way to prevent this in core, but I will foolproof this a bit on Laps side.

@danieliser
Copy link
Author

Lol, so simple, yea I can change that in a few seconds.

WTH is the global $plugin var used for that would make any since haha. Seems like bad design on WP cores part lol.

If you already tested changing that to another var name and it works I guess you can consider this closed then 💯

@danieliser
Copy link
Author

That said pretty sure I copy/pasted that plugin template from somewhere, so you may see this again for others who used it lol.

@Rarst
Copy link
Owner

Rarst commented Jun 25, 2019

Well, core relies on global vars extensively, that is why best practice for plugins is to prefix the heck out of everything in global scope.

I still intend to adjust this in Laps, because otherwise I will just get this explode into someone's face repeatedly, if infrequently...

@danieliser
Copy link
Author

@Rarst lol fixed my plugin, appears its more common than we thought. This plugin also does it, modified it on our install and sent an email to them.

https://wordpress.org/plugins/smartlook-visitor-screen-recording/

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

No branches or pull requests

2 participants