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

Fix DST boundary issue occurring on Cloudflare env (fixes #108) #114

Merged
merged 1 commit into from
Nov 16, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions app/analytics/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { SearchFilters } from "~/lib/types";

import dayjs, { ManipulateType } from "dayjs";
import dayjs from "dayjs";
import utc from "dayjs/plugin/utc";
import timezone from "dayjs/plugin/timezone";

Expand Down Expand Up @@ -94,26 +94,28 @@

const initialRows: { [key: string]: number } = {};

// NOTE: Need to explicitly use dayjs to increment by 1 day/1 hour/etc, because
// dayjs will respect and adjust for daylight savings time boundaries.
//
// For example, in 2024, Daylight Savings Time began on 3/10/24 at 2:00 AM.
// In UTC, before the switch, America/New_York was 5 hours behind UTC (+5:00).
// But after the switch, it becomes 4 hours behind UTC (+4:00). Dayjs
// accounts for this difference.
//
// There is no unit test affirming this behavior, because I could not figure
// out how to get vitest/mock dates to recreate DST changes.
// See: https://github.com/benvinegar/counterscale/pull/62

while (startDateTime.getTime() < endDateTime.getTime()) {
const key = dayjs(startDateTime).utc().format("YYYY-MM-DD HH:mm:ss");
initialRows[key] = 0;

startDateTime = dayjs(startDateTime)
// increment by either DAY or HOUR
.add(1, intervalType.toLowerCase() as ManipulateType)
.toDate();
if (intervalType === "DAY") {
// WARNING: Daylight savings hack. Cloudflare Workers uses a different Date
// implementation than Node 20.x, which doesn't seem to respect DST
// boundaries the same way(see: https://github.com/benvinegar/counterscale/issues/108).
//
// To work around this, we add 25 hours to the start date/time, then get the
// start of the day, then convert it back to a Date object. This works in both
// Node 20.x and Cloudflare Workers environments.
startDateTime = dayjs(startDateTime)
.add(25, "hours")
.tz(tz)
.startOf("day")
.toDate();
} else if (intervalType === "HOUR") {
startDateTime = dayjs(startDateTime).add(1, "hour").toDate();
} else {
throw new Error("Invalid interval type");
}

Check warning on line 118 in app/analytics/query.ts

View check run for this annotation

Codecov / codecov/patch

app/analytics/query.ts#L117-L118

Added lines #L117 - L118 were not covered by tests
}

return initialRows;
Expand Down
Loading