joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.1] Error handling on extension update

Open BrainforgeUK opened this issue 10 months ago • 27 comments

Pull Request for Issue #43319 .

Summary of Changes

Catch errors from the extension being updated and abort update with a sensible message.

Testing Instructions

(a) Installed this Plugin. plg_updatetest1.zip

(b) Enable the plugin.

(c) Go to Joomla updater and check for updates.

(d) Then select the plugin installed above PLG_HIKASHOPSHIPPING_BFEXTRAFEATURES and update it.

Actual result BEFORE applying this Pull Request

Error 0

Expected result AFTER applying this Pull Request

Error 1

Link to documentations

Please select:

  • [ ] Documentation link for docs.joomla.org:

  • [x] No documentation changes for docs.joomla.org needed

  • [ ] Pull Request link for manual.joomla.org:

  • [x] No documentation changes for manual.joomla.org needed

BrainforgeUK avatar Apr 20 '24 13:04 BrainforgeUK

Cannot be tested image

brianteeman avatar Apr 20 '24 15:04 brianteeman

Try this test plugin - screenshots before applying the pull request changes. plg_updatetest2.zip

Error 10 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Error 15 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Error 12 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Error 16

BrainforgeUK avatar Apr 20 '24 17:04 BrainforgeUK

image

brianteeman avatar Apr 20 '24 17:04 brianteeman

Test does not work on Unix system! - must be filename case, investigating.

BrainforgeUK avatar Apr 20 '24 17:04 BrainforgeUK

I am using windows

brianteeman avatar Apr 20 '24 17:04 brianteeman

Sorted, no change to the above needed, can go ahead with testing. Appologizes for inconvenience, update server issue, not looked at that for a long time!

BrainforgeUK avatar Apr 20 '24 18:04 BrainforgeUK

With Debug Disabled image

With Debug Enabled image

PLEASE check your work before submitting it

brianteeman avatar Apr 20 '24 19:04 brianteeman

Changed.

Sorry, copied the changed code across and forgot the use statement. Failed to check the files changed count - thought I had copied the language text across.

BrainforgeUK avatar Apr 20 '24 19:04 BrainforgeUK

You also forgot to check the language string you used and the string you created.

brianteeman avatar Apr 20 '24 22:04 brianteeman

Retested as: https://github.com/joomla/joomla-cms/pull/43321#issuecomment-2067737115

With changes made the message is now:

restest2

BrainforgeUK avatar Apr 22 '24 16:04 BrainforgeUK

I am all in favour of meaningfull error messages but they also have to be helpful. In this example the advice is not helpful at all as it means nothing at all to the user. This is a message that is only useful to the original extension developer and is not something that an end user shoudl ever see. Extension developers dont release updates without testing they work do they

brianteeman avatar Apr 22 '24 21:04 brianteeman

Regardless of who is to blame for the error what I propose is better and more useful than what Joomla currently displays.

This is actually not a plugin bug - the plugin is used by and depends on a 3rd party extension, it is that dependency which causes the problem when the update is attempted with the plugin enabled. Could modify such plugins to protect against such errors but I see that as unnecessary code in the plugin when Joomla can handle it more gracefully.

I suppose could add another line to the error message: "Please contact the extension developer."

Could enhance further by adding contact information from current XML?

BrainforgeUK avatar Apr 22 '24 21:04 BrainforgeUK

The question is. Does the new message help the end user more than the original message. For me the answer is no

brianteeman avatar Apr 22 '24 22:04 brianteeman

The end user is an admin who hopefully knows the extensions installed on their site.

The current error is missleading as it implies it is a bug in the Joomla installer ... which led to me being asked a question which started me on this.

BrainforgeUK avatar Apr 22 '24 22:04 BrainforgeUK

I think that this helps "Please contact the extension developer of the Plugin ... ". Then I know that it is not a bug in Joomla or in my confirguration and don't waste time with debugging.

chmst avatar May 09 '24 11:05 chmst

@chmst the problem is that according to @BrainforgeUK the problem is not with the developer of this plugin but with the developer of a completely different extension upon which it depends - so which extension developer am I going to contact? For me that is not the problem. The problem is that the extension that is being installed is not checking for the presence of the extension it is dependent upon. ergo nothing sensible can be changed in core to fix an extension developers errors

brianteeman avatar May 09 '24 12:05 brianteeman

@brianteeman - thanks, agreed.

chmst avatar May 09 '24 12:05 chmst

there is only so much that Joomla can do. Extension developers need to take responsibility for their own code being installed correctly.

brianteeman avatar May 09 '24 12:05 brianteeman

should this PR get test or not?

fgsw avatar Aug 16 '24 09:08 fgsw

I do't see why not. Improves on what is there already but as indicated in previous comments it is not perfect. There can never be a perfect solution.

BrainforgeUK avatar Aug 16 '24 13:08 BrainforgeUK

I do't see why not.

I don't see positive comments. Don't wanna waste my time.

fgsw avatar Aug 16 '24 14:08 fgsw

I have tested this item :red_circle: unsuccessfully on bb1c6aa4acc1d4944f815b6f0950579616ac4582

Hi BrainForgeUK, I have really tried to test this during PBF 2024, and I am sorry, but neither pig_updatetest1.zip nor png_updatetest2.zip yield the results that you show/want... perhaps because we are testing it now against Joomla 5.2.0-beta2-dev but couldn't get it to work out ;(


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

I should specify that I considered this a fail since it did NOT include the path line and just showed the first 2 lines:

Error updating: PLG_BF_MANUAL Class "hikashopShippingPlugin" not found

exlemor avatar Aug 24 '24 09:08 exlemor

I have tested this item :white_check_mark: successfully on bb1c6aa4acc1d4944f815b6f0950579616ac4582

Tested successfully. Seems like a no brainer to me to improve this.

I've tested on J5.1.3 and then there was no path visible (3rd line in the alert) but on 5.2.0-beta2-dev it was fine.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

TLWebdesign avatar Aug 24 '24 10:08 TLWebdesign

I have tested this item :white_check_mark: successfully on bb1c6aa4acc1d4944f815b6f0950579616ac4582

Works as described.

Thank you for this PR! The new error message has more details, so it helps the website admins understand the issue better when they get such an error.

btw: I've tested this PR successfully on 5.2-dev: screencapture-local-test-nl-administrator-index-php-2024-08-24-12_27_09


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

pe7er avatar Aug 24 '24 10:08 pe7er

On Joomla 5.1.3 i'm not seeing the path. I tested it both on a localhost and on a live test server. See attached screenshot: Scherm­afbeelding 2024-08-24 om 12 45 57

TLWebdesign avatar Aug 24 '24 10:08 TLWebdesign

@TLWebdesign as this PR is for 5.1 and not 5.2 then you should be marking this as an unsuccessful test

brianteeman avatar Aug 24 '24 16:08 brianteeman

I have tested this item :red_circle: unsuccessfully on bb1c6aa4acc1d4944f815b6f0950579616ac4582

On 5.1.3 i'm not getting the path to the file to show.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

TLWebdesign avatar Aug 24 '24 17:08 TLWebdesign

This pull request has been automatically rebased to 5.2-dev.

HLeithner avatar Sep 02 '24 08:09 HLeithner

I have tested this item :white_check_mark: successfully on bb1c6aa4acc1d4944f815b6f0950579616ac4582

Tested Succesfully on 5.2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

TLWebdesign avatar Sep 02 '24 08:09 TLWebdesign

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43321.

Quy avatar Oct 28 '24 05:10 Quy