Skip to content

Conversation

@SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented May 13, 2025

Resolves

Currently when a miss behaving event listener errors with out proper error catching this causes the emitting process to error also.

The issue here is that at this causes data loss and loss of functionality in unrelated apps. For instance, when saving a Calendar Appointment we emit a create calendar object event for every attendee that the event is saved to.

E.g.
Attendee 1 (Save Object) -> Emit Event ->Attendee 2 (Save Object) -> Emit Event -> Organizer (Save Object) -> Emit Event

So when a event listener errors after the first emit the entire process crashes and we lose data, even though the emitting process experienced no errors. As event listeners can be implemented by any app, there is no way to make sure that the code being executes is safe.

Some example of recent issues that causes user confusion and data loss..

Constructor errors

TypeError: OCA\Talk\Listener\CalDavEventListener::__construct(): Argument #6 ($userId) must be of type string, null given
#26 /spreed/lib/Listener/CalDavEventListener.php(32): OCA\Talk\Listener\CalDavEventListener::__construct

Runtime errors

ErrorException: Warning: Attempt to read property "LOCATION" on null
#17 /spreed/lib/Listener/CalDavEventListener.php(68): OCA\Talk\Listener\CalDavEventListener::handle

Summary

  • extends the symfony event dispatcher to catch errors in event listeners, to prevent the emitting process crashing in the event that a event listener errors without handling the exception

Checklist

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PSR includes error handling and unfortunately catching listener errors would "break" the standard: https://www.php-fig.org/psr/psr-14/#error-handling

@SebastianKrupinski
Copy link
Contributor Author

Yeah, I figured this might not fly.

@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 💌 📅 👥 Groupware team May 14, 2025
@SebastianKrupinski SebastianKrupinski deleted the fix/noid-catch-listener-erros-instead-of-failing branch May 14, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress

Projects

Development

Successfully merging this pull request may close these issues.

3 participants