Skip to content

feat(indexing): process ES client indexing errors (WIP)#267

Draft
superbiche wants to merge 1 commit into
Jeroen-G:masterfrom
superbiche:feat/handle-indexing-errors
Draft

feat(indexing): process ES client indexing errors (WIP)#267
superbiche wants to merge 1 commit into
Jeroen-G:masterfrom
superbiche:feat/handle-indexing-errors

Conversation

@superbiche

Copy link
Copy Markdown

This is a first shot at handling indexing error in ElasticEngine::update($models).

Related to #221 (we are struggling with indexing errors too).

For now, I'm just retrieving ES client errors, then build a Collection of error strings, recursively inserting each "caused_by" until we get to the root cause.

I'm not sure about what the next steps should be, but I have some ideas:

  • Add an option in config/explorer.php to log these errors, eg:

      "log_indexing_errors" => true,
      "log_indexing_errors_logger" => "elasticsearch-explorer-errors",
    
  • Throw a ModelBulkUpdateFailed Exception, which could format properly the error message, along with the Model class and Index name.
    Throwing this should also be toggled in a configuration option, because the whole MakeSearchable will fail if we throw, even if some models were actually indexed. I'm not a fan of this option but at least it's very strict.

  • Fire an event (ModelBulkUpdateFailedEvent ?) that can be handled in userland, so we leave the actual behavior to the developer (to log, throw, use Sentry or something similar, send an alert...)

I like the logging approach, as it's really easy to implement, but I also think the event approach can enable more specific workflows. So I'd suggest we go with both.

I'm open for any suggestion regarding how the message is formatted, and which approach could be acceptable (if we go with the event approach, we should supplement the error messages with event properties to get the raw data.

What do you think?

Cheers,
Michel

@Jeroen-G

Jeroen-G commented Apr 3, 2025

Copy link
Copy Markdown
Owner

I like the logging option the most, but I don't think we need an extra config option for it. You may just log it as soon as logging is enabled!
Thank you for helping to make Explorer better!

@superbiche

Copy link
Copy Markdown
Author

@Jeroen-G you're right, we can use the existing logging option.
I really have a use case for an event though, would it be okay if I add this option too, disabled by default?

Do you have any thought about the log format, or should we keep it simple for now, and enhance it when people come with actual use cases for enhancements?

I'll work on this later today or this weekend.

Thanks !

@Jeroen-G

Jeroen-G commented Apr 3, 2025

Copy link
Copy Markdown
Owner

It should work with the logger that you can configure, which is a PSR3 LoggerInterface.
This also means that you can then write your own logger that implements it and dispatches an event for errors.

@ert485

ert485 commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Any update on this @superbiche?

The lack of error visibility caused me some grief, and I'd like to have confidence that all of my elasticsearch indices are fully in sync with my database.

UPDATE - I also made a PR - #273

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants