Improve error handling and reliability on indexing

Thanks for opening this issue!
I agree, this does sound sensible in your scenario. However, in practice, there are several problems:

• Normally/often, multiple items are indexed at once. Depending on where the exception is encountered, we have no way of knowing which item(s) is/are actually the cause (and should be skipped in the future).
• Indexing can fail for any number of reasons, some of which are just temporary (e.g., Solr server can't be reached), and since we can never know the complete set of possibilities (a LogicException from the Responsive Image module?!?), we have no way of knowing whether a specific error will also occur the next time we attempt to index that item. (No matter where the exception is caught, really.) This would probably require a considerably more complex system to work with most cases, and not worsen matters in more scenarios than it helps.

In general, we just try to do the best error handling we can. E.g., when rendering an item doesn't work, we should probably just log a warning and not include the "rendered_item" field's value in the indexed item. No big deal, and far better than blocking the whole indexing process, of course. However, the rendering method doesn't actually document that it can throw any exceptions – which, unfortunately, is inconsistent in large parts of Core. So, we can't even know which methods (as long as they are outside of the Search API) could even throw exceptions, or which one. This, of course, makes it almost impossible to handle all such cases properly. I'd say that either the rendering code or the Responsive Image module are at fault here – probably the latter one, I'd guess.

However, to solve your specific problem in our module (and not wait for Core to do something about it), we could probably just catch all exceptions that are thrown from rendering in \Drupal\search_api\Plugin\search_api\processor\RenderedItem::addFieldValues() and just log an error message and otherwise ignore it. And yes, I guess including the item ID would make sense in such cases, thanks!

Do you really mean "at once" or in a batch one item after another.

Generally, items in an indexing batch are all handed to the various sub-steps (field value extraction, preprocessing, actual indexing on server) as one single array. So, if we want to identify an individual item that makes problems, it would have to be the job of each individual sub-system (processor plugin, backend plugin, etc.) to do that. A centralized solution in just the "framework" part of the module wouldn't be possible (except for field extraction).

Isn't it always an entity which gets indexed?

No, you can index any kind of data as long as you write a datasource plugin for it.

What if SAPI …

Yes, something like that is what I mean with “a considerably more complex system”. Something like this might make sense, but it's a major framework addition which requires both good planning and quite some work in implementation. So, it'll have to wait until I have the free time available for it.

Could that be implemented by the use of one (or two) index-plugins? One plugin with highest priority (add entities to list an skip blacklisted entities) and the other one with lowest priority (unlist indexed entities).

Only for the "preprocessing by enabled processor" step. You couldn't spot problems during field extraction (where your actual problem was) or server indexing this way.

Anyways, the attached patch would implement the improved error handling for the "Rendered item" processor. Please test/review!

For the other logged warning, which you quote in your IS, it doesn't really make sense to include the item IDs, since the problem is not with the items, but with the field configuration, which doesn't have values set for all datasources or bundles. But maybe we could rephrase the message to make that clearer? Suggestions?

After committing this, I'd say we just leave the issue open to tackle the larger problem – when I have a bit of time to spare for it.

This worked for me.
It was really great to be able to see the node that was having an issue
Does it work for entities as well?

Thanks a lot for reporting, good to hear it works for you!
Committed.
Yes, it will also work for all other kinds of entities, not just nodes.

The problem still occurs in case there is an error coming from the rendering of the node which does not originate an exception. (PHP Error for instance).

Small fix to address those cases attached.

File with correct name format.

Patch reroll.

Reopening the issue, this seems like good hardening of our error handling.

+++ b/src/Plugin/search_api/processor/RenderedItem.php
@@ -296,6 +296,11 @@
+        // In case it is not an exception (it is an error), just transforms

typo in transform

Fixing the patch.

I think this is a good idea, this should make the rest of our error handling more robust.

Are you sure this is still needed, and not just a duplicate of the (already fixed) #3252430: Catch exception on indexing using Search API and keep indexation continuing?
Seems this just needlessly casts the Throwable to an exception?