User tests: Successful: Unsuccessful:
Pull Request for Issue #39457.
Joomla gracefully handles errors and presents a nice interface to the admin with the ability to "return to dashboard". However, if the error is caused by a system plugin, the administrator interface becomes unusable.
Within the ExceptionHandler
class, this pull request tries to identify a plugin directory from the error trace log, and provide a button for the admin to disable it. If the admin consents, the plugin is disabled.
The whole operation is completed within the ExceptionHandler
class itself, because we are uncertain of whether the CMS can reach any other existing structure, since the plugin error is halting normal execution.
Edit any enabled legacy system plugin files and write erroneous code. This pull request should be able to handle parse errors and fatal errors, but not compile errors, which are unlikely for a published plugin. Warnings and notices are not caught by the Joomla exception handler, because the administrator remains usable.
Example of a parse error:
print 'a but no-semi-colon'
print 'b';
Example of a fatal error:
if (JFactory::getApplication()->isAdmin()) { return true; } // Deprecated since 2014 as Brian said!
Joomla shows the common error page and cannot recover by means of the administrator interface.
A message should be enqueued and shown in the message area, with a button to disable the plugin. Since that sometimes fails, depending on how deep the app execution has gone before the plugin error, another disable button has been added in the Atum administrator error_full.php
file, at the bottom, next to "Return to Dashboard".
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Category | ⇒ | Administration Language & Strings Templates (admin) Libraries |
Status | New | ⇒ | Pending |
It only works for legacy plugin events. Those are the main cause of the problem.
Then you need to update the test instructions and provide an example. I just tested exactly what you said to test
I have tested this item
Way to ignore the fact that there are valid reasons for plugins to throw exceptions.
There are a lot of code style errors reported here by PHPCS: https://ci.joomla.org/joomla/joomla-cms/60302/1/6
Then you need to update the test instructions and provide an example. I just tested exactly what you said to test
Initial comment updated.
Way to ignore the fact that there are valid reasons for plugins to throw exceptions.
This is not a wrapper around plugin exceptions. It occurs when the administrator has failed and is wrapping up using the ExceptionHandler
class as a last chance to show some interface. It only happens when a system plugin has crippled the administrator.
There are a lot of code style errors reported here by PHPCS: https://ci.joomla.org/joomla/joomla-cms/60302/1/6
Thank you, I will look at these.
This is not a wrapper around plugin exceptions. It occurs when the administrator has failed and is wrapping up using the
ExceptionHandler
class as a last chance to show some interface. It only happens when a system plugin has crippled the administrator.
It catches anything thrown by plugins, something that absolutely should not be done. It is not your concern what the plugins throws or why. This PR is conceptually wrong and should be closed.
It catches anything thrown by plugins, something that absolutely should not be done.
Of course not, but in my tests it only catches what bricks your administrator until you manually fix it. Can you provide an example of what you're saying?
I assume that this is the intended result ?
Yes, Brian, indeed. Thank you. I am open to suggestions.
Still, I'm stressing out that we are losing people to this silly thing every day.
It catches anything thrown by plugins, something that absolutely should not be done.
Of course not, but in my tests it only catches what bricks your administrator until you manually fix it.
@mavrosxristoforos Well, you catch any throwable. And that might include exceptions which should not be caught because the backend would still be bricked or malfunction when you don't catch them and disable a plugin because they are not caused by that plugin but by something under the hood. At least that's what I think @SharkyKZ could have in mind.
I have an idea how that could be sorted out, but I don't know if it's good or not.
Let's say we bring back the old methods isAdmin and isSite, but not as a proxy for the isClient calls (which never would work because e.g. !isAdmin()
will never mean that it is site when mapping a boolean to a 4 state value). We let these methods just throw special, dedicated, new exceptions when they are called, nothing else. Then you don't just catch any callable where you do that right now, you catch only these special exception types.
This will limit the functionality added by this PR to foreseen cases (calls to the old isAdmin or isSite methods, probably also other cases could be implemented, too).
What do you guys and other readers think about that?
@richard67 Handling specific errors is definitely a good idea!
I think, though, that re-introducing isAdmin
and isClient
to return a different thing may become even more frustatring for extension developers. If it just throws a dedicated Exception
, then that's ok IMO.
However, I feel I must insist that I am not handling every error here. Please read the rest of the ExceptionHandler
class. This method is only invoked right before rendering the Joomla error screen from Brian's screenshot. The error log does not include plugin files unless the error is directly caused during their execution. I only address administrator errors that have a system plugin invoking them somewhere in the process. Can you provide any example in which this PR is handling legitimate errors that are not causing the whole administrator to crash?
Still, if we handle dedicated exceptions, that's ok with me. I am open to suggestions. I just want to see Joomla giving an option to admins that do not know how to manually fix their broken administrator.
What happens if you don't have permission to disable the plugin?
In general it is not bad to have some kind of safe mode. But catching everything is dangerous. The first use case which pops up in my mind is when a security plugin throws an exception because of malicious behavior. With this pr can then the bad user disable that security plugin. This should definitely not be possible.
Labels |
Added:
Language Change
?
?
|
is this abandoned?
Not yet. I'm hoping to be able to commit an update soon. I have implemented all suggestions mentioned in this thread, but haven't been able to test them yet. I'm having trouble with re-setting up the test environment...
This is a very dangerous PR and should not even be considered.
Here are some real world failure modes I have seen:
I could go on.
There are multiple problems with this approach.
First of all, regardless of whether the plugin was innocent or not, killing it instead of reporting the error makes it outright impossible to troubleshoot the problem. This is a very well-known, well-understood issue which plagues WordPress and makes our lives as developers miserable.
You will inevitably end up disabling a critical plugin which must never be disabled. For example, the core has WebAuthn, the Joomla user plugin, and the MFA plugins. Disabling them will end up making it impossible to log into the site, or degrade the site's security. In most real world sites there are critical third party plugins such as security plugins, sales processing, transactional email sending, extension frameworks, site builder plugins etc. Disabling any of these ends up acting as unattended bricking of the site. Yikes!
In most cases you will be killing an innocent plugin caught up in something which is not its fault, or one which deliberately throws an Exception to protect the site, and have Joomla slander it as broken. This opens OpenSourceMatters, Inc to legal liability for defamation and loss of profit.
Inevitably, us 3PDs will attempt to around the problem caused by Joomla by gimping the dangerously bad core feature, like we've done before (see Joomla Update and how it determines if our extensions have a migration path to a new Joomla version). In the case of this error handler we'll just add our own error handler around our plugin event handlers and classes which simply issues our own HTTP headers and message and then die()
s. At best, this creates a jarring experience for users. At worst, when employed by 3PDs who don't have the same experience with core code and error handling me and other top developers have, will lead to broken sites e.g. if they register a global error handler while unregistering Joomla's built-in error handler.
I am not completely against an error handler. I am against this implementation. An error handler, used wisely, can help with both objectives of a. troubleshooting error conditions and b. being able to operate the site in a "safe mode".
The current error handler (Symfony ErrorHandler) is great for backend developers but sucks for the moderately technical and non-technical site integrators and site owners, i.e. the majority of Joomla users.
The site integrator / owner wants to know which extension the error originated from so they can ask for support and, possibly, disable it temporarily until help comes their way.
I would propose displaying the following additional information in the error handler page:
I will ask @joomla/joomla-experience-team-jxt to chip in because this is a very important user experience element. We want to improve UX for Joomla, not make people's lives harder.
@nikosdion Γεια σου Νίκο! So glad to read your ideas. Thank you for brainstorming us with all this valuable input on how to handle a REAL problem, that I feel as if I'm the only one facing with Joomla.
Let me point out once again, that this PR specifically handles SYSTEM plugins. Neither security plugins, nor user plugins.
However, this (what you wrote in your post) is exactly what I have been looking for, ever since starting issue #39457.
Still, it is important to state that this PR only ever occurs when the core has failed and is wrapping up just to show some interface. So, it only makes sense to assume that this interface must actually allow you to do something to solve the problem.
There are many valid reasons why an admin may not have FTP access to a website, and resorting to external tools to fix the problem is, still, IMHO bad UX...
@mavrosxristoforos Γειά σου, πατρίδα :)
Let me point out once again, that this PR specifically handles SYSTEM plugins. Neither security plugins, nor user plugins.
All the examples I had used are actually things I have seen implemented as system plugins
I can tell you with the authority of someone who's made and maintains a security extensions for Joomla that all security plugins —not just mine— are system plugins, by necessity. We have to run as early as we can during the Joomla application initialisation. This means system plugin and onAfterInitialise. We also need to handle just over 20 event types, making the system plugin a far more appealing solution than a scattering of 4–6 different plugins on top of the system plugin we need anyway.
Extension frameworks also tend to be system plugins because they need to do initialisation very early in the application boot up. One prime example, though I am not sure if it's still relevant in Joomla 4, is Koowa a.k.a. Nooku Framework.
For whatever reason (I have not taken on through a step by step debugger... yet!) page builder plugins and similar, uh, widget solutions (hint hint wink wink) are also system plugins. If these plugins are unpublished the frontend fails miserably and possibly uncovering some internals which should remain hidden.
Handling payment callbacks essentially requires having your own endpoint. Normally, this is through com_ajax and a plugin of a custom type. Some recalcitrant payment providers do not accept a URL with query string parameters (the previous Alpha Bank API in Greece for example, as I'm sure you know) so you either need a random .php file as your entry point (BAD!) or a system plugin which essentially handles its own, fixed route (e.g. /com_myecommerce/callback/weird_bank.html).
Plugins doing scheduled tasks such as sending emails also tend to be system plugins because the Scheduled Tasks feature is new, undocumented, and people still need to support as far back as Joomla 3.10 in the same codebase.
I have seen all of these on real world sites, I realise few core contributors have, that's why I raised the alarm
However, this (what you wrote in your post) is exactly what I have been looking for, ever since starting issue #39457.
Good!
There are many valid reasons why an admin may not have FTP access to a website, and resorting to external tools to fix the problem is, still, IMHO bad UX...
Yes, I thought about that too and I gave you half of the solution: the instructions to disable the extension includes a link to the extensions manager page.
Now, what about a failure which prevents that from working at all? I have a solution for that, but I did not explain it above because it's more involved. You're interested so let me go ahead.
As long as the logged in user has the core.edit_state privilege on com_plugins the instructions to disable the plugin can also include a single use, time-limited URL to disable the plugin. The idea is that upon hitting the error you create a secure token in #__user_keys
with an expiration time 60 seconds into the future. When a special URL, let's say /administrator/index.php?__joomlaDisablePlugin=123&__token=blahblah
is accessed, the AdministratorApplication removes the token, disables the referenced system plugin, and clears the system
cache. You can ensure only the right plugin is removed by putting its ID in the token series, e.g. set the series to something like __joomla_error_admin_plugin_123
. Therefore only the specific user, from the specific user agent, can disable a specific plugin, within a minute of the plugin throwing an error.
The observant reader may have recognised this approach as being similar to the way Admin Tools' Rescue Mode works to disable plg_system_admintools. This is indeed the case. What I am proposing is indeed something we have been successfully using in production in our own software the last 3 years. I didn't just pluck hair off my butt.
This pull request has been automatically rebased to 5.0-dev.
This pull request has been automatically rebased to 5.1-dev.
This pull request has been automatically rebased to 5.2-dev.
Title |
|
This has obviously been abandoned and should be closed
Yes, I'm currently not working on it. I would appreciate any input to restart it. I believe I'm not the only one seeing the need to maintain a usable back-end.
This pull request has been automatically rebased to 5.3-dev.
Title |
|