fix: report when player state is error #26989

pauldambra · 2024-12-17T19:48:22Z

let's capture an event when we show an error
reduce the likelihood we'll show an error unnecessarily
*fly-by improvements to the toolbar's event debugger cos i was using it to figure this out
don't overreport some settings changes

github-actions · 2024-12-17T19:59:53Z

Size Change: +2.9 kB (+0.26%)

Total Size: 1.11 MB

Filename	Size	Change
`frontend/dist/toolbar.js`	1.11 MB	+2.9 kB (+0.26%)

_{compressed-size-action}

posthog-bot · 2024-12-17T20:08:16Z

📸 UI snapshots have been updated

5 snapshot changes in total. 0 added, 5 modified, 0 deleted:

chromium: 0 added, 5 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-12-17T20:28:11Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra · 2024-12-17T20:40:21Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

@@ -1018,7 +1018,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
            cache.pausedMediaElements = values.endReached ? [] : playingElements
        },
        restartIframePlayback: () => {
-            cache.pausedMediaElements.forEach((el: HTMLMediaElement) => el.play())
+            cache.pausedMediaElements?.forEach((el: HTMLMediaElement) => el.play())


saw an error from trying to access this when it was undefined while debugging the rest of this

pauldambra · 2024-12-17T20:40:56Z

frontend/src/toolbar/debug/EventDebugMenu.tsx

@@ -167,7 +189,13 @@ export const EventDebugMenu = (): JSX.Element => {
                </div>
            </ToolbarMenu.Body>
            <ToolbarMenu.Footer noPadding>
-                <SettingsBar border="top" className="justify-end">
+                <SettingsBar border="top" className="justify-between">
+                    <SettingsMenu


found myself needing this while figuring out the rest of this

pauldambra · 2024-12-17T20:42:34Z

frontend/src/lib/utils/eventUsageLogic.ts

@@ -441,8 +441,6 @@ export const eventUsageLogic = kea<eventUsageLogicType>([
        reportRecordingsListPropertiesFetched: (loadTime: number) => ({ loadTime }),
        reportRecordingsListFilterAdded: (filterType: SessionRecordingFilterType) => ({ filterType }),
        reportRecordingPlayerSeekbarEventHovered: true,
-        reportRecordingPlayerSpeedChanged: (newSpeed: number) => ({ newSpeed }),


i hate this gigantic logic that just hides calling posthog capture

pauldambra · 2024-12-17T20:42:51Z

frontend/src/scenes/session-recordings/player/playerSettingsLogic.ts

@@ -122,4 +123,13 @@ export const playerSettingsLogic = kea<playerSettingsLogicType>([
            (preferredSidebarStacking) => preferredSidebarStacking === SessionRecordingSidebarStacking.Vertical,
        ],
    }),
+
+    listeners({
+        setSpeed: ({ speed }) => {


moved these captures closer to the source

pauldambra · 2024-12-17T20:43:39Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

@@ -848,9 +842,10 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
        startScrub: () => {
            actions.stopAnimation()
        },
-        setSpeed: ({ speed }) => {
-            actions.reportRecordingPlayerSpeedChanged(speed)


there are always three instances of the player logic, all three are listening to the player settings logic, so all three were reporting this making usage look higher than it is... luckily not an important metric

pauldambra · 2024-12-17T20:44:06Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

@@ -1143,6 +1138,16 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
                actions.reportMessageTooLargeWarningSeen(values.sessionRecordingId)
            }
        },
+        currentPlayerState: (prev, next) => {


pretty sure we're setting this more than we need to, but this at least can report on that too high a number

posthog-bot · 2024-12-17T20:47:44Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-12-18T11:51:26Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-12-18T12:12:04Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra · 2024-12-18T13:35:39Z

frontend/src/scenes/session-recordings/player/PlayerMeta.tsx

@@ -59,26 +59,6 @@ function URLOrScreen({ lastUrl }: { lastUrl: string | undefined }): JSX.Element
    )
 }

-function PlayerWarningsRow(): JSX.Element | null {


we don't set these anywhere any more. so we don't need this code

pauldambra · 2024-12-18T13:37:44Z

frontend/src/scenes/session-recordings/player/controller/PlayerController.tsx

@@ -82,7 +82,7 @@ function ShowMouseTail(): JSX.Element {

    return (
        <SettingsToggle
-            title="Show mouse tail"
+            title="Show a tail following the cursor to make it easier to see"


various changes to ensure there are tooltips on buttons

pauldambra · 2024-12-18T13:38:00Z

frontend/src/scenes/session-recordings/player/controller/PlayerController.tsx

            </div>
+            <InspectDOM />


and to separate the inspect dom button from the player buttons

pauldambra · 2024-12-18T13:38:35Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

+        setPlayerError: (reason: string) => ({ reason }),
+        clearPlayerError: true,


"show" didn't really mean show, so now we store the reason and have a clear action

pauldambra · 2024-12-18T13:39:51Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

-                const isStillLoading = values.isRealtimePolling || values.snapshotsLoading
-                const isPastEnd = values.sessionPlayerData.end && timestamp > values.sessionPlayerData.end.valueOf()
-                if (isStillLoading) {
+                const isPastEnd = values.sessionPlayerData.end && timestamp >= values.sessionPlayerData.end.valueOf()


the logic in here confuses me... and generally when this shows an error it doesn't need to...

let's remove the error state and see if the real reason crops up, then fix the thing we understand

posthog-bot · 2024-12-18T13:47:15Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-12-18T14:07:41Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra · 2024-12-18T14:21:42Z

frontend/src/scenes/session-recordings/player/SessionRecordingPlayer.tsx

-    const lessThanFiveMinutesOld = dayjs().diff(start, 'minute') <= 5
-    const cannotPlayback = snapshotsInvalid && lessThanFiveMinutesOld && !messageTooLargeWarnings
-


giving this a better name

feat: report when player state is error

f4e59f0

github-actions bot added 2 commits December 17, 2024 20:04

Update UI snapshots for chromium (1)

922381a

Update UI snapshots for chromium (2)

0b15784

Update UI snapshots for chromium (1)

eef4c55

pauldambra requested a review from a team December 17, 2024 20:39

pauldambra commented Dec 17, 2024

View reviewed changes

pauldambra added 2 commits December 17, 2024 20:41

stop overreporting player speed changed

447aaf6

don't over report skip inactivity

35f31db

pauldambra commented Dec 17, 2024

View reviewed changes

Update UI snapshots for chromium (1)

a33aa9d

pauldambra added 3 commits December 17, 2024 20:58

more like this maybe

2008d71

don't report an error at the start of a recording

7c9059f

fix

396635b

pauldambra changed the title ~~feat: report when player state is error~~ fix: report when player state is error Dec 17, 2024

fix

65087da

pauldambra enabled auto-merge (squash) December 18, 2024 08:30

pauldambra and others added 3 commits December 18, 2024 11:24

Merge branch 'master' into feat/pd/report-when-player-state-is-error

8c2f730

fix

e8186c9

Update UI snapshots for chromium (2)

572f423

pauldambra disabled auto-merge December 18, 2024 11:54

Update UI snapshots for chromium (2)

1a2d79d

pauldambra added 6 commits December 18, 2024 12:54

Merge branch 'master' into feat/pd/report-when-player-state-is-error

278f656

Add a couple of tooltips

c23abb4

flyby fix and tooltips

e1231ff

flyby fix and tooltips

f66e139

fix test

87b9196

fix

8bc9f36

pauldambra commented Dec 18, 2024

View reviewed changes

marandaneto approved these changes Dec 18, 2024

View reviewed changes

Update UI snapshots for chromium (2)

f470b61

fix

3260f78

pauldambra enabled auto-merge (squash) December 18, 2024 14:00

Update UI snapshots for chromium (2)

4874f20

pauldambra commented Dec 18, 2024

View reviewed changes

pauldambra disabled auto-merge December 18, 2024 15:27

pauldambra merged commit 2f9e896 into master Dec 18, 2024
98 of 99 checks passed

pauldambra deleted the feat/pd/report-when-player-state-is-error branch December 18, 2024 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: report when player state is error #26989

fix: report when player state is error #26989

pauldambra commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

posthog-bot commented Dec 17, 2024

posthog-bot commented Dec 17, 2024

pauldambra Dec 17, 2024

pauldambra Dec 17, 2024

pauldambra Dec 17, 2024

pauldambra Dec 17, 2024

pauldambra Dec 17, 2024

pauldambra Dec 17, 2024

posthog-bot commented Dec 17, 2024

posthog-bot commented Dec 18, 2024

posthog-bot commented Dec 18, 2024

pauldambra Dec 18, 2024

pauldambra Dec 18, 2024

pauldambra Dec 18, 2024

pauldambra Dec 18, 2024

pauldambra Dec 18, 2024

posthog-bot commented Dec 18, 2024

posthog-bot commented Dec 18, 2024

pauldambra Dec 18, 2024

		setPlayerError: (reason: string) => ({ reason }),
		clearPlayerError: true,

		const lessThanFiveMinutesOld = dayjs().diff(start, 'minute') <= 5
		const cannotPlayback = snapshotsInvalid && lessThanFiveMinutesOld && !messageTooLargeWarnings

fix: report when player state is error #26989

fix: report when player state is error #26989

Conversation

pauldambra commented Dec 17, 2024 • edited Loading

github-actions bot commented Dec 17, 2024 • edited Loading

posthog-bot commented Dec 17, 2024

📸 UI snapshots have been updated

posthog-bot commented Dec 17, 2024

📸 UI snapshots have been updated

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

posthog-bot commented Dec 17, 2024

📸 UI snapshots have been updated

posthog-bot commented Dec 18, 2024

📸 UI snapshots have been updated

posthog-bot commented Dec 18, 2024

📸 UI snapshots have been updated

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

posthog-bot commented Dec 18, 2024

📸 UI snapshots have been updated

posthog-bot commented Dec 18, 2024

📸 UI snapshots have been updated

Choose a reason for hiding this comment

pauldambra commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading