endOf('day') fails on days that don't start at midnight

[mattjohnsonpint]

Consider:

moment("2016-10-16").endOf('day').format("YYYY-MM-DD HH:mm:ss")

In most time zones, this will return "2016-10-16 23:59:59". However, in Brazil, this will return "2016-10-17 00:59:59". This is incorrect, because only the hour from 00:00 to 00:59 on 2016-10-16 is missing. The end of 2016-10-16 is still 23:59:59 on the same date.

This is also reproducible with moment-timezone:

moment.tz("2016-10-16","America/Sao_Paulo").endOf('day').format("YYYY-MM-DD HH:mm:ss")

The problem is in the endOf method. To compute the end of the day, we take the start of the day, add one day, then subtract one millisecond. That logic is flawed when the day doesn't start at midnight. Instead of adding one day, we need to add the exact duration length of the particular day.

[mattjohnsonpint]

This was originally reported as moment/moment-timezone#327.

#2749 is related, but slightly different.

[maggiepint]

As opposed to computing the exact length of time of the day, can't you just do:

moment('2016-10-16').startOf('day').add(1, 'day').startOf('day').subtract(1, 'millisecond').format()
"2016-10-16T23:59:59-02:00"

Works fine on the 15th when pushed into the 16th as well:

moment('2016-10-15').startOf('day').add(1, 'day').startOf('day').subtract(1, 'millisecond').format()
"2016-10-15T23:59:59-03:00"

Is there an edge case that I"m not thinking of?

I suppose there might be a performance benefit to doing it differently, but heck, you can read it!

[mattjohnsonpint]

Hmmmm, something else screwy here:

moment.tz("2016-10-16","America/Sao_Paulo").startOf('day').add(1,'day').startOf('day').subtract(1,'ms').format()
// "2016-10-16T23:59:59-02:00"  (ok)

moment.tz("2016-10-15","America/Sao_Paulo").startOf('day').add(1,'day').startOf('day').subtract(1,'ms').format()
// "2016-10-14T23:59:59-03:00"  (wrong date)

moment.tz("2016-10-15","America/Sao_Paulo").startOf('day').add(1,'day').format()
// "2016-10-15T23:00:00-03:00"  should have skipped forward to "2016-10-16T01:00:00-02:00"

Are we just dealing with the whole ambiguous browser behavior here? Or is there something more to it?

[maggiepint]

In chrome with the windows timezone of Brasillia:

moment("2016-10-15").startOf('day').format()
"2016-10-15T00:00:00-03:00"
moment("2016-10-15").startOf('day').add(1, 'day').format()
"2016-10-16T01:00:00-02:00"

In chrome with my timezone using Moment Timezone:

moment.tz("2016-10-15","America/Sao_Paulo").startOf('day').format()
"2016-10-15T00:00:00-03:00"
moment.tz("2016-10-15","America/Sao_Paulo").startOf('day').add(1, 'day').format()
"2016-10-15T23:00:00-03:00"

COOL!

Something up with moment timezone? I don't really know that code so it's hard to say. Doesn't seem like the browser if the first works though.

[maggiepint]

I think that this is a result of the fact that moment timezone will always push an invalid date back, if it has been 'added' into that date. Whether by design or on accident I am not sure.

//setting moves forward
moment.tz("2016-10-16T00:00:00","America/Sao_Paulo").format()
"2016-10-16T01:00:00-02:00"
//adding moves backward
moment.tz("2016-10-15T00:00:00","America/Sao_Paulo").add(1, 'day').format()
"2016-10-15T23:00:00-03:00"

//setting moves forward
moment.tz("2016-03-13T02:00:00","America/Chicago").format()
"2016-03-13T03:00:00-05:00"
//adding moves backward
moment.tz("2016-03-12T02:00:00","America/Chicago").add(1, 'day').format()
"2016-03-13T01:00:00-06:00"

//one more time for funzies
moment.tz("2016-03-27T01:00:00","Europe/London").format()
"2016-03-27T02:00:00+01:00"

moment.tz("2016-03-26T01:00:00","Europe/London").add(1, 'day').format()
"2016-03-27T00:00:00Z"

I think this has something to do with trying to use the keepTime setting when you can't keep the time. I don't know this code well enough to tell you why the code is the way it is, but I know for sure that if the keepTime call in this code were 0 instead of 1, everything would go forward as expected. It's because keep time is on that it's pushed back. And keep time should be on, it just is funky with this edge case.

    moment.updateOffset = function (mom, keepTime) {
        var zone = moment.defaultZone,
            offset;

        if (mom._z === undefined) {
            if (zone && needsOffset(mom) && !mom._isUTC) {
                mom._d = moment.utc(mom._a)._d;
                mom.utc().add(zone.parse(mom), 'minutes');
            }
            mom._z = zone;
        }
        if (mom._z) {
            offset = mom._z.offset(mom);
            if (Math.abs(offset) < 16) {
                offset = offset / 60;
            }
            if (mom.utcOffset !== undefined) {
                mom.utcOffset(-offset, keepTime);
            } else {
                mom.zone(offset, keepTime);
            }
        }
    };

[icambron]

I don't know enough about moment-timezone to be helpful here, but perhaps the ticket on why keepTime is used will be helpful to anyone investigating: moment/moment-timezone#28

[ichernev]

#1564

This is the last big change in handling zones in moment. Not sure if its related to this issue, but it should help.

@maggiepint can you explain a bit the

will always push an invalid date back

part. I mean -- do we have an example that exposes a problem with moment-timezone, or we just need some smart code in moment to detect DST at the start/end of the day and handle it appropriately. The second is medium-to-easy. If we have to refactor again the zone passing between the moment-tz and moment, then its hell -- I'd rather wait for 3.0 and hope the new interface won't have this issue.

[maggiepint]

@ichernev the way the code stands today, we have two issues in this one issue. One is the issue that @mj1856 originally raised, which is that days that don't start at midnight end up with a funky end of day value. This is a moment issue.

The other is that moment timezone, when presented with an invalid date, goes forward if that invalid date is used in the constructor, but it goes backward if it is 'added' into that date. This is, as it stands, an issue with moment timezone and not moment. I'm not sure how the timezone interface changes that - I would have to think about it a bit.

[ichernev]

@maggiepint thank you for the clear explanation.

So to fix the moment problem : endOf('day') being inaccurate -- just "aim" for the end of the day, go there, if its 00:00, finish, otherwise aim again, if the second aim reaches 00:00 we're done otherwise use the first shot. The second aim will help if we jump across DST and are "distorted" by an hour. If the end of day is an invalid time because of DST, the second aim won't hurt.

We should use this algo for all endOf, maybe even startOf, but that is harder to reason with the set-to-zero we currently do.

[MarcosEich]

Hi guys, I am also being affected by this issue on the current version of moment (2.14.1).

[keithwillcode]

I have #3716 submitted to fix this, but am currently seeing issues with Travis CI for transpile reasons. Will be looking into it more soon.

[m0nzderr]

This one is also related (default Brazilian locale).

Operation results in the same date:

moment("2017-10-16T00:59:59.999").subtract(1,'day').endOf('day')
=> "2017-10-16T00:59:59.999"

[Seldaek]

Hey - I just submitted #4164 which fixes this for startOf as well as endOf. Would love to get feedback there so we can hopefully finally move this forward.

[darakeon]

@mj1856 , I closed my PR because the problem with endOf in DST edge seems to be fixed. Could this be fixed too?

[lourenci]

I have tested the #4164 and this looks like be fixed on it. Waiting for merge.