10up / ElasticPress

A fast and flexible search and query engine for WordPress.
https://elasticpress.io
GNU General Public License v2.0
1.24k stars 312 forks source link

Trying to get property of non-object in /elasticpress/classes/class-ep-api.php on line 690 #881

Closed halmos closed 6 years ago

halmos commented 7 years ago

Have you searched for similar issues before submitting this one? Yes

Is this a bug, question or feature request? Bug

Describe the issue you encountered: When there is an error with a taxonomy and WP returns taxonomy with value offalse, a Trying to get property of non-object error is thrown. At https://github.com/librarystack/ElasticPress/blob/develop/classes/class-ep-api.php#L690, EP tries to access object property $taxonomy->name. We need a sanity check prior to this to prevent the error . Addding if(! $taxonomy) continue; on previous line fixes issue.

Current WordPress version: 4.8

Current ElasticPress version: 2.3.2

Current Elasticsearch version: 1.5

Where do you host your Elasticsearch server: AWS

ivankristianto commented 7 years ago

@halmos i'm wondering if you altered the $selected_taxonomies using filter ep_sync_taxonomies via plugin/themes? https://github.com/librarystack/ElasticPress/blob/develop/classes/class-ep-api.php#L679

halmos commented 7 years ago

Hi @ivankristianto - No - we're not making use of the ep_sync_taxonomy filter. However, initial testing suggest there may be a conflict with another plugin (https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront) - or at least deactivating this plugin prevents the error from occurring. Possibly it is doing something unexepected with the taxonomies, or one of the filters we're using with that plugin is causing issue. In any case, it seems like checking for valid taxonomy object is a safe idea.

Here's a stack trace of the error for your reference:

[02-Jul-2017 10:48:50 UTC] PHP Stack trace:
[02-Jul-2017 10:48:50 UTC] PHP   1. {main}() /srv/www/librarystack/htdocs/wp-admin/post.php:0
[02-Jul-2017 10:48:50 UTC] PHP   2. wp_set_post_lock($post_id = 6092) /srv/www/librarystack/htdocs/wp-admin/post.php:148
[02-Jul-2017 10:48:50 UTC] PHP   3. update_post_meta($post_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992530:1', $prev_value = *uninitialized*) /srv/www/librarystack/htdocs/wp-admin/includes/post.php:1503
[02-Jul-2017 10:48:50 UTC] PHP   4. update_metadata($meta_type = 'post', $object_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992530:1', $prev_value = '') /srv/www/librarystack/htdocs/wp-includes/post.php:1781
[02-Jul-2017 10:48:50 UTC] PHP   5. do_action($tag = 'updated_postmeta', $arg = '195629', 6092, '_edit_lock', '1498992530:1') /srv/www/librarystack/htdocs/wp-includes/meta.php:280
[02-Jul-2017 10:48:50 UTC] PHP   6. WP_Hook->do_action($args = array (0 => '195629', 1 => 6092, 2 => '_edit_lock', 3 => '1498992530:1')) /srv/www/librarystack/htdocs/wp-includes/plugin.php:453
[02-Jul-2017 10:48:50 UTC] PHP   7. WP_Hook->apply_filters($value = '', $args = array (0 => '195629', 1 => 6092, 2 => '_edit_lock', 3 => '1498992530:1')) /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:323
[02-Jul-2017 10:48:50 UTC] PHP   8. EP_Sync_Manager->action_queue_meta_sync($meta_id = '195629', $object_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992530:1') /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:298
[02-Jul-2017 10:48:50 UTC] PHP   9. ep_prepare_post($post_id = 6092) /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-sync-manager.php:132
[02-Jul-2017 10:48:50 UTC] PHP  10. EP_API->prepare_post($post_id = 6092) /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-api.php:2566
[02-Jul-2017 10:48:50 UTC] PHP  11. EP_API->prepare_terms($post = class WP_Post { public $ID = 6092; public $post_author = '1'; public $post_date = '2017-02-01 12:15:03'; public $post_date_gmt = '2017-02-01 17:15:03'; public $post_content = ''; public $post_title = 'eflux journal 2'; public $post_excerpt = ''; public $post_status = 'publish'; public $comment_status = 'closed'; public $ping_status = 'open'; public $post_password = ''; public $post_name = 'eflux-journal-2'; public $to_ping = ''; public $pinged = ''; public $post_modified = '2017-06-27 21:42:17'; public $post_modified_gmt = '2017-06-28 01:42:17'; public $post_content_filtered = ''; public $post_parent = 0; public $guid = 'http://librarystack.dev/?p=6092'; public $menu_order = 0; public $post_type = 'post'; public $post_mime_type = ''; public 

and slightly different stacktrace producing the same error:

[02-Jul-2017 10:49:38 UTC] PHP Notice:  Trying to get property of non-object in /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-api.php on line 691
[02-Jul-2017 10:49:38 UTC] PHP Stack trace:
[02-Jul-2017 10:49:38 UTC] PHP   1. {main}() /srv/www/librarystack/htdocs/wp-admin/admin-ajax.php:0
[02-Jul-2017 10:49:38 UTC] PHP   2. do_action($tag = 'wp_ajax_heartbeat', $arg = *uninitialized*) /srv/www/librarystack/htdocs/wp-admin/admin-ajax.php:91
[02-Jul-2017 10:49:38 UTC] PHP   3. WP_Hook->do_action($args = array (0 => '')) /srv/www/librarystack/htdocs/wp-includes/plugin.php:453
[02-Jul-2017 10:49:38 UTC] PHP   4. WP_Hook->apply_filters($value = '', $args = array (0 => '')) /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:323
[02-Jul-2017 10:49:38 UTC] PHP   5. wp_ajax_heartbeat('') /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:298
[02-Jul-2017 10:49:38 UTC] PHP   6. apply_filters($tag = 'heartbeat_received', $value = array (), array ('wp-refresh-post-lock' => array ('post_id' => '6092', 'lock' => '1498992563:1')), 'post') /srv/www/librarystack/htdocs/wp-admin/includes/ajax-actions.php:2813
[02-Jul-2017 10:49:38 UTC] PHP   7. WP_Hook->apply_filters($value = array (), $args = array (0 => array (), 1 => array ('wp-refresh-post-lock' => array ('post_id' => '6092', 'lock' => '1498992563:1')), 2 => 'post')) /srv/www/librarystack/htdocs/wp-includes/plugin.php:203
[02-Jul-2017 10:49:38 UTC] PHP   8. wp_refresh_post_lock($response = array (), $data = array ('wp-refresh-post-lock' => array ('post_id' => '6092', 'lock' => '1498992563:1')), $screen_id = 'post') /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:298
[02-Jul-2017 10:49:38 UTC] PHP   9. wp_set_post_lock($post_id = 6092) /srv/www/librarystack/htdocs/wp-admin/includes/misc.php:798
[02-Jul-2017 10:49:38 UTC] PHP  10. update_post_meta($post_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992578:1', $prev_value = *uninitialized*) /srv/www/librarystack/htdocs/wp-admin/includes/post.php:1503
[02-Jul-2017 10:49:38 UTC] PHP  11. update_metadata($meta_type = 'post', $object_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992578:1', $prev_value = '') /srv/www/librarystack/htdocs/wp-includes/post.php:1781
[02-Jul-2017 10:49:38 UTC] PHP  12. do_action($tag = 'updated_postmeta', $arg = '195629', 6092, '_edit_lock', '1498992578:1') /srv/www/librarystack/htdocs/wp-includes/meta.php:280
[02-Jul-2017 10:49:38 UTC] PHP  13. WP_Hook->do_action($args = array (0 => '195629', 1 => 6092, 2 => '_edit_lock', 3 => '1498992578:1')) /srv/www/librarystack/htdocs/wp-includes/plugin.php:453
[02-Jul-2017 10:49:38 UTC] PHP  14. WP_Hook->apply_filters($value = '', $args = array (0 => '195629', 1 => 6092, 2 => '_edit_lock', 3 => '1498992578:1')) /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:323
[02-Jul-2017 10:49:38 UTC] PHP  15. EP_Sync_Manager->action_queue_meta_sync($meta_id = '195629', $object_id = 6092, $meta_key = '_edit_lock', $meta_value = '1498992578:1') /srv/www/librarystack/htdocs/wp-includes/class-wp-hook.php:298
[02-Jul-2017 10:49:38 UTC] PHP  16. ep_prepare_post($post_id = 6092) /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-sync-manager.php:132
[02-Jul-2017 10:49:38 UTC] PHP  17. EP_API->prepare_post($post_id = 6092) /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-api.php:2566
[02-Jul-2017 10:49:38 UTC] PHP  18. EP_API->prepare_terms($post = class WP_Post { public $ID = 6092; public $post_author = '1'; public $post_date = '2017-02-01 12:15:03'; public $post_date_gmt = '2017-02-01 17:15:03'; public $post_content = ''; public $post_title = 'eflux journal 2'; public $post_excerpt = ''; public $post_status = 'publish'; public $comment_status = 'closed'; public $ping_status = 'open'; public $post_password = ''; public $post_name = 'eflux-journal-2'; public $to_ping = ''; public $pinged = ''; public $post_modified = '2017-06-27 21:42:17'; public $post_modified_gmt = '2017-06-28 01:42:17'; public $post_content_filtered = ''; public $post_parent = 0; public $guid = 'http://librarystack.dev/?p=6092'; public $menu_order = 0; public $post_type = 'post'; public $post_mime_type = ''; public $comment_count = '0'; public $filter = 'raw' }) /srv/www/librarystack/htdocs/wp-content/plugins/elasticpress/classes/class-ep-api.php:595
halmos commented 7 years ago

On further investigation I can confirm that the source of the error is actually emanating from https://github.com/10up/ElasticPress/blob/develop/features/woocommerce/woocommerce.php#L137

I should i have noted that i have WC 2.6.8 installed. It seems that it cannot be assumed that product_visibility taxonomy is available.

ep_wc_whitelist_taxonomies hooks into ep_sync_taxonomies:

In my case, $product_visibility = get_taxonomy( 'product_visibility' ); returns false which results in non-taxononomy-object being added to $woo_taxonomies to array which causes error above.

I'm wondering if ep_wc_whitelist_taxonomies should apply only to WC custom posts (and not general posts?), or at the very least validate the taxonomy.

ivankristianto commented 7 years ago

@halmos thanks you for digging this. I think it is a good idea to add the validation on the taxonomy before proceed.

ivankristianto commented 7 years ago

So after digging this I need to do 2 things:

  1. Need to fix product_visibility tax in woocommerce feature. Check if it is available or not before push to array.
  2. Need to add the sanity check to $taxonomy. If it is false, bail
tlovett1 commented 6 years ago

Hey @ivankristianto I had to remove the is_a WP_Taxonomy check is WP_Taxonomy does not exist prior to 4.7. Is there another solution here?

ivankristianto commented 6 years ago

@tlovett1 I added another PR: https://github.com/10up/ElasticPress/pull/939 It checked the $taxonomy object property name