fix: account for negative timestamps from rrweb (#17849)

PostHog · Oct 23, 2023 · d8b7c32 · d8b7c32
1 parent abb5b59
commit d8b7c32
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 4 deletions.
diff --git a/plugin-server/src/worker/ingestion/process-event.ts b/plugin-server/src/worker/ingestion/process-event.ts
@@ -397,17 +397,27 @@ export const gatherConsoleLogEvents = (
     return consoleLogEntries
 }
 
+export const getTimestampsFrom = (events: RRWebEvent[]): ClickHouseTimestamp[] =>
+    events
+        // from millis expects a number and handles unexpected input gracefully so we have to do some filtering
+        // since we're accepting input over the API and have seen very unexpected values in the past
+        // we want to be very careful here before converting to a DateTime
+        // TODO we don't really want to support timestamps of 1,
+        //  but we don't currently filter out based on date of RRWebEvents being too far in the past
+        .filter((e) => (e?.timestamp || -1) > 0)
+        .map((e) => DateTime.fromMillis(e.timestamp))
+        .filter((e) => e.isValid)
+        .map((e) => castTimestampOrNow(e, TimestampFormat.ClickHouse))
+        .sort()
+
 export const createSessionReplayEvent = (
     uuid: string,
     team_id: number,
     distinct_id: string,
     session_id: string,
     events: RRWebEvent[]
 ) => {
-    const timestamps = events
-        .filter((e) => !!e?.timestamp)
-        .map((e) => castTimestampOrNow(DateTime.fromMillis(e.timestamp), TimestampFormat.ClickHouse))
-        .sort()
+    const timestamps = getTimestampsFrom(events)
 
     // but every event where chunk index = 0 must have an eventsSummary
     if (events.length === 0 || timestamps.length === 0) {

diff --git a/plugin-server/tests/main/process-event.test.ts b/plugin-server/tests/main/process-event.test.ts
@@ -36,6 +36,7 @@ import {
     createSessionReplayEvent,
     EventsProcessor,
     gatherConsoleLogEvents,
+    getTimestampsFrom,
     SummarizedSessionRecordingEvent,
 } from '../../src/worker/ingestion/process-event'
 import { delayUntilEventIngested, resetTestDatabaseClickhouse } from '../helpers/clickhouse'
@@ -1404,6 +1405,31 @@ const sessionReplayEventTestCases: {
             message_count: 1,
         },
     },
+    {
+        snapshotData: {
+            events_summary: [
+                // a negative timestamp is ignored
+                { timestamp: 1682449093000, type: 3, data: { source: 2 }, windowId: '1' },
+                { timestamp: 1682449095000, type: 3, data: { source: 2 }, windowId: '1' },
+                { timestamp: -922167545571, type: 3, data: { source: 2 }, windowId: '1' },
+            ],
+        },
+        expected: {
+            click_count: 3,
+            keypress_count: 0,
+            mouse_activity_count: 3,
+            first_url: null,
+            first_timestamp: '2023-04-25 18:58:13.000',
+            last_timestamp: '2023-04-25 18:58:15.000',
+            active_milliseconds: 1,
+            console_log_count: 0,
+            console_warn_count: 0,
+            console_error_count: 0,
+            size: 217,
+            event_count: 3,
+            message_count: 1,
+        },
+    },
     {
         snapshotData: {
             events_summary: [
@@ -1483,6 +1509,20 @@ test(`snapshot event with no event summary timestamps is ignored`, () => {
     }).toThrowError()
 })
 
+test.each([
+    { events: [], expectedTimestamps: [] },
+    { events: [{ without: 'timestamp property' } as unknown as RRWebEvent], expectedTimestamps: [] },
+    { events: [{ timestamp: undefined } as unknown as RRWebEvent], expectedTimestamps: [] },
+    { events: [{ timestamp: null } as unknown as RRWebEvent], expectedTimestamps: [] },
+    { events: [{ timestamp: 'what about a string' } as unknown as RRWebEvent], expectedTimestamps: [] },
+    // we have seen negative timestamps from clients 🙈
+    { events: [{ timestamp: -1 } as unknown as RRWebEvent], expectedTimestamps: [] },
+    { events: [{ timestamp: 0 } as unknown as RRWebEvent], expectedTimestamps: [] },
+    { events: [{ timestamp: 1 } as unknown as RRWebEvent], expectedTimestamps: ['1970-01-01 00:00:00.001'] },
+])('timestamps from rrweb events', ({ events, expectedTimestamps }) => {
+    expect(getTimestampsFrom(events)).toEqual(expectedTimestamps)
+})
+
 function consoleMessageFor(payload: any[]) {
     return {
         timestamp: 1682449093469,