woocommerce / woocommerce-admin

(Deprecated) This plugin has been merged to woocommerce/woocommerce
https://woocommerce.github.io/woocommerce-admin/#/
Other
361 stars 144 forks source link

Uncaught ArgumentCountError: Too few arguments to function orders_lookup_import_order #3169

Closed khag7 closed 5 years ago

khag7 commented 5 years ago

This is showing up in my server's error_log and it appears to be coming from this plugin.

If I'm not posting in the right place please point me in the right direction.

Thanks.

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function Automattic\WooCommerce\Admin\ReportsSync::orders_lookup_import_order(), 0 passed in /wp-includes/class-wp-hook.php on line 286 and exactly 1 expected in /wp-content/plugins/woocommerce-admin/src/ReportsSync.php:462
Stack trace:
#0 /wp-includes/class-wp-hook.php(286): Automattic\WooCommerce\Admin\ReportsSync::orders_lookup_import_order()
#1 /wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters('', Array)
#2 /wp-includes/plugin.php(531): WP_Hook->do_action(Array)
#3 /wp-content/plugins/woocommerce/includes/libraries/action-scheduler/classes/ActionScheduler_Action.php(22): do_action_ref_array('wc-admin_import...', Array)
#4 /wp-content/plugins/woocommerce/includes/libraries/action-scheduler/classes/ActionScheduler_Abstract_QueueRunner.php(59) in /wp-content/plugins/woocommerce-admin/src/ReportsSync.php on line 462
rrennick commented 5 years ago

@khag7 Thanks for the report. What versions of WC Admin and WooCommerce are you running?

khag7 commented 5 years ago

WooCommerce Version 3.7.1 WC Admin Version 0.21.0

These errors are also showing in the WooCommerce Scheduled Actions list. If I understand correctly, this is all scheduled through Action Scheduler and fails every time it runs. https://domain.com/wp-admin/admin.php?page=wc-status&tab=action-scheduler&status=failed

I'm getting the same error from a different plugin as well. Not sure what is causing scheduled actions to fail in this way, maybe its a problem on our end rather than a problem with your plugin.

rrennick commented 5 years ago

maybe its a problem on our end rather than a problem with your plugin.

I've had 30 instances of that action run in my dev install just this morning. What I'm trying to do is see if I can identify what is in your install that's causing it to fail. What other plugins do you have active? And, what theme?

khag7 commented 5 years ago

Sorry for the delay. I haven't had time to put into solving this and I didn't want to give you a list of 3 dozen plugins. I can almost guarantee this is going to be a 3rd party plugin, after seeing some of the plugins running on this client's installations. I have two pretty good guesses but I'll wait and see what I can find out. What I think is happening is a 3rd party plugin is somehow interfering with the Action Scheduler and then wc-admin and mailchimp-woocommerce are showing up in the error log because somehow bad data is getting put into the action scheduler. The action scheduler should probably be patched to prevent something like this in the future, but I'll wait until I can see for sure what the heck is going on before I point any fingers. Here's the plugins I am running on a clean install and getting the same error (intermittently, not with every order):

Mailchimp for WooCommerce Connects WooCommerce to Mailchimp to sync your store data, send targeted campaigns to your customers, and sell more stuff. Version 2.3 | By Mailchimp

WooCommerce An eCommerce toolkit that helps you sell anything. Beautifully. Version 3.7.1 | By Automattic

WooCommerce Admin A new JavaScript-driven interface for managing your store. The plugin includes new and improved reports, and a dashboard to monitor all the important key metrics of your site. Version 0.21.0 | By WooCommerce

WooCommerce Authorize.net Premium Premium quality Authorize.net extension for WooCommerce stores. Authorize.net Certified Solution. Based on updated Authorize.net APIs. Version 10.8 | By Indatos Technologies

(edited: I disabled 2 more and still getting the error with just those listed above. The 3 above are well developed pieces of software, the last is a non-official implementation of Authorize.NET and I'm fairly certain its the culprit. I will update when I figure out what is going on).

khag7 commented 5 years ago

Okay I figured out a specific condition that causes the error.

/*
Plugin Name: WooCommerce Save Order Bug
*/

add_action( 'save_post', function( $post_id ) {

    //set some early returns. we only want this to happen when saving a shop order 
    if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) return;
    if ( ! isset( $_POST['post_type'] ) ) return;
    if ( 'shop_order' != $_POST['post_type'] ) return;
    if ( ! current_user_can( 'edit_post', $post_id ) ) return;

    //now we're presumably saving a shop order

    //lets create a WC_Order from this post_id (this is the bugged part)
    $order = new WC_Order($post_id);

    //we don't even need to do anything with $order, the error will happen (or not)
    //but an example of something someone might be doing in this instance...
    //maybe we want to log every time one of our users saves an order
    //$order->add_order_note( "Order was updated by {$user_name}" );

});
  1. Clean install of WP.
  2. Install WooCommerce.
  3. Install the "Bug Plugin"
  4. Create a new order from the admin area, save it. Don't need to enter any items or customer. You can, bug will happen regardless.
  5. Install WooCommerce-Admin or Mailchimp-WooCommerce (probably any other plugin which makes use of the Action Scheduler and fires an action on a shop_order save).
  6. Go back to the order you created and update it. Just click update. Don't need to change anything. You can, but you don't need to.
  7. And you can update again and again and see the same error.

Now, that's obviously a very specific condition set, but its the only way I can reproduce the bug 100% of the time. Create the order, THEN activate a plugin which makes use of Action Scheduler, THEN update the order.

In my client's production site, they leave those plugins active all the time and they occasionally get this error. Not consistently. I'm able to force the error to be consistent by deactivating and reactivating the plugins. Usually when they are capturing payments via the Authorize.net plugin which makes use of the $order = new WC_Order($post_id) line I put into the Bugged plugin above.

So what's wrong here? Is this an Action Scheduler issue? Is there something wrong with using new WC_Order inside save_post?

rrennick commented 5 years ago

So what's wrong here?

The first thing that stands out is

$order = new WC_Order($post_id);

Your code should be using wc_get_order( $post_id ); to properly initialize the order object.

khag7 commented 5 years ago

The Authorize.NET plugin my client uses is pretty poorly coded. I'd prefer they use the SkyVerge version but this is already bought and paid for and "works" so its staying. Anyway, that plugin makes use of that line, and thats the line I was able to determine was possibly causing an error.

I updated my "bugged" plugin with your suggestion, and it seems to fix the issue.

I really appreciate your help with this. I'll apply the same fix to the plugin being used on our production site and see if it fixes the issue.

If you have time to put some thought into it... why does executing new WC_Order($post_id) result in this strange behavior and causes code execution to error FATALLY elsewhere, without itself even throwing an error. Further, that $order variable I created was never used. just the act of constructing the Order object caused failure elsewhere. Is this a bug in the WC_Order constructor that should be patched? Should the Action Scheduler team be alerted this is possible so they can do some sort of sanity check rather than a FATAL fail? I'm not necessarily concerned with raising hell over this when clearly its one poorly coded 3rd party plugin, but at the same time the WP development community tends to do its best to code in a way that expects awful 3rd party plugins to exist and handle errors gracefully.

khag7 commented 5 years ago

I updated the "bugged" plugin but added a reference to $order, specifically trying to set an order note using the object I created "correctly" using wc_get_order. Interestingly, this still fails.

  1. Create a new blank order.
  2. Deactivate and reactivate WooCommerce Admin
  3. Go back to order and update it.
/*
Plugin Name: WooCommerce Save Order Bug
*/

add_action( 'save_post', function( $post_id ) {
    if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) return;
    if ( ! isset( $_POST['post_type'] ) ) return;
    if ( 'shop_order' != $_POST['post_type'] ) return;
    if ( ! current_user_can( 'edit_post', $post_id ) ) return;

    $order = wc_get_order( $post_id );
    $order->add_order_note( "Order was updated" );

});
rrennick commented 5 years ago

@khag7 I'm going to close this issue as a discussion carried out across 2 repositories is confusing for anyone not directly in the conversation.

khag7 commented 5 years ago

Seems reasonable, It really doesn't need to be on this repo now that its obvious this isn't an issue specific to this repo. Appreciate your help and time.