Details
- Reviewers
mollekopf - Group Reviewers
Syncroton Developers
Diff Detail
- Repository
- rS syncroton
- Branch
- dev/ignore-itip-parsing-errors
- Lint
Lint Skipped - Unit
No Test Coverage - Build Status
Buildable 47988 Build 18204: arc lint + arc unit
Event Timeline
What I find a bit odd is that we already try to catch exceptions in Command/Sync.php on all getEntry calls, so that should already take care of it, no?
I suppose it would be better to just make sure that the logic in Sync.php works instead of ignoring ical errors specifically.
Indeed. I just noticed that the actual error wasn't an exception. It was PHP Fatal error: Uncaught Error: __clone method called on non-object in /usr/share/roundcubemail/plugins/libcalendaring/lib/libcalendaring_vcalendar.php:633, so to catch it we'd have to catch Throwable, I suppose, I'm not sure.
Still, it makes sense to not skip the entire message, but sync it as it was a normal message, ignoring the fact it has an invitation.
Throwable catches that fatal error. Now looking at this I found that we could change the catch in https://git.kolab.org/diffusion/RPK/browse/master/plugins/libcalendaring/lib/libcalendaring_vcalendar.php$158
It seems to me we should:
- Adjust the catch in https://git.kolab.org/diffusion/RPK/browse/master/plugins/libcalendaring/lib/libcalendaring_vcalendar.php (to specifically just ignore a broken itip).
- Adjust all the catches in Sync.php to also catch Throwable
- Add a test that triggers this error to the syncroton testsuite
- Remove the catch in this patch again, so we don't end up with 3 layers potentially catching.
..