Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syncronizing calendars crashes the app #90

Open
MoralCode opened this issue Jun 9, 2021 · 2 comments
Open

Syncronizing calendars crashes the app #90

MoralCode opened this issue Jun 9, 2021 · 2 comments

Comments

@MoralCode
Copy link

MoralCode commented Jun 9, 2021

Steps to reproduce
System: Linux 5.4.0-74-generic # 83-Ubuntu
Version: current main branch

  1. Open Focal
  2. it preforms a sync normally
  3. either click the button in the header bar to resync or wait a while for it to happen automatically

Expected Behavior: Successful sync

Actual Behavior: App crashes with the following console message:

sync: no changes
sync: no changes
sync: no changes
**
ERROR:/path/to/focal/src/reminder.c:73:check_occurrence_add_notification: 'notify_range.start < at && at < notify_range.end' should be TRUE
Bail out! ERROR:/path/to/focal/src/reminder.c:73:check_occurrence_add_notification: 'notify_range.start < at && at < notify_range.end' should be TRUE
@MoralCode
Copy link
Author

MoralCode commented Jun 9, 2021

removing the assert line thats failing per the comment in the code seems to solve it, but im not sure what this line is meant to be protecting against. Is it just meant to skip the code for adding a reminder if the current time is not within the notify_range? this might be better accomplished by returning early from the function rather than using assert and crashing the app.

If this is an appropriate fix, I can make it a pull request if that would help

@ohwgiles
Copy link
Owner

ohwgiles commented Jun 9, 2021

I think before removing it we need to understand why it is failing because it could indicate a different bug. The call to check_occurrence_add_notification is only from check_event_add_notification, which uses event_each_recurrence. event_each_recurrence should only return events within this range. If the assertion is failing then that is not the case. Most likely it is a timezone confusion, i.e. the returned events really are within the range but a wrong timezone conversion pushes them out of the range. Suggest adding debug prints (i.e. of the Event* ev, next, at, notify_range) to figure out exactly what is going on here.

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

No branches or pull requests

2 participants