When indexing somme kind of exception as php exception is not catched by the module Search API.
This patch provide a way to catch all kind of exception as a Throwable instead of Exception.

CommentFileSizeAuthor
#6 3252430-6--render_item_processor_catch_all_throwables.patch2.19 KBdrunken monkey
catch_exceptions_during_rendering_for_processor.patch1.24 KBmiarynah

Comments

miarynah created an issue. See original summary.

andregp’s picture

andregp commented
Status: Active » Needs review
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Version: 8.x-1.21 » 8.x-1.x-dev
Status: Needs review » Postponed (maintainer needs more info)

Normally, \Throwable are more serious errors which I’d be hesitant to catch. Can you provide a specific example of the kind of error you’d want to catch?

jrockowitz’s picture

jrockowitz commented

The errors that I need to catch are usually related to a node not being renderable on a very large site, and Search API seems to be the only system that is catching the error.

Also, when a fatal error occurs during indexing, it immediately becomes a critical issue because no content is being indexed vs. one node not being indexed, which is a minor issue.

miarynah’s picture

miarynah commented

@drunken monkey
in my case, some node on more than 2k nodes has error while redering content as rendered_item in index field and indexing is stoped immediatly. The error was an url rendering in twig and not be catched by the default exception. This patch catch the error and keep indexing.
Note that these node was test content and we can't identify wich of 2k nodes it is

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new 3252430-6--render_item_processor_catch_all_throwables.patch2.19 KB

OK, I guess this does make sense. We want as little chance as possible of something just terminating the whole indexing process, and as we can’t guarantee that other code doesn’t contain fatal bugs, we should just catch everything we can.

Little problem is that we’d also need to adapt the signature of LoggerTrait::logException() to still be able to use it with \Error instances as well. However, for a trait, I don’t think such a change is a BC problem at all, since overriding trait methods works differently than for classes/interfaces. However, I still created a change record for it.

Please test/review!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Component: General code » Plugins

Status: Needs review » Needs work

The last submitted patch, 6: 3252430-6--render_item_processor_catch_all_throwables.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review

Please test/review so I can commit.

  • drunken monkey committed a78e805c on 8.x-1.x 
    Issue #3252430 by drunken monkey: Fixed error in render code breaking...
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Well, I’m reasonably certain that this won’t break anything major, so just committed this now.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.