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

BUG: "Invalid time value" when pointing to a Nextcloud-powered ICS #61

Open
gabek opened this issue Sep 1, 2023 · 13 comments
Open

BUG: "Invalid time value" when pointing to a Nextcloud-powered ICS #61

gabek opened this issue Sep 1, 2023 · 13 comments
Labels
bug Something isn't working

Comments

@gabek
Copy link

gabek commented Sep 1, 2023

Hi there. Thanks for the fantastic plugin! This is super useful.

I'm running into a scenario where I've added an additional ICS URL, that is generated by a Nextcloud server. However it throws "RngeError: Invalid time value" and then dies. It also then fails to handle the other calendar that I have configured, that the plugin is happy with.

Have you seen success with Nextcloud ICS URLs in the past? Thanks again!

image

@muness muness changed the title "Invalid time value" when pointing to a Nextcloud-powered ICS BUG: "Invalid time value" when pointing to a Nextcloud-powered ICS Sep 1, 2023
@muness
Copy link
Member

muness commented Sep 1, 2023

Thanks for the report, @gabek . I rely on node-ical to parse the ICS payload so can't say much about why it's failing. I'll try to update to the latest release to see if it's something they've fixed recently.

It also then fails to handle the other calendar that I have configured

I will see what error handling I can improve to fix that - if node-ical fails to parse a calendar, I think I should use a toast to display an error but keep parsing other calendars. That sound reasonable to you?

@muness
Copy link
Member

muness commented Sep 1, 2023

Hmm, looks like I am already on the latest version, 0.16.1

But looking at that call stack again, it does look like it's something in the plugin that's throwing the exception. The only place I call date.toISOString() is at const dateLookupKey = date.toISOString().slice(0, 10); which tries to deal with recurring events, the most complicated part of the code. I'll look to see if there's another way to strip the time.

Any chance you can localize this down to one event and can share an ICS file with just that? i.e. use Nextcloud to create a new calendar with just one recurring event and see if it has the problem?

@gabek
Copy link
Author

gabek commented Sep 1, 2023

My ICS feed is rather large, so it could be anything. I'm not sure if it's one single event that's a problem or if every entry is. But should be easy to answer. I'm going to create a new calendar and add a single event in it and seee what we get.

@gabek
Copy link
Author

gabek commented Sep 1, 2023

Good and bad news. I was able to create a new calendar with a single event, an event that is listed as "all day" and an event that reoccurs every day and there was no problem reading this.

Do you know what field in ICS would be worth looking at? I could just export all of the datetimes for that field and see what one might look different/invalid.

@gabek
Copy link
Author

gabek commented Sep 4, 2023

I think I should use a toast to display an error but keep parsing other calendars. That sound reasonable to you?

That makes sense. Even better yet, if only a single event in a calendar is unparseable, just skip that single event and maybe continue going through the calendar. They might all fail, if there's some incompatibility, but at least they'd be attempted.

@muness muness added the bug Something isn't working label Oct 24, 2023
muness added a commit that referenced this issue Oct 26, 2023
Previously, the code did not handle errors properly when retrieving and parsing calendar data. This update introduces error handling for downloading and filtering events from the calendars.

- Added error handling for downloading calendars and parsing errors.
- Store error messages encountered during processing.
- Notify the user about any errors encountered during processing.

This change ensures that any errors encountered during the retrieval and processing of calendars are properly handled and communicated to the user.

Should help with BUG: "Invalid time value" when pointing to a Nextcloud-powered ICS #61
@muness
Copy link
Member

muness commented Oct 26, 2023

Added error handling in #73 so that other calendars can at least keep getting parsed.

I am hoping that #72 , when I have it finished will address the actual issue of date parsing issue per event.

@muness
Copy link
Member

muness commented Oct 30, 2023

@gabek - I just merged #72 in 1.3.0 . Would you see if this fixes the Nextcloud-powered ICS date parse issues?

@gabek
Copy link
Author

gabek commented Oct 30, 2023

Done! Different error now it seems.

image

@muness
Copy link
Member

muness commented Oct 30, 2023

Thanks for checking. Let me see how I can add a debug build that way you can click and see the source / specific lines of code causing the issue(s).

@muness
Copy link
Member

muness commented Nov 4, 2023

@gabek - if you look at the latest release, https://github.com/muness/obsidian-ics/releases/tag/1.3.2 , it now builds a debug version of the plugin (main-debug.js) alongside the normal production one.

If you'd download that main-debug.js from the release and replace the <vault>/.obsidian/plugins/ics/main.js file with it, you'll then be able to click on the error and get the specific place in the stack where the error happened. Can you give that a try and share what you see?

If you can share a public nextCloud ICS with the error I can debug myself, but this is the next best thing barring that.

@gabek
Copy link
Author

gabek commented Nov 7, 2023

Here you go, this looks hopefully a little more useful! While the calendar I'm using this with is my personal internal calendar, I can probably create an additional new one that's public and has some test events in it.

image

image

@muness
Copy link
Member

muness commented Nov 8, 2023

Thanks @gabek , I expected to see the non-minified code, so I'll have to check if my debug build is working correctly.
image

const eventTimeZone = import_moment_timezone.tz.zone(event.rrule.origOptions.tzid);
offset = localTimeZone.utcOffset(date) - eventTimeZone.utcOffset(date);

I think the exception is that import_moment_timezone.tz.zone(event.rrule.origOptions.tzid); is coming back as undefined. I'd like to check the event.rrule.origOptions.tzid value so I can dig in. Can you open that up in the Scope window on the right?

Alternatively an ICS would be awesome as it'd make it a lot easier for me to debug this. You can even create it, and then upload it here as an attachment.

@muness
Copy link
Member

muness commented Nov 8, 2023

@gabek while working on #80 , I added debugging messages for recurring events that get cloned when a rrule is matched. Once you update to 1.3.3, you should be able to see those in the console if you enable verbose logging as in the screenshot
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants