Joomla! Issue Tracker | Joomla! CMS #39965 - [5.0] Smart Search: Improving error handling of Indexing

Language Change bug PR-5.0-dev

Fixed in Code Base
30 May 2023
Medium
Build: 4.3-dev
# 39965
Diff
Hackwar:4.3-finder-errorhandling

Pending

Pending continuous-integration/drone/pr Build is pending Details

User tests: Successful: Unsuccessful:

Hackwar
28 Feb 2023

Pull Request for Issue #39717.

Summary of Changes

The indexer in Smart Search currently has situations where errors are returned and instead of gracefully reporting an error, the whole indexing just silently stops and does not proceed. This PR tries to catch all output and notices from finder plugins and return that as properly formatted error. I also removed some unused code.

Testing Instructions

Go to a finder plugin, for example /plugins/finder/content/src/Extension/Content.php and trigger any output in the index() method. For example you can add var_dump($item->title).
In the Smart Search component in the backend click on "Index".

Actual result BEFORE applying this Pull Request

The indexing starts, but shortly after it does not progress any further.

Expected result AFTER applying this Pull Request

The indexing starts and shortly after an error message is displayed. The whole response can be found in the browser console.

Link to documentations

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

8b37e66 28 Feb 2023

Improving error handling of Smart Search Indexing

joomla-cms-bot - change - 28 Feb 2023

Category

⇒

Administration com_finder Language & Strings

Hackwar - open - 28 Feb 2023

Hackwar - change - 28 Feb 2023

Status

New

⇒

Pending

Fedik - test_item - 28 Feb 2023 - Tested successfully

Fedik - comment - 28 Feb 2023

I have tested this item ✅ successfully on 8b37e66

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39965.}

9daad47 28 Feb 2023

Update administrator/language/en-GB/com_finder.ini

Hackwar - change - 28 Feb 2023

Labels

Added: Language Change PR-4.3-dev

wilsonge - comment - 1 Mar 2023

OK this just hides the message and doesn't give us any way to actually fix the problem? Like if there are PHP errors - yes we don't want things to silently fail - but we do want to make sure that they are logged so we can fix them properly in the future rather than just silently swallowing them.

Fedik - comment - 1 Mar 2023

I think George is right.
It probably can be improved in clinet side

joomla-cms/build/media_source/com_finder/js/indexer.es6.js

Lines 162 to 167 in 214119e

    
           onSuccess: (response) => { 
        
             handleResponse(JSON.parse(response)); 
        
           }, 
        
           onError: (xhr) => { 
        
             handleFailure(xhr); 
        
           },

Need kind of

let response;
try {
  response = JSON.parse(data);
} catch (e) {
  Joomla.renderMessages(Joomla.ajaxErrorsMessages(xhr, 'parsererror'));
}
handleResponse(response);

Need to test

Hackwar - comment - 1 Mar 2023

I tried the try-catch and had all of that ready, but it made everything a lot uglier. The thing is that you need to have access to the output and that is now properly in the browser console.

I've used this approach because we have most of the necessary code already in there. We have ob_start() everywhere, but we are missing actually suppressing the output.

Fedik - comment - 1 Mar 2023

I will prepare another pr, need some time

Fedik - comment - 1 Mar 2023

There it is #39973, please review ?

chmst - comment - 4 Mar 2023

@Hackwar as #39973 is RTC, could you close this one?

Hackwar - comment - 4 Mar 2023

They are both valid from my perspective.