Sharing service: Prevent various undefined array key warnings#43599
Sharing service: Prevent various undefined array key warnings#43599coder-karen merged 5 commits intotrunkfrom
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
…or any non object value including false.
| $this->total = (int) $total; | ||
|
|
||
| $this->name = $this->service->get_name(); | ||
| if ( is_object( $this->service ) ) { |
There was a problem hiding this comment.
Would it work to be more specific, and check for the Sharing_Source instead, so we know get_name will be in there?
There was a problem hiding this comment.
Yes, good point. I was also able to come up with steps to replicate the warning and test the fix now for this specific change too (although it looks straightforward to check if it's an instance of Sharing_Source, I wanted to be sure there wouldn't be additional errors) , so updated the testing instructions.
Fixes VULCAN-137
Proposed changes:
Warning: Undefined array key "sharing_label",Undefined array key "custom",Undefined array key "hidden",Undefined array key "visible", as well as a rarer fatal -Uncaught Error: Call to a member function get_name() on false$this->servicean instance ofSharing_Source, as it can be an bool.Other information:
Jetpack product discussion
pdWQjU-1g0-p2
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
I've been unable to replicate the warnings in practice, but it would seem they are likely happening when options are corrupted or not fully initialized. In most cases the warnings can be replicated by modifiying the relevant options values. So, a good way to test this is with a sandboxed Jurassic Ninja site. I recommend using a JN site because we're modifying options (so if you are using the JN site for anything else you may want to note the options when retrieveing them, and reset them back to original values afterwards):
?specialops=trueoptions - make sure to select 'Enable Sandbox access' (you don't need the Jetpack Beta tester plugin).wpshrun the following:tail -f /tmp/php-errors- and you should see undefined array key warnings related to thecustomandsharing_labelkeys.visibleandhiddenkeys now.The only other change is the check to see if
$this->serviceis an instance of Sharing_Source - this has only occurred twice over the last month, and proof-reading the change should be sufficient.Uncaught Error: Call to a member function get_name() on false) by directly setting$this->serviceto false here, before we set$this->name./wp-admin/admin.php?page=stats. You should see the warning on trunk$this->servicestill set to false, the warning should not exist.