[superlog] Reject unrecognized timezones before passing to ClickHouse#488
[superlog] Reject unrecognized timezones before passing to ClickHouse#488superlog-app[bot] wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
The latest updates on your projects. Learn more about Unkey Deploy
|
|
Found 7 test failures on Blacksmith runners: Failures
|
Greptile SummaryThis PR fixes a live incident where browsers reporting
Confidence Score: 4/5The fix is targeted and correct — the Intl.DateTimeFormat probe reliably blocks the specific class of bad timezones that triggered the incident. The only rough edge is that rejected timezones are silently substituted with UTC, giving callers no signal that their parameter was discarded. The core change is small and well-tested: adding an Intl.DateTimeFormat guard inside validateTimezone and threading it through both query handlers. The validation logic is sound, the tests cover the incident case directly, and the fallback is safe. The import placement is a style nit, and the silent UTC substitution is an observability gap rather than a correctness issue — but callers sending an invalid timezone will silently receive UTC-bucketed data with no indication anything went wrong. apps/api/src/routes/query.ts — the import ordering and the silent UTC fallback with no log are both in this file. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Browser
participant API as /v1/query (Elysia)
participant Validator as validateTimezone()
participant CH as ClickHouse
Browser->>API: "POST ?timezone=Etc/Unknown"
API->>Validator: validateTimezone("Etc/Unknown")
Note over Validator: regex passes (chars ok)<br/>Intl.DateTimeFormat throws RangeError
Validator-->>API: "" (empty string)
API->>API: "" || "UTC" → "UTC"
API->>CH: "query with timeZone="UTC""
CH-->>API: results
API-->>Browser: HTTP 200 (UTC data, no error signal)
Browser->>API: "POST ?timezone=America/New_York"
API->>Validator: validateTimezone("America/New_York")
Note over Validator: regex passes<br/>Intl.DateTimeFormat succeeds
Validator-->>API: "America/New_York"
API->>CH: "query with timeZone="America/New_York""
CH-->>API: results
API-->>Browser: HTTP 200 (correct data)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Browser
participant API as /v1/query (Elysia)
participant Validator as validateTimezone()
participant CH as ClickHouse
Browser->>API: "POST ?timezone=Etc/Unknown"
API->>Validator: validateTimezone("Etc/Unknown")
Note over Validator: regex passes (chars ok)<br/>Intl.DateTimeFormat throws RangeError
Validator-->>API: "" (empty string)
API->>API: "" || "UTC" → "UTC"
API->>CH: "query with timeZone="UTC""
CH-->>API: results
API-->>Browser: HTTP 200 (UTC data, no error signal)
Browser->>API: "POST ?timezone=America/New_York"
API->>Validator: validateTimezone("America/New_York")
Note over Validator: regex passes<br/>Intl.DateTimeFormat succeeds
Validator-->>API: "America/New_York"
API->>CH: "query with timeZone="America/New_York""
CH-->>API: results
API-->>Browser: HTTP 200 (correct data)
Reviews (1): Last reviewed commit: "[superlog] Reject unrecognized timezones..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| (async () => { | ||
| const requestId = generateRequestId(); | ||
| const timezone = q.timezone || "UTC"; | ||
| const timezone = validateTimezone(q.timezone) || "UTC"; |
There was a problem hiding this comment.
When validateTimezone rejects a timezone (returns ""), both the execute and compile paths silently substitute "UTC" and return HTTP 200. The caller has no programmatic signal that their ?timezone= parameter was discarded — they receive data bucketed in UTC without any indication. If a user's browser reports an unusual-but-passing-regex timezone that is rejected by Intl, their dashboard will silently display UTC times. Adding at minimum a server-side warning log (and ideally a response header such as X-Applied-Timezone: UTC) would make these silent substitutions visible during incident debugging.
| import { db } from "@databuddy/db"; | ||
| import { validateTimezone } from "@databuddy/validation"; | ||
| import { readBooleanEnv } from "@databuddy/env/boolean"; |
There was a problem hiding this comment.
The new @databuddy/validation import (package initial v) is inserted between @databuddy/db (d) and @databuddy/env/boolean (e). It should appear after the @databuddy/shared/… imports. If Ultracite/Biome's useSortedImports rule is active, this will produce a lint error.
| import { db } from "@databuddy/db"; | |
| import { validateTimezone } from "@databuddy/validation"; | |
| import { readBooleanEnv } from "@databuddy/env/boolean"; | |
| import { db } from "@databuddy/db"; | |
| import { readBooleanEnv } from "@databuddy/env/boolean"; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Analytics dashboard queries fail with ClickHouse error code 36 (
Cannot load time zone Etc/Unknown) when a user's browser reports an unrecognized timezone such asEtc/Unknown. The API passes the raw?timezone=query parameter directly to ClickHouse without validating it against the IANA timezone database, so any unknown timezone name causes every query in the batch to fail, leaving the dashboard with no data even though the request returns HTTP 200.The existing
validateTimezonehelper in@databuddy/validationonly checks character format (/^[A-Za-z_/+-]{1,64}$/), whichEtc/Unknownpasses. It never verifies that the timezone is actually recognized by the runtime. The fix adds anIntl.DateTimeFormatprobe insidevalidateTimezoneso that any timezone the JS runtime rejects is also rejected by the helper. The/v1/queryexecute and compile handlers are updated to runvalidateTimezoneon the incoming timezone param instead of using it raw, falling back to"UTC"when validation fails.Alternative approach: validate at the ClickHouse query layer (e.g., in
SimpleQueryBuilder.compile) and substituteUTCthere. This patch chooses the earlier sanitization point so the fix benefits all call sites that usevalidateTimezone, not only the query route.Incident on Superlog
Was this PR helpful? Leave feedback — goes straight to the Superlog team.
Summary by cubic
Reject unrecognized IANA timezones and default to UTC in query endpoints to prevent ClickHouse error 36 and broken analytics dashboards. Timezone input is now validated with
Intl.DateTimeFormatviavalidateTimezonein@databuddy/validation.validateTimezone; invalid names return an empty string./v1/querycompile and execute now usevalidateTimezone(q.timezone) || "UTC".Etc/UnknownandAmerica/Fakecity.Written for commit 0ec3162. Summary will update on new commits.