Launchpad: Adjust Logstash ref to fix fatal on WoA sites#31284
Conversation
|
Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run to get started. More details: p9dueE-5Nn-p2 |
|
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 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Mu Wpcom plugin:
|
| * @param array $task_definitions The tasks to initialize. | ||
| */ | ||
| function wpcom_launchpad_init_listeners( $task_definitions ) { | ||
| require_once WP_CONTENT_DIR . '/lib/log2logstash/log2logstash.php'; |
There was a problem hiding this comment.
If you still wanted to do this on simple wpcom for now, could you use a file_exists check before the require and function_exists check in the catch?
There was a problem hiding this comment.
Is the IS_WPCOM global defined on Atomic sites? That might be a simpler check... looks like that's what we do in launchpad.php, though that's also filtering by blog ID.
There was a problem hiding this comment.
Ah yes, if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) would work for checking simple wpcom sites. That constant shouldn't be on Atomic sites, it's how the status package checks for example:
jetpack/projects/packages/status/src/class-host.php
Lines 61 to 68 in b5e0513
|
Verified this will correct the issue. If we want to merge as-is, will need to add a changelog and fix the lint error about empty catch block. |
|
I'm headed out for the weekend, @nelsonec87 or @andres-blanco please feel free to commandeer this PR and get it shipped to avoid blocking deployment to WoA. Thank you!! |
Drop verbose framing and references outside wpcomsh / jetpack (log2logstash, RFC 4648, libsodium-seal aside, fix #31284, memcached/Atomic detail, Error_Handler precedent). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes #31283
Proposed changes:
IS_WPCOMcheck to prevent fatal errors on WoA sites, and return WP_Error if task initialization fails for any site.Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
/launchpaddirectory, not just this patched file. You'll find the files in/wpcomsh/vendor/automattic/jetpack-mu-wpcom/src/features/launchpad/tail -F /tmp/php-errorson the WoA site