From 4919410ff0194de88cef2be9ed791b2931f95e76 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 08:03:22 -0500 Subject: [PATCH 1/2] Don't reset analytics ID when leaving calls We shouldn't be calling PosthogAnalytics.instance.logout() when leaving the call in widget mode, because all this does is reset your analytics ID. In the big picture this is probably inflating our user count metrics. --- src/rtcSessionHelpers.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 719af998..30809730 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -156,7 +156,6 @@ const widgetPostHangupProcedure = async ( logger.error("Failed to send close action", e); } widget.api.transport.stop(); - PosthogAnalytics.instance.logout(); } }; From 65304473df0ea84d2ea01090f104a52e69ddbd51 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 08:52:31 -0500 Subject: [PATCH 2/2] Enable analytics only while authenticated The one place where we should log out of PostHog and reset our analytics ID is when the user is logging out. This matches the behavior in Element Web and makes sense, I think, because logging out is essentially a request for the app to forget who you are. This means we should also start analytics at the point of logging in / reauthenticating. I noticed while making this change that there was an unused branch in setClient, so I cleaned it up rather than making myself update it. --- src/ClientContext.tsx | 42 +++++++++++-------------- src/analytics/PosthogAnalytics.ts | 9 ++++-- src/auth/LoginPage.tsx | 2 +- src/auth/RegisterPage.tsx | 2 +- src/auth/useRegisterPasswordlessUser.ts | 2 +- src/home/UnauthenticatedView.tsx | 4 +-- 6 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 7d7542a0..e13d78a4 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -50,7 +50,7 @@ export type ValidClientState = { reactions: boolean; thumbnails: boolean; }; - setClient: (params?: SetClientParams) => void; + setClient: (client: MatrixClient, session: Session) => void; }; export type AuthenticatedClient = { @@ -65,11 +65,6 @@ export type ErrorState = { error: Error; }; -export type SetClientParams = { - client: MatrixClient; - session: Session; -}; - const ClientContext = createContext(undefined); export const ClientContextProvider = ClientContext.Provider; @@ -79,7 +74,7 @@ export const useClientState = (): ClientState | undefined => export function useClient(): { client?: MatrixClient; - setClient?: (params?: SetClientParams) => void; + setClient?: (client: MatrixClient, session: Session) => void; } { let client; let setClient; @@ -96,7 +91,7 @@ export function useClient(): { // Plain representation of the `ClientContext` as a helper for old components that expected an object with multiple fields. export function useClientLegacy(): { client?: MatrixClient; - setClient?: (params?: SetClientParams) => void; + setClient?: (client: MatrixClient, session: Session) => void; passwordlessUser: boolean; loading: boolean; authenticated: boolean; @@ -160,7 +155,11 @@ export const ClientProvider: FC = ({ children }) => { initializing.current = true; loadClient() - .then(setInitClientState) + .then((initResult) => { + setInitClientState(initResult); + if (PosthogAnalytics.instance.isEnabled()) + PosthogAnalytics.instance.startListeningToSettingsChanges(); + }) .catch((err) => logger.error(err)) .finally(() => (initializing.current = false)); }, []); @@ -196,24 +195,20 @@ export const ClientProvider: FC = ({ children }) => { ); const setClient = useCallback( - (clientParams?: SetClientParams) => { + (client: MatrixClient, session: Session) => { const oldClient = initClientState?.client; - const newClient = clientParams?.client; - if (oldClient && oldClient !== newClient) { + if (oldClient && oldClient !== client) { oldClient.stopClient(); } - if (clientParams) { - saveSession(clientParams.session); - setInitClientState({ - widgetApi: null, - client: clientParams.client, - passwordlessUser: clientParams.session.passwordlessUser, - }); - } else { - clearSession(); - setInitClientState(null); - } + saveSession(session); + setInitClientState({ + widgetApi: null, + client, + passwordlessUser: session.passwordlessUser, + }); + if (PosthogAnalytics.instance.isEnabled()) + PosthogAnalytics.instance.startListeningToSettingsChanges(); }, [initClientState?.client], ); @@ -229,6 +224,7 @@ export const ClientProvider: FC = ({ children }) => { clearSession(); setInitClientState(null); await navigate("/"); + PosthogAnalytics.instance.logout(); PosthogAnalytics.instance.setRegistrationType(RegistrationType.Guest); }, [navigate, initClientState?.client]); diff --git a/src/analytics/PosthogAnalytics.ts b/src/analytics/PosthogAnalytics.ts index e0e7d9e9..0af25d15 100644 --- a/src/analytics/PosthogAnalytics.ts +++ b/src/analytics/PosthogAnalytics.ts @@ -13,6 +13,7 @@ import posthog, { import { logger } from "matrix-js-sdk/src/logger"; import { type MatrixClient } from "matrix-js-sdk/src/matrix"; import { Buffer } from "buffer"; +import { type Subscription } from "rxjs"; import { widget } from "../widget"; import { @@ -101,6 +102,7 @@ export class PosthogAnalytics { private anonymity = Anonymity.Disabled; private platformSuperProperties = {}; private registrationType: RegistrationType = RegistrationType.Guest; + private optInListener: Subscription | null = null; public static hasInstance(): boolean { return Boolean(this.internalInstance); @@ -146,7 +148,6 @@ export class PosthogAnalytics { ); this.enabled = false; } - this.startListeningToSettingsChanges(); // Triggers maybeIdentifyUser } private sanitizeProperties = ( @@ -328,6 +329,8 @@ export class PosthogAnalytics { if (this.enabled) { this.posthog.reset(); } + this.optInListener?.unsubscribe(); + this.optInListener = null; this.setAnonymity(Anonymity.Disabled); } @@ -406,7 +409,7 @@ export class PosthogAnalytics { } } - private startListeningToSettingsChanges(): void { + public startListeningToSettingsChanges(): void { // Listen to account data changes from sync so we can observe changes to relevant flags and update. // This is called - // * On page load, when the account data is first received by sync @@ -415,7 +418,7 @@ export class PosthogAnalytics { // * When the user changes their preferences on this device // Note that for new accounts, pseudonymousAnalyticsOptIn won't be set, so updateAnonymityFromSettings // won't be called (i.e. this.anonymity will be left as the default, until the setting changes) - optInAnalytics.value$.subscribe((optIn) => { + this.optInListener ??= optInAnalytics.value$.subscribe((optIn) => { this.setAnonymity(optIn ? Anonymity.Pseudonymous : Anonymity.Disabled); this.maybeIdentifyUser().catch(() => logger.log("Could not identify user"), diff --git a/src/auth/LoginPage.tsx b/src/auth/LoginPage.tsx index b3805ef7..da20a86b 100644 --- a/src/auth/LoginPage.tsx +++ b/src/auth/LoginPage.tsx @@ -53,7 +53,7 @@ export const LoginPage: FC = () => { return; } - setClient({ client, session }); + setClient(client, session); const locationState = location.state; // eslint-disable-next-line @typescript-eslint/ban-ts-comment diff --git a/src/auth/RegisterPage.tsx b/src/auth/RegisterPage.tsx index bb2c09a4..b82afd5a 100644 --- a/src/auth/RegisterPage.tsx +++ b/src/auth/RegisterPage.tsx @@ -95,7 +95,7 @@ export const RegisterPage: FC = () => { } } - setClient?.({ client: newClient, session }); + setClient?.(newClient, session); PosthogAnalytics.instance.eventSignup.cacheSignupEnd(new Date()); }; diff --git a/src/auth/useRegisterPasswordlessUser.ts b/src/auth/useRegisterPasswordlessUser.ts index 6dad6ebd..728fe131 100644 --- a/src/auth/useRegisterPasswordlessUser.ts +++ b/src/auth/useRegisterPasswordlessUser.ts @@ -47,7 +47,7 @@ export function useRegisterPasswordlessUser(): UseRegisterPasswordlessUserType { recaptchaResponse, true, ); - setClient({ client, session }); + setClient(client, session); } catch (e) { reset(); throw e; diff --git a/src/home/UnauthenticatedView.tsx b/src/home/UnauthenticatedView.tsx index ced13985..d84d2309 100644 --- a/src/home/UnauthenticatedView.tsx +++ b/src/home/UnauthenticatedView.tsx @@ -89,7 +89,7 @@ export const UnauthenticatedView: FC = () => { // @ts-ignore if (error.errcode === "M_ROOM_IN_USE") { setOnFinished(() => { - setClient({ client, session }); + setClient(client, session); const aliasLocalpart = roomAliasLocalpartFromRoomName(roomName); navigate(`/${aliasLocalpart}`)?.catch((error) => { logger.error("Failed to navigate to alias localpart", error); @@ -111,7 +111,7 @@ export const UnauthenticatedView: FC = () => { if (!createRoomResult.password) throw new Error("Failed to create room with shared secret"); - setClient({ client, session }); + setClient(client, session); await navigate( getRelativeRoomUrl( createRoomResult.roomId,