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

Fail gracefully when errors are detected #251

Merged
merged 1 commit into from
Jun 19, 2021

Conversation

christianwach
Copy link
Member

Overview

Rewrites the error checking that the WordPress plugin performs and fails gracefully instead of throwing fatal errors and making WordPress inoperable. Introduces a "CiviCRM Troubleshooting" page to help diagnose and fix problems.

Before

When the PHP version was less than the required version, CiviCRM would call wp_die() and make WordPress inoperable. Other error checking during initialize was either never performed or never acted upon.

After

New framework for handling errors that provides a configurable "CiviCRM Troubleshooting" page to help diagnose and fix problems when they are encountered.

The plugin header Requires PHP: parameter prevents installation when an insufficient PHP version is detected. However, this does not handle the same situation after a CiviCRM upgrade. This Troubleshooting page is now shown instead of CiviCRM's admin UI:

Screenshot 2021-06-18 at 15 12 48

It's more difficult to handle broken file systems - but, where possible, the following Troubleshooting page is now shown instead of CiviCRM's admin UI. (Try renaming civicrm/CRM/Core/Config.php to civicrm/CRM/Core/Config.php.bak to see this page)

Screenshot 2021-06-18 at 15 11 24

Whilst not complete in its coverage of possible errors, this code can be developed to be more comprehensive in future. The major improvement is that WordPress does not stop functioning whilst problems are attended to.

@christianwach
Copy link
Member Author

christianwach commented Jun 18, 2021

Hmm, not sure why this is failing - civilint passes and I don't know what No report files were found. Configuration error? means. @seamuslee001 Is this a problem with Jenkins?

@seamuslee001
Copy link
Contributor

@christianwach

===============================[ php -l ]===============================
PHP Parse error:  syntax error, unexpected ')' in assets/templates/metaboxes/metabox.error.help.php on line 26

Parse error: syntax error, unexpected ')' in assets/templates/metaboxes/metabox.error.help.php on line 26

Errors parsing assets/templates/metaboxes/metabox.error.help.php
===============================[ phpcs  ]===============================

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@kcristiano
Copy link
Member

@christianwach I have a server to test with multiple obsolete php versions. The code looks good - once the linting error is updated and the file name question is taken care of I can test.

Thanks for this it's a great improvement

@christianwach
Copy link
Member Author

Jenkins re test this please

@seamuslee001
Copy link
Contributor

@christianwach
Copy link
Member Author

@seamuslee001 Oh my. There must be something screwy with my civilint then grimaces.

@christianwach
Copy link
Member Author

Jenkins re test this please

@christianwach
Copy link
Member Author

Ah dammit, going to have to go through all the files individually it seems.

@kcristiano
Copy link
Member

@christianwach Thanks for this.

On an r-run with the Nightly tarball we are trapped as you detailed. No way out at all as the code need php 7.2+ and all we get is an error screen.

Applying the patch brings us to an info page and we do not crash the site, the admin.

Upgrade php, reload and the upgrader works as expected.

@kcristiano kcristiano merged commit b2b0529 into civicrm:master Jun 19, 2021
@christianwach christianwach deleted the install branch June 21, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants