python-caldav / caldav

Apache License 2.0
314 stars 94 forks source link

Error with calendar.date_search() and Google Calendar #310

Closed flozz closed 1 year ago

flozz commented 1 year ago

I am using this library to fetch events of a meeting room calendar from Google Calendar.

When I try to search events by date I have the following error:

ERROR:caldav:Deviation from expectations found.  Please raise an issue at https://github.com/python-caldav/caldav/issues or reach out to t-caldav@tobixen.no, include this error and the traceback and tell what server you are using
Traceback (most recent call last):
  File "[...]/__env__/lib/python3.11/site-packages/caldav/lib/error.py", line 28, in assert_
    assert condition
AssertionError
Traceback (most recent call last):
  File "[...]/./gcal.py", line 78, in <module>
    events = calendar.date_search(start=start_date, end=end_date)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 907, in date_search
    objects = self.search(
              ^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 1069, in search
    if any(key in component for key in recurrance_properties):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 1069, in <genexpr>
    if any(key in component for key in recurrance_properties):
           ^^^^^^^^^^^^^^^^
TypeError: argument of type 'NoneType' is not iterable

You will find a minimal example to reproduce this issue in the following article: https://blog.lasall.dev/post/tell-me-why-google-and-caldav/

tobixen commented 1 year ago

Thanks for the report.

I don't know when I will have time to look properly into it, but if you could dig a bit and eventually write a pull request, it would be much appreciated. Here is the code section that fails (assuming you're on v.1.2.1):

            for o in objects:
                ## This should not be needed
                o.load(only_if_unloaded=True)
                component = o.icalendar_component
                recurrance_properties = ["exdate", "exrule", "rdate", "rrule"]
                if any(key in component for key in recurrance_properties):
                    o.expand_rrule(start, end)

Apparently it breaks because component is None, while it's expected to be a icalendar object from the icalendar library. The trivial work-around is to throw in something like this:

                if component is None:
                    continue

However, the interesting thing here is ... why is it None in the first place? Could you debug a bit and send me the output of o.data? (if it's non-public, you can mask out private parts of it. If it's neither confidential nor public, you may send it by email)

flozz commented 1 year ago

I already tried the if not component: continue to workaround the issue while I am working on my project, but it cause issues later (missing vevents (KeyError: 'vevent')). So it is a really temporary "fix" ^^


I modified to code to log o.data:

            for o in objects:
                print("-"*80)  # XXX
                print(o.data)  # XXX

and I got the following result (CALNAME anonymized):

[...] previous events are ok so I do not paste them here...
--------------------------------------------------------------------------------
BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
X-WR-CALNAME:XXXXXXXXXXXXXXXX (10)
X-WR-TIMEZONE:Europe/Paris
BEGIN:VTIMEZONE
TZID:Europe/Paris
X-LIC-LOCATION:Europe/Paris
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
TZNAME:CEST
DTSTART:19700329T020000
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
TZNAME:CET
DTSTART:19701025T030000
RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU
END:STANDARD
END:VTIMEZONE
END:VCALENDAR

ERROR:caldav:Deviation from expectations found.  Please raise an issue at https://github.com/python-caldav/caldav/issues or reach out to t-caldav@tobixen.no, include this error and the traceback and tell what server you are using
Traceback (most recent call last):
  File "[...]/__env__/lib/python3.11/site-packages/caldav/lib/error.py", line 28, in assert_
    assert condition
AssertionError
Traceback (most recent call last):
  File "[...]/./gcal.py", line 77, in <module>
    events = calendar.date_search(start=start_date, end=end_date)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 907, in date_search
    objects = self.search(
              ^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 1071, in search
    if any(key in component for key in recurrance_properties):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/__env__/lib/python3.11/site-packages/caldav/objects.py", line 1071, in <genexpr>
    if any(key in component for key in recurrance_properties):
           ^^^^^^^^^^^^^^^^
TypeError: argument of type 'NoneType' is not iterable

the event looks empty compared to the previous one that take 286 lines...

tobixen commented 1 year ago

Yes, it's empty, contains only timezone information. Probably such events should be filtered out. Perhaps something like this (if I had more time I would have embedded the empty icalendar data in a unit test and fixed this myself):

            for o in objects:
                ## This would not be needed if the servers would follow the standard ...
                o.load(only_if_unloaded=True)

            ## Google sometimes returns empty objects
            objects = [o for o in objects if o.icalendar_component]

            for o in objects:
                component = o.icalendar_component
                recurrance_properties = ["exdate", "exrule", "rdate", "rrule"]
                if any(key in component for key in recurrance_properties):
                    o.expand_rrule(start, end)

cause issues later (missing vevents (KeyError: 'vevent')).

From my code? I should rewrite it to use the icalendar library rather than vobjects.

flozz commented 1 year ago

From my code? I should rewrite it to use the icalendar library rather than vobjects.

Nope, this one was in my code... But I handled it properly now :)


An other strange behavior is that sometime when I search by date it returns me old instances of recurrent events that are ended in the calendar:

Example (Event B & D)
--------------------------------------------------------------------------------
 => Event A 
    Tue May 30 14:00:00 2023
--------------------------------------------------------------------------------
 => Event B
    Mon Jan  2 09:30:00 2023
--------------------------------------------------------------------------------
 => Event C
    Tue May 30 14:30:00 2023
--------------------------------------------------------------------------------
 => Event D
    Tue May 30 11:00:00 2023
--------------------------------------------------------------------------------
 => Event E
    Tue May 30 09:30:00 2023
--------------------------------------------------------------------------------
 => Event F
    Tue May 30 17:30:00 2023
--------------------------------------------------------------------------------
 => Event G
    Tue May 30 09:30:00 2023
--------------------------------------------------------------------------------
 => Event D
    Tue Jan 24 11:00:00 2023
--------------------------------------------------------------------------------
 => Event B
    Tue Jan  3 09:30:00 2023
--------------------------------------------------------------------------------
 => Event B
    Fri Dec 30 09:30:00 2022
--------------------------------------------------------------------------------
 => Event B
    Tue Apr  4 14:00:00 2023
--------------------------------------------------------------------------------
 => Event H
    Tue May 30 16:00:00 2023
--------------------------------------------------------------------------------
 => Event I
    Tue May 30 10:00:00 2023
--------------------------------------------------------------------------------
 => Event B
    Wed Jan 11 09:30:00 2023
--------------------------------------------------------------------------------
 => Event J
    Tue May 30 09:00:00 2023
--------------------------------------------------------------------------------
 => Event B
    Wed Mar  8 09:30:00 2023
--------------------------------------------------------------------------------
 => Event K
    Tue May 30 15:00:00 2023

I managed to filter those events but maybe I should report this as a new issue?

tobixen commented 1 year ago

calendar.date_search is legacy and stays there for backward compatibility, use calendar.search instead. Anyway, that won't fix the issue above.

The same lines ...

                if any(key in component for key in recurrance_properties):
                    o.expand_rrule(start, end)

Does it enter the second line there? If so, the problem is in the caldav library and should be reported as a separate issue - if not, the problem is at google side. If the problem is at Googles side, I do have some plans to fix it in the library side.

flozz commented 1 year ago

calendar.date_search is legacy and stays there for backward compatibility, use calendar.search instead.

Oh, good to know, I will use calendar.search() then :)


I confirm it passes here:

                if any(key in component for key in recurrance_properties):
                    print("X"*80)  # XXX
                    print(o.data)  # XXX
                    o.expand_rrule(start, end)

It seems to be a Google side issue. They send a big VCALENDAR with multiple VEVENT... for the Event B, it was moved to an other meeting room, so it exists today but in an other calendar... I do not know how they messed up with that... ¬_¬'

tobixen commented 1 year ago

It seems like they can't handle the expanding correctly, perhaps that's why it messes up the meeting rooms too. Up until recently, Google didn't even try to do expansion.

The caldav library asks the server to do the expansion, originally because I deemed it too difficult to do it on the client side. Someone eventually helped me to fix client side expansion, but it's only done if the server doesn't do expansion.

The simple workaround for this is to simply not ask the server to do expansion. Find those lines and try to rip them out:

       if expand:
            if not start or not end:
                raise error.ReportError("can't expand without a date range")
            data += cdav.Expand(start, end)

Now that the library supports client-side expansion, client-side expansion should eventually be the default, however, I'm worried that such a change of behaviour may break something for someone.

tobixen commented 1 year ago

The more proper fix would be to add a parameter server_side_expand to the search method, default value True in 1.x and changed to False in 2.0, and then perhaps ...

       if expand:
            if not start or not end:
                raise error.ReportError("can't expand without a date range")
            if server_side_expand:
                data += cdav.Expand(start, end)
flozz commented 1 year ago

Hello,

It took me some time to come back on my project about Google Calendar meeting rooms. I confirm that the proposed workaround is working for me, I opened a PR with it. Feel free to close the PR if you think that a better solution must be found :)

tobixen commented 1 year ago

I'll close this ticket ... and possibly open another one on the client/server side expansion.