harsh icon indicating copy to clipboard operation
harsh copied to clipboard

`harsh ask` doesn't work correctly between 1am and 2am just after DST happens

Open TheodoreEhrenborg opened this issue 4 years ago • 7 comments

Describe the bug

Between 1am and 2am, harsh ask does not ask me about today's habits, even if they are not done. It will still ask me about yesterday's habits if they are not done.

To Reproduce

Steps to reproduce the behavior:

  1. Let's say today is Saturday.
  2. At 11:10 pm on Saturday, run harsh ask and answer all the questions as normal.
  3. At 12:10 am on Sunday morning, run harsh ask. Observe that it asks you for Sunday's habits, as expected. Don't give answers to any of the questions.
  4. At 1:10 am on Sunday morning, run harsk ask. Observe that it prints nothing, as if there were no habits to do on Sunday.
  5. At 2:10 am on Sunday morning, run harsh ask. Observe that it asks you for Sunday's habits, just like it did two hours ago.

Make sure your habit and log files are where they should be:

~/.config/harsh/habits
~/.config/harsh/log

Yup, they're there.

Expected behavior

I expected harsh ask to ask me about today's habits even if I run it at 1:10 am.

Environment (please complete the following information):

  • OS: Mac
  • OS version: Monterey 12.3
  • Harsh version 0.8.16

Additional context I live in the UK, and hence my computer is in British Summer Time. When I say 1am, I mean in British Summer Time.

I'd guess that this bug is caused by the Go library/something unique about my computer/something else, and not by harsh. I don't actually need to use harsh between 1am and 2am, so I'm posting this bug mostly because I'm curious why harsh does this.

P.S. I think harsh is great, and it has made me better at getting habits done.

TheodoreEhrenborg avatar Apr 09 '22 22:04 TheodoreEhrenborg

@TheodoreEhrenborg

Interestingly enough, I live in a country without DST (we're on the equator) so it's completely possible there may be a DST bug. Will have to test this and see. Thanks for the report! I'm assuming it's not that debilitating at the moment, so gimme till after the weekend on this one to check it out.

thanks for the super intresting bug report! (he said as he thought about how to write tests that emulate british summer time... 8-/ ).

wakatara avatar Apr 20 '22 08:04 wakatara

Thanks for getting back to me! I almost never use harsh after 1am, so this bug isn't a big problem.

TheodoreEhrenborg avatar Apr 20 '22 10:04 TheodoreEhrenborg

@TheodoreEhrenborg

Wow, this one is super fascinating, but not technically a bug, I think. It looks like this happens at the boundary transition of daily savings time because of the way Go Lang handles DST in general. Are you still seeing this behaviour now? In other words, did this happen just on the cusp of Britain folding into DST, or you are still experiencing it now (cause realizing with my 100 days loops you may be.). I will have to work a special condition in here to handle this but it is definitely a peculiarity of Go (imho).

Link to a 2020 filed issue son this: https://github.com/golang/go/issues/41272#issuecomment-689142737

Repasted for brevity tho this is similar to how my code works - I add one day to each date so imagine this is very related to it though will have to look more deeply into the edge case.

The result is surprising but this is working as documented. time.Time.AddDate will adjust the time, and then, as the documentation says, normalize the result just as is done by time.Date.

In your example we add one day to 2020-09-05 midnight. That gives us 2020-09-06 midnight. But in Chile there is no such time. There is 2020-09-06 23:59:59 -0400. And there is 2020-09-06 01:00:00 -0300. But there is no midnight. So we have to normalize. There are two plausible choices: 2020-09-05 23:00:00 -0400 and 2020-09-06 01:00:00 -0300. As the documentation for time.Date says, in this ambiguous cases we make no guarantee as to which one we select. In this case we happen to choose 2020-09-05 23:00:00, and the timezone for that has to be -0400. And that is the first 23 hour day.

Then the code adds 1 day to that. There result is unambiguous: adding one day to a time at 23:00:00, gives the next day at 23:00:00. In this case the range between those two times includes the skipped hour between midnight and 1am. So that is the second 23 hour day.

So although the results are confusing, there really is no bug. Unfortunately, at a daylight savings time transition, you need to figure out what you want to do. For example, instead of time.AddDate(0, 0, 1), you could write time.Add(24 * time.Hour). Then the difference would always be 24 hours, but your times would start being at 1am instead of midnight. You'll have to decide what is right for your situation.

Closing the issue because the code is working as documented.

Lol... one of my friends often complained about how many issues timezones caused, but this is amazing. I will try and see if I can force to UST and offset Time.now with timezone to "figure out" which day you are after a transition.

Fascinating bug! I am kinda agog. Thanks for pointing this one out! (though imagine it is going to be a total PITA to fix.

wakatara avatar Apr 20 '22 13:04 wakatara

Oh wow, so I can reproduce the problem now and it is related to the above. This only seems to occur if the harsh ask "lookback" date crosses the boundary between DST and non-DST dates.

Example:

~/Code/dev/go/harsh master !1 ❯ go run ./harsh.go ask                                                                                                     1.18.1
2022-04-20 01:30:00 +0100 BST
2022-03-21 01:30:00 +0000 GMT
2022-04-19:

Dailies
                  Read  ━━━━━━─━━─•·· ━─━━─━━─━━━━━━━  [y/n/s/⏎] 

vs. with both dates within BST bounds.

~/Code/dev/go/harsh master !1 ❯ go run ./harsh.go ask                                                                                                     1.18.1
2022-04-20 01:30:00 +0100 BST
2022-03-31 01:30:00 +0100 BST
2022-04-19:

Dailies
                  Read  •·· ━─━━─━━─━━━━━━━ ! [y/n/s/⏎] 
2022-04-20:

Dailies
              Slept 8h       ━   ━   ━━━━━  ! [y/n/s/⏎] 

Kinda a cool, if slightly terrifying bug in Go Lang (my Rust implementation of same code is not affected by this as Rust has a NaiveDate type in the chrono crate which counts days as dates rather than counts backwards with the Time primitive in Go Lang.). I used a standard Go lang pattern to iterate over time as dates in GoLang so imagine similar issues must hit a lot of folks.

Will play around with a "we're crossing DST so gotta adjust" check though need to write some tests to check. It definitely borders on one of the weirder bugs I've seen (like if it happened midnight to 1am I would get it... but 1am to 2am??).

wakatara avatar Apr 20 '22 14:04 wakatara

Further realized the DST hour change rules are different in almost every country that implements DST. In the UK it's at 1am. 2am in Canada. Different also in Chile. So, appears to only be relevant for the first hour of DST changes offset to non-DST. Hmmm...

Yeesh. OK, luckily there is a time.IsDST() but atm unsure how to puzzle this out with golang's time element since it explicitly avoids dealing with day changes across DST boundaries as days (cowards 😝).

wakatara avatar Apr 20 '22 15:04 wakatara

Are you still seeing this behaviour now? In other words, did this happen just on the cusp of Britain folding into DST, or you are still experiencing it now (cause realizing with my 100 days loops you may be.).

Yup, it's still happening as of the most recent time I checked (last Sunday?). I first noticed the behavior after the UK switched into DST. I think that was the first time I ever tried to use harsh after 1am, so I don't know how it would have responded before then.

Thanks for looking at the (complicated) DST rules. I hope there's a simple solution.

TheodoreEhrenborg avatar Apr 21 '22 09:04 TheodoreEhrenborg

I think you'll see it happen for 8 days after DST kicked in at the 1am time since that is the lookback window.

The issue is actually in the getTodos() function by the looks of things that AskHabit() calls.

Thanks for bringing it to my attention. Fascinating, if scary problem, and learned a little something about GoLang I did not know before!.

wakatara avatar Apr 21 '22 11:04 wakatara

Heya @TheodoreEhrenborg !

I just pushed 0.8.19 which should fix this bug as I've refactored harsh to use civil, a naivedate library from google, rather than use the underlying time.Time function which caused (and ultimately meant the problem was ridiculously hard to solve because of the way Go implements DST and its AddDate function.

Also, made the code a bit more compact as less time to string conversion and easier reasoning since civil implments a year, month, day struct.

Enjoy! 😀

wakatara avatar Oct 03 '22 06:10 wakatara