Local Time Conversions Across DST Shift - Bug or My Design Flaw?

[Brent_NC]

1) Give a description of the problem
As I was developing a piston related to calendar integrations, I came across a test scenario that I’m unsure how to interpret. It appears that as we cross the future EDT - to - EST transition in November, perhaps not unexpectedly, we lose an hour (“fall back”). However, I wouldn’t have expected a simple string conversion to date/time via the datetime() function to subtract the hour.

I apologize if this has been addressed in other topics. I was unable to find a conversation that addressed this particular issue. I did read the topics related to last November’s DST ST issues. However, am assuming those bugs were thoroughly squashed.

Thanks for your help! WebCoRE and this Community are awesome!!

2) What is the expected behavior?
Convert text “Nov 05, 2018 12:00 AM” to date/time value ‘11/05/2018, 12:00:00 AM’

3) What is happening/not happening?
While I was assuming that text “Nov 05, 2018 12:00 AM” would be converted to ‘11/05/2018, 12:00:00 AM’, it is actually be converted to ‘11/04/2018, 11:00:00 PM’.

4) Post a Green Snapshot of the piston

5) Attach any logs (From ST IDE and by turning logging level to Full)
( 8/18/2018, 9:20:56 PM +914ms
+1ms ╔Starting piston… (v0.3.107.20180806)
+223ms ║╔Subscribing to devices…
+319ms ║╚Finished subscribing (108ms)
+347ms ╚Piston successfully started (347ms)
8/18/2018, 8:43:32 PM +85ms
+1ms ╔Received event [Home].test = 1534639412085 with a delay of 0ms
+279ms ║RunTime Analysis CS > 17ms > PS > 244ms > PE > 18ms > CE
+282ms ║Runtime (39683 bytes) successfully initialized in 244ms (v0.3.107.20180806) (280ms)
+282ms ║╔Execution stage started
+288ms ║║Cancelling statement #1’s schedules…
+300ms ║║Executed virtual command setVariable (9ms)
+305ms ║║Executed virtual command setVariable (3ms)
+310ms ║║Executed virtual command setVariable (2ms)
+318ms ║║Executed virtual command setVariable (3ms)
+326ms ║║Executed virtual command setVariable (3ms)
+333ms ║║Executed virtual command setVariable (3ms)
+335ms ║╚Execution stage complete. (53ms)
+336ms ╚Event processed successfully (336ms))

[bangali]

cant replicate this. please share a green snapshot of the piston.

[Brent_NC]

Hi Bangali. Thanks for your response. I haven’t figured out how to post a green snapshot from an iPad. The picture comes up but I haven’t been able to figure out how to save that picture for upload to the forum. I’d appreciate any advice.

Note that the dates in my example probably only work for installations in US Eastern Timezones that switch back to Standard Time on 04-Nov (2:00 AM, I think).

[bangali]

unfortunately other than native screenshot on ipad dont know of a way to save or upload the image.

with a native screenshot longer pistons will get truncated in the image but at least the restore code of the piston will show up and i can use that to restore and take a look at the piston.

[WCmore]

If anyone is troubleshooting this, I just ran a dozen commands in my console:
(once for each month)

formatDateTime(time('Jan 06, 2019 12:00 AM'), "h:mm a")

and I noticed a definite pattern…

[bangali]

try one other thing … i see the func name you are using is Datetime change all instance of it to datetime and please check if that makes a difference.

[bangali]

looking a bit more it seems the piston is trying to adjust for DST and causing it to set back by an hour on that day. try adding a 3 digit timezone like EST and see if the results are more consistent.

[Brent_NC]

Here is the green snapshot with Datetime changed to datetime.

[Brent_NC]

…and a version with time zones explicitly designated… (Note Nov 5th being EST while Nov 3/4 are EDT)

[bangali]

thanks. when timezone is specified does it work right? no way to tell from the green snapshot

[ipaterson]

Time zone handling is just rough, this particular one is caused by using the current time zone offset to interpret future dates rather than the time zone on that date. There are probably better solutions for this but I’m pressed for time today. This instead interprets the time based on today’s time zone as a starting point then adjusts to the actual time zone on that date. This would certainly fail in the hour before/after daylight savings changes but seems resilient outside of that range.

// add local timezone
if (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)) {
    def d = (new Date()).parse(dateOrTimeOrString + ' ' + formatLocalTime(now(), 'z'))
    return (new Date()).parse(dateOrTimeOrString + ' ' + formatLocalTime(d, 'z'))
} 
return (new Date()).parse(dateOrTimeOrString)

@bangali if you have time please feel free to come up with a much better way to do this. I was able to test in the expression evaluator, as you suspected this only affects dates without an explicit timezone (noting that the expected format is an explicit numeric offset like -0500 or abbreviated code and that EST/EDT are two separate offsets).

The original code in webcore-piston.groovy is in localToUtcTime around line 7483

return (new Date()).parse(dateOrTimeOrString + (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)? ' ' + formatLocalTime(now(), 'z') : ''))

A few lines down the time zone is taken from location.timeZone which uses IANA format (e.g. America/New_York) from which all offsets can be determined. That would be more accurate if it is reliably available.

[bangali]

no worries … i will take a look if i have time. for now if the user uses 3 letter timezone the time should translate fine.

The original code in webcore-piston.groovy is in localToUtcTime around line 7483

the challenge is it uses the current timezone of the location instead of the timezone at the time of the date string. which means whenever current timezone is different from target date timezone it will break. eg EST -> EDT or EDT -> EST.

dont see an easy solution for that.

[ipaterson]

the challenge is it uses the current timezone of the location instead of the timezone at the time of the date string. which means whenever current timezone is different from target date timezone it will break.

Correct, I suspect that location.timeZone is a better way to handle this case rather than what I did above to get close for the first parse then reinterpret the date relative to the offset at that date. With these sorts of things I am never sure if there was good reason to do it the other way or if it’s just oversight.

I’ll take a look at the commit history for those when I have a chance… this is a core bit of code so I would need to make sure that there isn’t anything else adjusting for its inaccuracy that would then break once it’s fixed

[bangali]

Correct, I suspect that location.timeZone is a better way to handle this case rather than what I did above to get close for the first parse then reinterpret the date relative to the offset at that date.

it does use the location timezone:

return (new Date()).parse(dateOrTimeOrString + (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)? ' ' + formatLocalTime(now(), 'z') : ''))

formatLocalTime(…) uses location.timeZone this is around line 7522:

formatter.setTimeZone(location.timeZone)

what we need is a function which based on current location checks if it uses DST for that location and if it does then calculate DST change between current and target date and adjust the time accordingly.

[Brent_NC]

thanks. when timezone is specified does it work right? no way to tell from the green snapshot

Hi Bangali. Yes. I think so. If I designate the post-DST date/time as EST vs EDT, it converts as expected. Thanks again for looking into this!

[Brent_NC]

Time zone handling is just rough, this particular one is caused by using the current time zone offset to interpret future dates rather than the time zone on that date.

Absolutely agree. Looking forward to the day we all simply operate on UTC.

My current track of thinking on this is that it’s more of a (my) perception problem than a logic problem. I agree that trying to fix this in the core may well break other things. If you look at the results below, when I feed the datetime function “Nov 05, 2018 12:00 AM EDT”, the system is smart enough to know that my use of EDT with 05-Nov-18 in my region is really an invalid nomenclature since EST will have started the day before. It feeds back to me a corrected date & time of 04-Nov-2018 11:00 PM EST. Similarly, if I omit the time zone, the system just assumes I’m referring to my current time zone of EDT and responds similarly.

So if I want the system to process a specific, future date / time, I need to be more specific about my own assumptions and use a valid time zone, appropriate to the period.

I could be completely off base here but wanted to plant an alternate thread of thought that may lead back to addressing this as a Piston design question vs bug fix. Thanks!

[bangali]

its not when a tz is specified … its when a tz is not specified that theres a bug.

[bangali]

replacing this line in localToUtcTime:

return (new Date()).parse(dateOrTimeOrString + (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)? ' ' + formatLocalTime(now(), 'z') : ''))

with this code should fix it. needs a bit more testing off course but only affects strings passed in to datetime function without timezone:

 def newDate = (new Date()).parse(dateOrTimeOrString + (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)? ' ' + formatLocalTime(now(), 'z') : ''))
 if (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/))	{
    def currentOffset = (new Date(now())).format("Z", location.timeZone)
    def newOffset = (new Date(newDate)).format("Z", location.timeZone)
    if (currentOffset != newOffset)
       newDate = newDate + ((currentOffset.toInteger() - newOffset.toInteger()) * 36000L)
 }
 return newDate

[Brent_NC]

Thanks Bangali!

I’m unfamiliar with the release management processes here. I am assuming your fix has not been applied to the core system. Is that correct? Merely rerunning the same piston yields unchanged results for me.

x

[ipaterson]

I am assuming your fix has not been applied to the core system.

Correct, we’re just spitballing solutions and publishing the changes privately to test for now.

 def newDate = (new Date()).parse(dateOrTimeOrString + (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)? ' ' + formatLocalTime(now(), 'z') : ''))
 if (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/))	{
    def currentOffset = (new Date(now())).format("Z", location.timeZone)
    def newOffset = (new Date(newDate)).format("Z", location.timeZone)
    if (currentOffset != newOffset)
       newDate = newDate + ((currentOffset.toInteger() - newOffset.toInteger()) * 36000L)
 }
 return newDate

I like the direction of this, it avoids parsing the date twice like my original solution and has the same reliability.

There do not seem to be any actual issues on the daylight savings boundaries (Nov 4, 2018 2:00am and Mar 10, 2019 2:00 am) with these approaches; just that people scheduling tasks on those days would need to realize that 1:30 AM will happen twice in November. A date of Mar 10, 2019 2:30 AM (technically an invalid time) automatically maps to 3:30 AM.

I have been unable to find documentation on that Date.parse method, all I can find is the static parse method which requires a format. The class method supports a variety of date formats but unlike the static method it doesn’t seem to support a time zone argument. I think our approach is the best way to handle this.

My next iteration uses the data available from the time zone:

if (!(dateOrTimeOrString =~ /(\s[A-Z]{3}((\+|\-)[0-9]{2}\:[0-9]{2}|\s[0-9]{4})?$)/)) {
    def newDate = (new Date()).parse(dateOrTimeOrString + ' ' + formatLocalTime(now(), 'Z'))
    def currentOffset = location.timeZone.getOffset(now())
    def newOffset = location.timeZone.getOffset(newDate)
    return newDate + (currentOffset == newOffset ? 0 : currentOffset - newOffset)
}
return (new Date()).parse(dateOrTimeOrString)

Yet another concern in this function is the abbreviations, many time zone abbreviations have more or fewer than 3 characters. If we attempt to create a date for Alaska (e.g. formatDateTime(datetime('Jan 3, 2018 4:00 pm AKST'), 'h:mm a')) even if the regex is fixed to allow it (new Date()).parse(dateOrTimeOrString) fails to parse it. I suppose there is nothing we can do about that other than encouraging people to use the offset rather than the abbreviation if their time zone is affected.