Daylight Saving Time causes parseFrequency to be mismatched

[FoxxMD]

As discovered by @megatwig here in my project the function genNextDay adding 24 hours for next day is not sufficient for Daylight Savings Time when a day "has" 25 hours due to clocks being set back.

function getNextDay (start) {
  return new Date(start + 24 * 60 * 60 * 1000).setHours(0, 0, 0, 0)
}

This is causing a log file to be created on every pino log statement as the filename does not match what pino-roll expects.

To Reproduce

I have modified utils.test.js in my branch dstIssue so that is fails when running on the date of DST.

To do this manually in utils.test.js line 43 (in test('parseFrequency()' ...) change const today to

const today = new Date('27 Oct 2024 00:00:00 GMT+0100')

and run from the project root with a timezone that would cause DST to occur:

TZ=Europe/Berlin node test/lib/utils.test.js

TAP version 13
# Subtest: parseSize()
    ok 1 - returns null on empty input
    ok 2 - handles input in B
    ok 3 - handles input in KB
    ok 4 - handles input in KB, capital
    ok 5 - handles input in MB, capital
    ok 6 - handles input in MB, capital
    ok 7 - considers numerical input as MB
    ok 8 - considers no unit as MB
    ok 9 - handles input in GB
    ok 10 - throws on empty string
    ok 11 - throws on non parseable string
    1..11
ok 1 - parseSize() # time=11.383ms

# Subtest: parseFrequency()
    ok 1 - returns null on empty input
    not ok 2 - supports daily frequency
      ---
      diff: |
        --- expected
        +++ actual
        @@ -1,5 +1,5 @@
         Object {
           "frequency": "daily",
        -  "start": null,
        -  "next": null,
        +  "start": 1730070000000,
        +  "next": 1730156400000,
         }
      at:
        line: 44
        column: 3
        file: test/lib/utils.test.js
        type: Test
      stack: |
        Test.<anonymous> (test/lib/utils.test.js:44:3)
      source: >2
          same(parseFrequency(), null, 'returns null on empty input')
          same(
        --^
            parseFrequency('daily'),
            { frequency: 'daily', start: startOfDay(today).getTime(), next: startOfDay(addDays(today, 1)).getTime() },
      ...

    not ok 3 - supports hourly frequency
      ---
      diff: |
        --- expected
        +++ actual
        @@ -1,5 +1,5 @@
         Object {
           "frequency": "hourly",
        -  "start": null,
        -  "next": null,
        +  "start": 1730120400000,
        +  "next": 1730124000000,
         }
      at:
        line: 49
        column: 3
        file: test/lib/utils.test.js
        type: Test
      stack: |
        Test.<anonymous> (test/lib/utils.test.js:49:3)
      source: >2
          )
          same(
        --^
            parseFrequency('hourly'),
            { frequency: 'hourly', start: startOfHour(today).getTime(), next: startOfHour(addHours(today, 1)).getTime() },
      ...

    ok 4 - supports custom frequency and does not return start
    ok 5 - throws on non parseable string
    1..5
    # failed 2 of 5 tests
not ok 2 - parseFrequency() # time=17.768ms

Tested on pino-roll 1.3.0 and 2.1.0

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[FoxxMD]

Yeah I'm working on a fix. The test to reproduce is not exactly right but the issue is the same.

[FoxxMD]

@mcollina PR is opened :) I also added a test for DST -> Standard Time that occurs in March. genNextDay/Hour were already OK for this since the time change is forwards instead of backwards but I went ahead and replaced with date-fns for genNextHour so both functions use (and account for) DST quirks the same.