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

Stepping through days is not correct for Asia/Beirut. #4152

Closed
hifall opened this issue Sep 1, 2017 · 6 comments · Fixed by #4338
Closed

Stepping through days is not correct for Asia/Beirut. #4152

hifall opened this issue Sep 1, 2017 · 6 comments · Fixed by #4338
Labels

Comments

@hifall
Copy link

hifall commented Sep 1, 2017

Description of the Issue and Steps to Reproduce:

I am seeing some strange behavior while using moment to step through each day between a moment range in timezone Asia/Beirut. I have created a minimal test case to demonstrate this behavior:

const moment = require('moment-timezone');

const tzStart = moment("2015-03-27T19:21:24.000Z").tz('Asia/Beirut');
const tzEnd = moment("2015-03-30T19:21:24.000Z").tz('Asia/Beirut');
var startOfDay = tzStart.clone();
var endOfDay = tzStart.clone();
endOfDay.endOf('day');
console.log('startOfDay: ' + startOfDay.format());
console.log('endOfDay: ' + endOfDay.format());
while (endOfDay.isBefore(tzEnd)) {
    console.log('     ');

    // Move to next day.
    startOfDay.add(1, 'days').startOf('day');
    endOfDay.add(1, 'days');
    console.log('startOfDay: ' + startOfDay.format());
    console.log('endOfDay: ' + endOfDay.format());
}

Please include the values of all variables used.

The above script produces:

startOfDay: 2015-03-27T21:21:24+02:00
endOfDay: 2015-03-27T23:59:59+02:00

startOfDay: 2015-03-28T00:00:00+02:00
endOfDay: 2015-03-28T23:59:59+02:00

startOfDay: 2015-03-28T00:00:00+02:00
endOfDay: 2015-03-29T23:59:59+03:00

startOfDay: 2015-03-28T00:00:00+02:00
endOfDay: 2015-03-30T23:59:59+03:00

This doesn't seem correct to me, as startOfDay in last 2 groups (in bold) stay on 2015-03-28, rather than moving to the next day.

If I change the timezone to another one like Asia/Shanghai, it produces:

startOfDay: 2015-03-28T03:21:24+08:00
endOfDay: 2015-03-28T23:59:59+08:00

startOfDay: 2015-03-29T00:00:00+08:00
endOfDay: 2015-03-29T23:59:59+08:00

startOfDay: 2015-03-30T00:00:00+08:00
endOfDay: 2015-03-30T23:59:59+08:00

startOfDay: 2015-03-31T00:00:00+08:00
endOfDay: 2015-03-31T23:59:59+08:00

Which is correct.

Environment:

Node.JS v6.10.3 on Ubuntu 16.0.4

Other information that may be helpful:

  • The time zone setting of the machine the code is running on: UTC
  • The time and date at which the code was run: Fri Sep 1 12:10:09 UTC 2017
  • Other libraries in use: moment-timezone

If you are reporting an issue, please run the following code in the environment you are using and include the output:

console.log( (new Date()).toString())
console.log((new Date()).toLocaleString())
console.log( (new Date()).getTimezoneOffset())
console.log(moment.version)

Fri Sep 01 2017 12:11:55 GMT+0000 (UTC)
9/1/2017, 12:11:55 PM
0
2.16.0
@sirxemic
Copy link

sirxemic commented Sep 1, 2017

https://timezonedb.com/time-zones/Asia/Beirut

2015-03-29 there starts at 1am, not 12am. Basically when you add 1 day to 2015-03-28T00:00:00+02:00 it becomes 2015-03-28T23:00:00+02:00 in moment.js.

You may want to do something like

const moment = require('moment-timezone');

const tzStart = moment("2015-03-27T19:21:24.000Z").tz('Asia/Beirut');
const tzEnd = moment("2015-03-30T19:21:24.000Z").tz('Asia/Beirut');

const iterator = tzStart.clone().hour(12);
const end = tzEnd.clone().hour(12);

console.log('startOfDay: ' + iterator.clone().startOf('day').format());
console.log('endOfDay: ' + iterator.clone().endOf('day').format());

while (iterator <= end) {
    console.log('     ');

    // Move to next day.
    iterator.add(1, 'days')
    console.log('startOfDay: ' + iterator.clone().startOf('day').format());
    console.log('endOfDay: ' + iterator.clone().endOf('day').format());
}

output:

startOfDay: 2015-03-27T00:00:00+02:00
endOfDay: 2015-03-27T23:59:59+02:00

startOfDay: 2015-03-28T00:00:00+02:00
endOfDay: 2015-03-28T22:59:59+02:00

startOfDay: 2015-03-29T01:00:00+03:00
endOfDay: 2015-03-30T00:59:59+03:00

startOfDay: 2015-03-30T00:00:00+03:00
endOfDay: 2015-03-30T23:59:59+03:00

startOfDay: 2015-03-31T00:00:00+03:00
endOfDay: 2015-03-31T23:59:59+03:00

...but as you can see it says 2015-03-30T00:59:59+03:00 is the end of 2015-03-29 ... which doesn't seem to be correct either...

In any case, having DST taking place at 12am is what's causing trouble here.

@hifall
Copy link
Author

hifall commented Sep 1, 2017

@sirxemic, thanks for pointing that out.

Our app depends on stepping through days like this, so this causes our app to break.

@mattjohnsonpint
Copy link
Contributor

Yes, there are multiple open bugs related to startOf and endOf not working correctly with time zones. I believe #3132 explains at least part of this. To simplify the bug, note the following:

moment.tz("2015-03-28T00:00:00+02:00", "Asia/Beirut").add(1, 'day').startOf('day').format()
//=> "2015-03-28T00:00:00+02:00" should be "2015-03-29T01:00:00+03:00"

moment.tz("2015-03-28T01:00:00+03:00", "Asia/Beirut").add(1, 'day').endOf('day').format()
//=> "2015-03-28T22:59:59+02:00" should be "2015-03-29T23:59:59+02:00"

To work around this bug:

moment.tz("2015-03-28T00:00:00+02:00", "Asia/Beirut").hour(12).add(1, 'day').startOf('day').format()
//=> "2015-03-29T01:00:00+03:00"

moment.tz("2015-03-28T01:00:00+03:00", "Asia/Beirut").hour(12).add(2, 'days').startOf('day').subtract(1, 'ms').format()
//=> "2015-03-29T23:59:59+03:00"

In your case, adjust your sample code as follows:

const tzStart = moment("2015-03-27T19:21:24.000Z").tz('Asia/Beirut');
const tzEnd = moment("2015-03-30T19:21:24.000Z").tz('Asia/Beirut');

const startOfDay = tzStart.clone().startOf('day');
const endOfDay = tzStart.clone().hour(12).add(1, 'day').startOf('day').subtract(1, 'ms');

while (endOfDay.isBefore(tzEnd)) {
    console.log('startOfDay: ' + startOfDay.format());
    console.log('endOfDay: ' + endOfDay.format());
    console.log('     ');

    // Move to next day.
    startOfDay.hour(12).add(1, 'days').startOf('day');
    endOfDay.hour(12).add(2, 'days').startOf('day').subtract(1, 'ms');
}

@hifall
Copy link
Author

hifall commented Sep 4, 2017

@mj1856, thanks for your workaround. After the workaround is applied, our internal test that was broken is not reporting the issue.

Although many other of our tests are still broken, which I might file GitHub issues on later.

@hifall
Copy link
Author

hifall commented Sep 4, 2017

I found that there is no test cases covering such scenarios in either moment's repo or moment-timezone's repo. I think at least there should be some tests to cover all supported timezones.

@hifall hifall mentioned this issue Sep 18, 2017
@icambron
Copy link
Member

@mj1856 Is this a bug in Moment or moment-timezone (or, possibly a combination)? I'm wondering if it needs to be moved to other repository?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants