mirror of
https://github.com/supabase/supabase.git
synced 2026-10-06 18:05:11 +03:00
fix(logs): guard chart query against unparseable timestamp params (#47485)
## Problem The per-service logs pages (e.g. `/project/[ref]/logs/auth-logs`) crashed with `RangeError: Invalid time value` (Sentry issue 7580074952). `calcChartStart` guarded `iso_timestamp_start` only against falsy values, so a truthy-but-unparseable timestamp (a malformed value in the URL query params) produced an Invalid Date, which propagated through `.add()` and threw when `startOffset.toISOString()` was called. The bug is on the legacy (non-OTEL) chart query path. The OTEL bucket helper had the same unguarded pattern; it did not crash but could skew the chart bucket size. ## Fix Validate parsed timestamps with `dayjs().isValid()` and fall back to now, matching the existing empty-param behavior. Applied to both `calcChartStart` (legacy) and `otelChartTruncFn` (OTEL). - Valid params produce identical output (existing tests unaffected) - Empty params still fall back to now - Malformed input no longer throws Added regression tests to `Logs.utils.test.ts` and `Logs.utils.otel.test.ts`. ## How to test - Run the logs unit tests: `pnpm test:studio` (or target `Logs.utils.test.ts` and `Logs.utils.otel.test.ts`) - In the dashboard, open a service logs page with a malformed timestamp in the URL, e.g. `/project/<ref>/logs/auth-logs?its=not-a-date` - Expected result: the page renders without crashing and the chart falls back to the default (now-based) time range <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved log chart time-range handling by safely resolving missing or invalid ISO timestamps via a shared timestamp resolver. * Updated chart bucketing and timestamp conflict logic to use the resolved endpoints, preventing errors and ensuring correct fallback granularity (including minute-level bucketing when needed). * **Tests** * Added regression coverage to confirm chart query generation (including OTEL queries) does not throw for unparseable start/end timestamps. * Verified fallback behavior to minute-level bucketing and non-throwing behavior for timestamp conflict handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
32798c3162
commit
ceff7b99bf
4 files changed
+47
-13
No files matched your search
@@ -493,6 +493,14 @@ describe('genChartQueryOtel', () => {
|
||||
`)
|
||||
})
|
||||
|
||||
it('does not throw and falls back to minute buckets for an unparseable time range', () => {
|
||||
const badParams = { iso_timestamp_start: 'not-a-date', iso_timestamp_end: 'also-bad' }
|
||||
expect(() => genChartQueryOtel(LogsTableName.EDGE, badParams, {})).not.toThrow()
|
||||
expect(fmt(genChartQueryOtel(LogsTableName.EDGE, badParams, {}))).toContain(
|
||||
'toStartOfMinute (timestamp)'
|
||||
)
|
||||
})
|
||||
|
||||
it('defaults the chart bucket to minute when no time range is given', () => {
|
||||
expect(fmt(genChartQueryOtel(LogsTableName.EDGE, {}, {}))).toMatchInlineSnapshot(`
|
||||
"-- Logs Chart Query (otel) ['edge_logs']
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
import dayjs from 'dayjs'
|
||||
import { z } from 'zod'
|
||||
|
||||
import { LogsTableName, type SqlFilterEntry } from './Logs.constants'
|
||||
import type { Filters, LogData, LogsEndpointParams, Metadata, QueryType } from './Logs.types'
|
||||
import { buildWhereClauses } from './Logs.utils'
|
||||
import { buildWhereClauses, resolveLogTimestamp } from './Logs.utils'
|
||||
import { parseOtelTimestamp } from '@/data/logs/otel-inspection.utils'
|
||||
import {
|
||||
joinSqlFragments,
|
||||
@@ -280,8 +279,8 @@ SELECT count() AS count FROM logs ${genOtelWhere(table, filters)}`
|
||||
|
||||
// Bucket by minute up to 12h, otherwise by hour (matches BigQuery).
|
||||
const otelChartTruncFn = (params: LogsEndpointParams): SafeLogSqlFragment => {
|
||||
const ite = params.iso_timestamp_end ? dayjs(params.iso_timestamp_end) : dayjs()
|
||||
const its = params.iso_timestamp_start ? dayjs(params.iso_timestamp_start) : dayjs()
|
||||
const ite = resolveLogTimestamp(params.iso_timestamp_end)
|
||||
const its = resolveLogTimestamp(params.iso_timestamp_start)
|
||||
const minuteDiff = ite.diff(its, 'minute')
|
||||
const hourDiff = ite.diff(its, 'hour')
|
||||
if (minuteDiff > 60 * 12) return safeSql`toStartOfHour`
|
||||
|
||||
@@ -5,10 +5,12 @@ import type { Filters, LogData } from './Logs.types'
|
||||
import {
|
||||
buildLogsPrompt,
|
||||
checkForLimitClause,
|
||||
ensureNoTimestampConflict,
|
||||
extractEdgeFunctionName,
|
||||
formatLogsAsCsv,
|
||||
formatLogsAsJson,
|
||||
formatLogsAsMarkdown,
|
||||
genChartQuery,
|
||||
genDefaultQuery,
|
||||
getAuthLogSeverity,
|
||||
parseMultigresEventMessage,
|
||||
@@ -329,4 +331,24 @@ describe('Logs.utils', () => {
|
||||
expect(checkForLimitClause('select event_message from edge_logs -- limit 10')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('genChartQuery', () => {
|
||||
// Regression: an unparseable iso_timestamp_start used to reach
|
||||
// startOffset.toISOString() as an Invalid Date and throw RangeError.
|
||||
test('falls back to minute buckets for an unparseable time range without throwing', () => {
|
||||
const params = { iso_timestamp_start: 'not-a-date', iso_timestamp_end: 'also-bad' }
|
||||
expect(() => genChartQuery(LogsTableName.AUTH, params as any, {})).not.toThrow()
|
||||
expect(genChartQuery(LogsTableName.AUTH, params as any, {})).toContain(
|
||||
'timestamp_trunc(t.timestamp, minute)'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureNoTimestampConflict', () => {
|
||||
test('does not throw for unparseable initial timestamps', () => {
|
||||
expect(() =>
|
||||
ensureNoTimestampConflict(['not-a-date', 'also-bad'], ['', '2024-01-01T00:00:00.000Z'])
|
||||
).not.toThrow()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -365,13 +365,20 @@ export const genCountQuery = (table: LogsTableName, filters: Filters): SafeLogSq
|
||||
return safeSql`SELECT count(*) as count FROM ${LOG_TABLE_SQL[table]} ${joins} ${where}`
|
||||
}
|
||||
|
||||
// Falls back to now for missing or unparseable params so an Invalid Date can't
|
||||
// reach .toISOString() downstream and throw RangeError (Sentry 7580074952).
|
||||
export const resolveLogTimestamp = (value?: string): Dayjs => {
|
||||
const parsed = dayjs(value)
|
||||
return value && parsed.isValid() ? parsed : dayjs()
|
||||
}
|
||||
|
||||
/** calculates how much the chart start datetime should be offset given the current datetime filter params */
|
||||
const calcChartStart = (
|
||||
params: Partial<LogsEndpointParams>
|
||||
): [Dayjs, 'minute' | 'hour' | 'day'] => {
|
||||
const ite = params.iso_timestamp_end ? dayjs(params.iso_timestamp_end) : dayjs()
|
||||
const ite = resolveLogTimestamp(params.iso_timestamp_end)
|
||||
// todo @TzeYiing needs typing
|
||||
const its: any = params.iso_timestamp_start ? dayjs(params.iso_timestamp_start) : dayjs()
|
||||
const its: any = resolveLogTimestamp(params.iso_timestamp_start)
|
||||
|
||||
let trunc: 'minute' | 'hour' | 'day' = 'minute'
|
||||
let extendValue = 60 * 6
|
||||
@@ -449,14 +456,12 @@ export const ensureNoTimestampConflict = (
|
||||
[nextStart, nextEnd]: TsPair
|
||||
): TsPair => {
|
||||
if (initialStart && initialEnd && nextEnd && !nextStart) {
|
||||
const resolvedDiff = dayjs(nextEnd).diff(dayjs(initialStart))
|
||||
let start = dayjs(initialStart)
|
||||
const end = resolveLogTimestamp(nextEnd)
|
||||
let start = resolveLogTimestamp(initialStart)
|
||||
|
||||
if (resolvedDiff <= 0) {
|
||||
// start ts is definitely before end ts
|
||||
const currDiff = Math.abs(dayjs(initialEnd).diff(start, 'minute'))
|
||||
// shift start ts backwards by the current ts difference
|
||||
start = dayjs(nextEnd).subtract(currDiff, 'minute')
|
||||
if (end.diff(start) <= 0) {
|
||||
const currDiff = Math.abs(resolveLogTimestamp(initialEnd).diff(start, 'minute'))
|
||||
start = end.subtract(currDiff, 'minute')
|
||||
}
|
||||
return [start.toISOString(), nextEnd]
|
||||
} else if (!nextEnd && nextStart) {
|
||||
|
||||
Reference in new issue
Block a user