DTSTART with TZID: between produces incorrect time #355

agordeev · 2019-07-16T08:58:27Z

Reporting an issue

const ruleStr = [
'DTSTART;TZID=America/Los_Angeles:20190603T181500', 
'RRULE:FREQ=WEEKLY;WKST=SU;BYDAY=MO,TU,WE,FR,SA'
].join('\n')
const rule = RRule.fromString(ruleStr)
const result = rule.between(
new Date('2019-07-16T07:00:00.000-07:00'), 
new Date('2019-07-23T07:00:00.000-07:00')
)
console.log(result)

outputs this:

0:Tue Jul 16 2019 15:15:00 GMT-0500 (CDT) {}
1:Wed Jul 17 2019 15:15:00 GMT-0500 (CDT) {}
2:Fri Jul 19 2019 15:15:00 GMT-0500 (CDT) {}
3:Sat Jul 20 2019 15:15:00 GMT-0500 (CDT) {}
4:Mon Jul 22 2019 15:15:00 GMT-0500 (CDT) {}

15:15:00 GMT-0500 (CDT) is 17:15 PDT, while DTSTART is DTSTART;TZID=America/Los_Angeles:20190603T181500

When I set tzid implemented by @davidgoli I get a different result:

const ruleStr = [
'DTSTART:20190603T181500', 
'RRULE:FREQ=WEEKLY;WKST=SU;BYDAY=MO,TU,WE,FR,SA'
].join('\n')
const rule = RRule.fromString(ruleStr)
const ruleSet = new RRuleSet()
ruleSet.rrule(rule)
ruleSet.tzid('America/Los_Angeles')
const result = ruleSet.between(
new Date('2019-07-16T07:00:00.000-07:00'), 
new Date('2019-07-23T07:00:00.000-07:00')
)
console.log(result)

0:Tue Jul 16 2019 13:15:00 GMT-0500 (CDT) {}
1:Wed Jul 17 2019 13:15:00 GMT-0500 (CDT) {}
2:Fri Jul 19 2019 13:15:00 GMT-0500 (CDT) {}
3:Sat Jul 20 2019 13:15:00 GMT-0500 (CDT) {}
4:Mon Jul 22 2019 13:15:00 GMT-0500 (CDT) {}

The expected result is:

0:Tue Jul 16 2019 20:15:00 GMT-0500 (CDT) {}
1:Wed Jul 17 2019 20:15:00 GMT-0500 (CDT) {}
2:Fri Jul 19 2019 20:15:00 GMT-0500 (CDT) {}
3:Sat Jul 20 2019 20:15:00 GMT-0500 (CDT) {}
4:Mon Jul 22 2019 20:15:00 GMT-0500 (CDT) {}

What's the reason of this?
rrule version: 2.6.2
I run node under CDT time zone. My system time zone is America/Los_Angeles

The text was updated successfully, but these errors were encountered:

agordeev · 2019-07-17T12:13:43Z

I clonned this repo and added this test:

  it('generates correct recurrences when recurrence is WEEKLY and has BYDAY specified', () => {
    const rrule = new RRule({
      freq: RRule.WEEKLY,
      dtstart: new Date(Date.UTC(2019, 6, 17, 18, 0, 0)),
      tzid: 'America/Los_Angeles',
      count: 10,
      interval: 1,
      wkst: RRule.SU,
      byweekday: [RRule.MO, RRule.TU, RRule.WE, RRule.FR, RRule.SA]
    })

    expect(rrule.all()).to.deep.equal([
      new Date('2019-07-17T18:00:00.000-07:00'), // WE
      new Date('2019-07-19T18:00:00.000-07:00'), // FR
      new Date('2019-07-20T18:00:00.000-07:00'), // SA
      new Date('2019-07-22T18:00:00.000-07:00'), // MO
      new Date('2019-07-23T18:00:00.000-07:00'), // TU
      new Date('2019-07-24T18:00:00.000-07:00'),
      new Date('2019-07-26T18:00:00.000-07:00'),
      new Date('2019-07-27T18:00:00.000-07:00'),
      new Date('2019-07-29T18:00:00.000-07:00'),
      new Date('2019-07-30T18:00:00.000-07:00')
    ])
  })

It succeeds only when I run it under UTC time zone (when I set "env": { "TZ": "UTC" } in launch.json). The test fails in any other time zone.
@davidgoli is this intended and I'm missing something here? If yes, how to properly use tzid parameter? I think it was added to tell rrule to run in a time zone specified.

ScarlettZebra · 2019-07-18T15:07:34Z

I have this same issue but with rrule.all(). UTC works, while any other time zone applies the offset again when it shouldn't.

ScarlettZebra · 2019-07-18T15:23:40Z

I think it might have something to do with line 1849 of rrule-tz.js.

var rezonedDate = rezoneIfNeeded(res, options);

rezoneIfNeeded, contrary to its name, does not perform any checks and always rezones unless it's UTC.

function rezoneIfNeeded(date, options) { return new datewithzone_DateWithZone(date, options.tzid).rezonedDate(); }

Looks like it's rezoning the date even though it's already correctly zoned.

ScarlettZebra · 2019-07-18T15:34:41Z

Changing rezonedDate() to simply return this.date seems to fix things for me. I can't be completely sure I've broken something somewhere else, but from a cursory glance, it looks like everything else works fine when this change is made.

davidgoli · 2019-07-18T19:01:25Z

I think it might have something to do with line 1849 of rrule-tz.js.

var rezonedDate = rezoneIfNeeded(res, options);

rezoneIfNeeded, contrary to its name, does not perform any checks and always rezones unless it's UTC.

function rezoneIfNeeded(date, options) { return new datewithzone_DateWithZone(date, options.tzid).rezonedDate(); }

Looks like it's rezoning the date even though it's already correctly zoned.

This is not correct. The rezonedDate method checks if tzid is set to determine whether to apply a zone offset.

davidgoli · 2019-07-18T19:03:01Z

@agordeev I will look into this issue. @hlee5zebra 's "fix" of simply removing timezone support was not necessary. Do you get the expected results if you simply don't use the tzid param in either case?

Regardless, the inconsistency between the RRule and RRuleSet behavior is concerning to me and probably a bug that needs a deeper look. Thanks for the test case!

ScarlettZebra · 2019-07-18T20:01:32Z

I see the issue that led to my problem -- my server code was not handling the DTSTART value time correctly, passing in UTC when it should be localized to the time zone specified by the TZID. Perhaps that may help you with your issue @agordeev . Thanks @davidgoli for pointing me in the right direction.

ScarlettZebra · 2019-07-18T20:23:28Z

Looks like I may have stumbled upon an issue in the codebase. The toString() method in datewithzone.ts will output a UTC time when a TZID is supplied, when it should instead output the localized time.

davidgoli · 2019-07-18T20:25:23Z

@hlee5zebra make sure to note this text in the README:

https://github.com/jakubroztocil/rrule#important-use-utc-dates

Important: Use UTC dates

Dates in JavaScript are tricky. RRule tries to support as much flexibility as possible without adding any large required 3rd party dependencies, but that means we also have some special rules.

By default, RRule deals in "floating" times or UTC timezones. If you want results in a specific timezone, RRule also provides timezone support. Either way, JavaScript's built-in "timezone" offset tends to just get in the way, so this library simply doesn't use it at all. All times are returned with zero offset, as though it didn't exist in JavaScript.

The bottom line is the returned "UTC" dates are always meant to be interpreted as dates in your local timezone. This may mean you have to do additional conversion to get the "correct" local time with offset applied.

For this reason, it is highly recommended to use timestamps in UTC eg. new Date(Date.UTC(...)). Returned dates will likewise be in UTC (except on Chrome, which always returns dates with a timezone offset).

davidgoli · 2019-07-18T20:29:39Z

The additional wrinkle is in most JS implementations, you either get UTC offset or local offset, but you can't switch between them. This also varies depending on the implementation. So a "UTC date" may be returned giving a toString() that includes the offset in some places, and not in others.

For example, in Chrome:

> new Date(Date.UTC(2016, 10, 5))
Fri Nov 04 2016 17:00:00 GMT-0700 (Pacific Daylight Time)

but in node:

> new Date(Date.UTC(2016, 10, 5))
2016-11-05T00:00:00.000Z

This is why for the best result, ignore the toString() value and exclusively use the toISOString() and getUTCHours() (etc) methods.

davidgoli · 2019-07-18T20:33:10Z

Note that this approach - using only UTC methods for all dates, and then "interpreting" them as being in local time - allows a uniform way of accessing dates & times returned by rrule without needing to consider the timezone of the rrule.

ScarlettZebra · 2019-07-18T22:14:38Z

Okay, I've been tinkering around with rrule.all() for a bit to try to make this issue reproducible, and here's what I've found to reproduce this issue reliably:

I've tried setting DTSTART as midnight within the respective time zones of 'America/Adak', 'America/Chicago' (my local time), 'America/New_York', and 'UTC', and the resulting rrule.toString() is printed accurately:

>> rRule.toString()
"DTSTART;TZID=America/Adak:20190718T000000
RRULE:FREQ=DAILY"

>> rRule.toString()
"DTSTART;TZID=America/Chicago:20190718T000000
RRULE:FREQ=DAILY"

>> rRule.toString()
"DTSTART;TZID=America/New_York:20190718T000000
RRULE:FREQ=DAILY"

>> rRule.toString()
"DTSTART:20190718T000000Z
RRULE:FREQ=DAILY"

While walking through the iterator function passed into rrule.all(), the first instance of the first parameter date turns out to be the following for each time zone when each Date is printed via .toISOString():

America/Adak:
"2019-07-18T04:00:00.000Z"

America/Chicago:
"2019-07-18T00:00:00.000Z"

America/New_York:
"2019-07-17T23:00:00.000Z"

UTC:
"2019-07-18T00:00:00.000Z"

It looks like when the time zone is not set to UTC (e.g. America/Adak, America/New_York), then the offset between your local time and the selected time zone is subtracted from the DTSTART date. So the New York ISO string is showing 23:00 since the offset between my local time and New York is +1, which when subtracted from midnight on 07/18/2019, results in what we see, which is 11pm on 07/17/2019.

Note that this doesn't happen for UTC though, which is curious.

Do you think this might be the issue, or is there some configuration that needs to be done that I might have missed?

ScarlettZebra · 2019-07-18T22:26:43Z

As a workaround to what I've been seeing, I've done the following, and this seems to reliably get me an accurate instance of JavaScript's Date:

rRule.all(function (date, i) {
    if (this.getSelectedTzid() !== 'UTC') {
        date = moment.tz({
            year: date.getUTCFullYear(),
            month: date.getUTCMonth(),
            date: date.getUTCDate(),
            hours: date.getUTCHours(),
            minutes: date.getUTCMinutes()
        }, this.getLocalTzid()).toDate();
    }

    ...
}.bind(this));

davidgoli · 2019-07-19T00:30:39Z

@hlee5zebra Yes, your approach of using the getUTCxxx methods is the correct one recommended by the readme.

Keep in mind that a pseudo-UTC date is almost never really in UTC time. UTC is just overloaded to be the "neutral" timezone, so the getUTCxxx methods can be used to retrieve the local time regardless of the original zone. For this reason, you should expect to see the same behavior without using tzid as you would by using your local timezone, in your local timezone. tzid should only be used to get the current local time of a recurrence in a different timezone. If you always want the recurrences to be in the user's local timezone, you should not use tzid.

This is why my rewrite of this library will not use the builtin JS Date object at all. It's simply too confusing.

agordeev · 2019-07-19T06:14:25Z

@agordeev I will look into this issue. @hlee5zebra 's "fix" of simply removing timezone support was not necessary. Do you get the expected results if you simply don't use the tzid param in either case?

Thanks for your reply David.

    const rrule = new RRule({
      freq: RRule.WEEKLY,
      dtstart: new Date(Date.UTC(2019, 6, 17, 18, 0, 0)),
      // tzid: 'America/Los_Angeles',
      count: 10,
      interval: 1,
      wkst: RRule.SU,
      byweekday: [RRule.MO, RRule.TU, RRule.WE, RRule.FR, RRule.SA]
    })

produces:

  -  [Date: 2019-07-17T18:00:00.000Z]
  -  [Date: 2019-07-19T18:00:00.000Z]
  -  [Date: 2019-07-20T18:00:00.000Z]
  -  [Date: 2019-07-22T18:00:00.000Z]
  etc..

So the dates/times are correct, but time zone is UTC. rrule considers DTSTART in UTC when I omit tzid param.

If you always want the recurrences to be in the user's local timezone, you should not use tzid.

I thought the only option to get recurrences in the user's time zone is to pass that time zone to tzid param? Keeping in mind the library is used with node.js on the server.

davidgoli · 2019-07-19T16:41:24Z

If the recurrence will be at 1800 in the user's local time regardless of the timezone, then you do not need to use tzid. Please make sure you've read about "floating" times as described in the README: https://github.com/jakubroztocil/rrule#important-use-utc-dates

braveheart-star · 2022-04-25T16:31:02Z

this issue is still opened?

ScarlettZebra mentioned this issue Jul 18, 2019

Do not rezone dates to prevent doubly applying offset. Accommodate relevant tests. #357

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DTSTART with TZID: between produces incorrect time #355

DTSTART with TZID: between produces incorrect time #355

agordeev commented Jul 16, 2019

agordeev commented Jul 17, 2019 •

edited

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 18, 2019

davidgoli commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 18, 2019

davidgoli commented Jul 18, 2019

davidgoli commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 19, 2019

agordeev commented Jul 19, 2019

davidgoli commented Jul 19, 2019

braveheart-star commented Apr 25, 2022

DTSTART with TZID: between produces incorrect time #355

DTSTART with TZID: between produces incorrect time #355

Comments

agordeev commented Jul 16, 2019

Reporting an issue

agordeev commented Jul 17, 2019 • edited

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 18, 2019

davidgoli commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 18, 2019

Important: Use UTC dates

davidgoli commented Jul 18, 2019

davidgoli commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

ScarlettZebra commented Jul 18, 2019

davidgoli commented Jul 19, 2019

agordeev commented Jul 19, 2019

davidgoli commented Jul 19, 2019

braveheart-star commented Apr 25, 2022

agordeev commented Jul 17, 2019 •

edited