From 39d2ede29554e5fd445ad4a4534850fa607f8e6f Mon Sep 17 00:00:00 2001 From: devthejo Date: Sun, 11 Jan 2026 15:17:55 +0100 Subject: [PATCH] fix: reload + improve subscriptions --- src/hooks/useLatestWithSubscription.js | 154 ++++++++++++-------- src/hooks/useStreamQueryWithSubscription.js | 147 ++++++++++++------- src/network/NetworkProviders.js | 16 ++ src/network/errorLink.js | 29 +++- src/stores/auth.js | 4 +- src/stores/tree.js | 30 +++- 6 files changed, 262 insertions(+), 118 deletions(-) diff --git a/src/hooks/useLatestWithSubscription.js b/src/hooks/useLatestWithSubscription.js index e0c2574..90a449e 100644 --- a/src/hooks/useLatestWithSubscription.js +++ b/src/hooks/useLatestWithSubscription.js @@ -2,8 +2,15 @@ import { useRef, useEffect, useMemo, useState } from "react"; import { useQuery } from "@apollo/client"; import * as Sentry from "@sentry/react-native"; import { useNetworkState } from "~/stores"; +import { createLogger } from "~/lib/logger"; +import { UI_SCOPES } from "~/lib/logger/scopes"; import useShallowMemo from "./useShallowMemo"; +const hookLogger = createLogger({ + module: UI_SCOPES.HOOKS, + feature: "useLatestWithSubscription", +}); + // Constants for retry configuration const MAX_RETRIES = 5; const INITIAL_BACKOFF_MS = 1000; // 1 second @@ -47,13 +54,15 @@ export default function useLatestWithSubscription( const retryCountRef = useRef(0); const subscriptionErrorRef = useRef(null); const timeoutIdRef = useRef(null); + const unsubscribeRef = useRef(null); + const lastWsClosedDateRef = useRef(null); useEffect(() => { const currentVarsHash = JSON.stringify(variables); if (currentVarsHash !== variableHashRef.current) { - console.log( - `[${subscriptionKey}] Variables changed, resetting subscription setup`, - ); + hookLogger.debug("Variables changed; resetting subscription setup", { + subscriptionKey, + }); highestIdRef.current = null; variableHashRef.current = currentVarsHash; initialSetupDoneRef.current = false; @@ -98,19 +107,19 @@ export default function useLatestWithSubscription( (highestIdRef.current === null || highestId > highestIdRef.current) ) { highestIdRef.current = highestId; - console.log( - `[${subscriptionKey}] Updated subscription cursor to highest ID:`, + hookLogger.debug("Updated subscription cursor to highest ID", { + subscriptionKey, highestId, - ); + }); } } else { // Handle empty results case - initialize with 0 to allow subscription for first item if (highestIdRef.current === null) { highestIdRef.current = 0; - console.log( - `[${subscriptionKey}] No initial items, setting subscription cursor to:`, - 0, - ); + hookLogger.debug("No initial items; setting subscription cursor", { + subscriptionKey, + highestId: 0, + }); } } }, [queryData, cursorKey, subscriptionKey]); @@ -134,12 +143,20 @@ export default function useLatestWithSubscription( if (!subscribeToMore) return; if (highestIdRef.current === null) return; // Wait until we have the highest ID + // Track WS close events so we only react when wsClosedDate actually changes + const wsClosedDateChanged = + !!wsClosedDate && wsClosedDate !== lastWsClosedDateRef.current; + if (wsClosedDateChanged) { + lastWsClosedDateRef.current = wsClosedDate; + } + // Check if max retries reached and we have an error if (retryCountRef.current >= maxRetries && subscriptionErrorRef.current) { - console.error( - `[${subscriptionKey}] Max retries (${maxRetries}) reached. Stopping subscription attempts.`, - subscriptionErrorRef.current, - ); + hookLogger.error("Max retries reached; stopping subscription attempts", { + subscriptionKey, + maxRetries, + error: subscriptionErrorRef.current, + }); // Report to Sentry when max retries are reached try { @@ -155,17 +172,24 @@ export default function useLatestWithSubscription( }, }); } catch (sentryError) { - console.error("Failed to report to Sentry:", sentryError); + hookLogger.error("Failed to report max-retries to Sentry", { + subscriptionKey, + error: sentryError, + }); } return; } // Wait for: - // - either initial setup not done yet - // - or a new wsClosedDate (WS reconnect) - // - or a retry trigger - if (initialSetupDoneRef.current && !wsClosedDate && retryTrigger === 0) { + // - initial setup not done yet + // - OR a new wsClosedDate (WS reconnect) + // - OR a retry trigger + if ( + initialSetupDoneRef.current && + !wsClosedDateChanged && + retryTrigger === 0 + ) { return; } @@ -178,6 +202,16 @@ export default function useLatestWithSubscription( timeoutIdRef.current = null; } + // Always cleanup any existing subscription before creating a new one + if (unsubscribeRef.current) { + try { + unsubscribeRef.current(); + } catch (_error) { + // ignore + } + unsubscribeRef.current = null; + } + // Calculate backoff delay if this is a retry const backoffDelay = retryCountRef.current > 0 @@ -187,15 +221,13 @@ export default function useLatestWithSubscription( ) : 0; - const retryMessage = - retryCountRef.current > 0 - ? ` Retry attempt ${retryCountRef.current}/${maxRetries} after ${backoffDelay}ms delay` - : ""; - - console.log( - `[${subscriptionKey}] Setting up subscription${retryMessage} with highestId:`, - highestIdRef.current, - ); + hookLogger.debug("Setting up subscription", { + subscriptionKey, + retryCount: retryCountRef.current, + maxRetries, + backoffDelay, + highestId: highestIdRef.current, + }); // Use timeout for backoff timeoutIdRef.current = setTimeout(() => { @@ -222,10 +254,12 @@ export default function useLatestWithSubscription( maxRetries, ); - console.error( - `[${subscriptionKey}] Subscription error (attempt ${retryCountRef.current}/${maxRetries}):`, + hookLogger.warn("Subscription error", { + subscriptionKey, + attempt: retryCountRef.current, + maxRetries, error, - ); + }); // If we haven't reached max retries, trigger a retry if (retryCountRef.current < maxRetries) { @@ -270,10 +304,11 @@ export default function useLatestWithSubscription( } }); - console.log( - `[${subscriptionKey}] Received ${filteredNewItems.length} new items, updated highestId:`, - highestIdRef.current, - ); + hookLogger.debug("Received new items", { + subscriptionKey, + receivedCount: filteredNewItems.length, + highestId: highestIdRef.current, + }); // For latest items pattern, we prepend new items (DESC order in UI) return { @@ -283,30 +318,27 @@ export default function useLatestWithSubscription( }, }); - // Cleanup on unmount or re-run - return () => { - console.log(`[${subscriptionKey}] Cleaning up subscription`); - if (timeoutIdRef.current) { - clearTimeout(timeoutIdRef.current); - timeoutIdRef.current = null; - } - unsubscribe(); - }; + // Save unsubscribe for cleanup on reruns/unmount + unsubscribeRef.current = unsubscribe; + + // Note: cleanup is handled by the effect cleanup below. } catch (error) { // Handle setup errors (like malformed queries) - console.error( - `[${subscriptionKey}] Error setting up subscription:`, + hookLogger.error("Error setting up subscription", { + subscriptionKey, error, - ); + }); subscriptionErrorRef.current = error; // Increment retry counter but don't exceed maxRetries retryCountRef.current = Math.min(retryCountRef.current + 1, maxRetries); - console.error( - `[${subscriptionKey}] Subscription setup error (attempt ${retryCountRef.current}/${maxRetries}):`, + hookLogger.warn("Subscription setup error", { + subscriptionKey, + attempt: retryCountRef.current, + maxRetries, error, - ); + }); // If we haven't reached max retries, trigger a retry if (retryCountRef.current < maxRetries) { @@ -328,16 +360,14 @@ export default function useLatestWithSubscription( }, }); } catch (sentryError) { - console.error("Failed to report to Sentry:", sentryError); + hookLogger.error("Failed to report setup error to Sentry", { + subscriptionKey, + error: sentryError, + }); } } - return () => { - if (timeoutIdRef.current) { - clearTimeout(timeoutIdRef.current); - timeoutIdRef.current = null; - } - }; + // Cleanup is handled by the effect cleanup below. } }, backoffDelay); @@ -347,6 +377,16 @@ export default function useLatestWithSubscription( clearTimeout(timeoutIdRef.current); timeoutIdRef.current = null; } + + if (unsubscribeRef.current) { + try { + hookLogger.debug("Cleaning up subscription", { subscriptionKey }); + unsubscribeRef.current(); + } catch (_error) { + // ignore + } + unsubscribeRef.current = null; + } }; }, [ skip, diff --git a/src/hooks/useStreamQueryWithSubscription.js b/src/hooks/useStreamQueryWithSubscription.js index 9b86fa2..fdff594 100644 --- a/src/hooks/useStreamQueryWithSubscription.js +++ b/src/hooks/useStreamQueryWithSubscription.js @@ -2,8 +2,15 @@ import { useRef, useEffect, useMemo, useState } from "react"; import { useQuery } from "@apollo/client"; import * as Sentry from "@sentry/react-native"; import { useNetworkState } from "~/stores"; +import { createLogger } from "~/lib/logger"; +import { UI_SCOPES } from "~/lib/logger/scopes"; import useShallowMemo from "./useShallowMemo"; +const hookLogger = createLogger({ + module: UI_SCOPES.HOOKS, + feature: "useStreamQueryWithSubscription", +}); + // Constants for retry configuration const MAX_RETRIES = 5; const INITIAL_BACKOFF_MS = 1000; // 1 second @@ -40,14 +47,16 @@ export default function useStreamQueryWithSubscription( const retryCountRef = useRef(0); const subscriptionErrorRef = useRef(null); const timeoutIdRef = useRef(null); + const unsubscribeRef = useRef(null); + const lastWsClosedDateRef = useRef(null); useEffect(() => { const currentVarsHash = JSON.stringify(variables); if (currentVarsHash !== variableHashRef.current) { - console.log( - `[${subscriptionKey}] Variables changed, resetting cursor to initial value:`, + hookLogger.debug("Variables changed; resetting cursor", { + subscriptionKey, initialCursor, - ); + }); lastCursorRef.current = initialCursor; variableHashRef.current = currentVarsHash; initialSetupDoneRef.current = false; @@ -99,10 +108,10 @@ export default function useStreamQueryWithSubscription( const newCursor = lastItem[cursorKey]; lastCursorRef.current = newCursor; - console.log( - `[${subscriptionKey}] Updated subscription cursor:`, - newCursor, - ); + hookLogger.debug("Updated subscription cursor", { + subscriptionKey, + cursor: newCursor, + }); } }, [queryData, cursorKey, subscriptionKey]); @@ -124,12 +133,20 @@ export default function useStreamQueryWithSubscription( if (skip) return; // If skipping, do nothing if (!subscribeToMore) return; + // Track WS close events so we only react when wsClosedDate actually changes + const wsClosedDateChanged = + !!wsClosedDate && wsClosedDate !== lastWsClosedDateRef.current; + if (wsClosedDateChanged) { + lastWsClosedDateRef.current = wsClosedDate; + } + // Check if max retries reached and we have an error - this check must be done regardless of other conditions if (retryCountRef.current >= maxRetries && subscriptionErrorRef.current) { - console.error( - `[${subscriptionKey}] Max retries (${maxRetries}) reached. Stopping subscription attempts.`, - subscriptionErrorRef.current, - ); + hookLogger.error("Max retries reached; stopping subscription attempts", { + subscriptionKey, + maxRetries, + error: subscriptionErrorRef.current, + }); // Report to Sentry when max retries are reached try { @@ -145,17 +162,24 @@ export default function useStreamQueryWithSubscription( }, }); } catch (sentryError) { - console.error("Failed to report to Sentry:", sentryError); + hookLogger.error("Failed to report max-retries to Sentry", { + subscriptionKey, + error: sentryError, + }); } return; } // Wait for: - // - either initial setup not done yet - // - or a new wsClosedDate (WS reconnect) - // - or a retry trigger - if (initialSetupDoneRef.current && !wsClosedDate && retryTrigger === 0) { + // - initial setup not done yet + // - OR a new wsClosedDate (WS reconnect) + // - OR a retry trigger + if ( + initialSetupDoneRef.current && + !wsClosedDateChanged && + retryTrigger === 0 + ) { return; } @@ -168,6 +192,16 @@ export default function useStreamQueryWithSubscription( timeoutIdRef.current = null; } + // Always cleanup any existing subscription before creating a new one + if (unsubscribeRef.current) { + try { + unsubscribeRef.current(); + } catch (_error) { + // ignore + } + unsubscribeRef.current = null; + } + // Calculate backoff delay if this is a retry const backoffDelay = retryCountRef.current > 0 @@ -177,15 +211,13 @@ export default function useStreamQueryWithSubscription( ) : 0; - const retryMessage = - retryCountRef.current > 0 - ? ` Retry attempt ${retryCountRef.current}/${maxRetries} after ${backoffDelay}ms delay` - : ""; - - console.log( - `[${subscriptionKey}] Setting up subscription${retryMessage} with cursor:`, - lastCursorRef.current, - ); + hookLogger.debug("Setting up subscription", { + subscriptionKey, + retryCount: retryCountRef.current, + maxRetries, + backoffDelay, + cursor: lastCursorRef.current, + }); // Use timeout for backoff timeoutIdRef.current = setTimeout(() => { @@ -212,10 +244,12 @@ export default function useStreamQueryWithSubscription( maxRetries, ); - console.error( - `[${subscriptionKey}] Subscription error (attempt ${retryCountRef.current}/${maxRetries}):`, + hookLogger.warn("Subscription error", { + subscriptionKey, + attempt: retryCountRef.current, + maxRetries, error, - ); + }); // If we haven't reached max retries, trigger a retry if (retryCountRef.current < maxRetries) { @@ -263,10 +297,10 @@ export default function useStreamQueryWithSubscription( newItemCursor > lastCursorRef.current ) { lastCursorRef.current = newItemCursor; - console.log( - `[${subscriptionKey}] New message received with cursor:`, - lastCursorRef.current, - ); + hookLogger.debug("Received item; cursor advanced", { + subscriptionKey, + cursor: lastCursorRef.current, + }); } const existing = itemMap.get(item[uniqKey]); @@ -289,30 +323,27 @@ export default function useStreamQueryWithSubscription( }, }); - // Cleanup on unmount or re-run - return () => { - console.log(`[${subscriptionKey}] Cleaning up subscription`); - if (timeoutIdRef.current) { - clearTimeout(timeoutIdRef.current); - timeoutIdRef.current = null; - } - unsubscribe(); - }; + // Save unsubscribe for cleanup on reruns/unmount + unsubscribeRef.current = unsubscribe; + + // Note: cleanup is handled by the effect cleanup below. } catch (error) { // Handle setup errors (like malformed queries) - console.error( - `[${subscriptionKey}] Error setting up subscription:`, + hookLogger.error("Error setting up subscription", { + subscriptionKey, error, - ); + }); subscriptionErrorRef.current = error; // Increment retry counter but don't exceed maxRetries retryCountRef.current = Math.min(retryCountRef.current + 1, maxRetries); - console.error( - `[${subscriptionKey}] Subscription setup error (attempt ${retryCountRef.current}/${maxRetries}):`, + hookLogger.warn("Subscription setup error", { + subscriptionKey, + attempt: retryCountRef.current, + maxRetries, error, - ); + }); // If we haven't reached max retries, trigger a retry if (retryCountRef.current < maxRetries) { @@ -334,16 +365,14 @@ export default function useStreamQueryWithSubscription( }, }); } catch (sentryError) { - console.error("Failed to report to Sentry:", sentryError); + hookLogger.error("Failed to report setup error to Sentry", { + subscriptionKey, + error: sentryError, + }); } } - return () => { - if (timeoutIdRef.current) { - clearTimeout(timeoutIdRef.current); - timeoutIdRef.current = null; - } - }; + // Cleanup is handled by the effect cleanup below. } }, backoffDelay); @@ -353,6 +382,16 @@ export default function useStreamQueryWithSubscription( clearTimeout(timeoutIdRef.current); timeoutIdRef.current = null; } + + if (unsubscribeRef.current) { + try { + hookLogger.debug("Cleaning up subscription", { subscriptionKey }); + unsubscribeRef.current(); + } catch (_error) { + // ignore + } + unsubscribeRef.current = null; + } }; }, [ skip, diff --git a/src/network/NetworkProviders.js b/src/network/NetworkProviders.js index 7fdc06e..9bd701e 100644 --- a/src/network/NetworkProviders.js +++ b/src/network/NetworkProviders.js @@ -18,8 +18,16 @@ import * as store from "~/stores"; import getRetryMaxAttempts from "./getRetryMaxAttemps"; +import { createLogger } from "~/lib/logger"; +import { NETWORK_SCOPES } from "~/lib/logger/scopes"; + const { useNetworkState, networkActions } = store; +const networkProvidersLogger = createLogger({ + module: NETWORK_SCOPES.APOLLO, + feature: "NetworkProviders", +}); + const initializeNewApolloClient = (reload) => { if (reload) { const { apolloClient } = network; @@ -47,6 +55,10 @@ export default function NetworkProviders({ children }) { const networkState = useNetworkState(["initialized", "triggerReload"]); useEffect(() => { if (networkState.triggerReload) { + networkProvidersLogger.debug("Network triggerReload received", { + reloadId: store.getAuthState()?.reloadId, + hasUserToken: !!store.getAuthState()?.userToken, + }); initializeNewApolloClient(true); setKey((prevKey) => prevKey + 1); } @@ -54,6 +66,10 @@ export default function NetworkProviders({ children }) { useEffect(() => { if (key > 0) { + networkProvidersLogger.debug("Network reloaded", { + reloadId: store.getAuthState()?.reloadId, + hasUserToken: !!store.getAuthState()?.userToken, + }); networkActions.onReload(); } }, [key]); diff --git a/src/network/errorLink.js b/src/network/errorLink.js index 0384566..1ca963c 100644 --- a/src/network/errorLink.js +++ b/src/network/errorLink.js @@ -7,6 +7,8 @@ import { NETWORK_SCOPES } from "~/lib/logger/scopes"; import getStatusCode from "./getStatusCode"; import isAbortError from "./isAbortError"; +import { getSessionState } from "~/stores"; + let pendingRequests = []; const resolvePendingRequests = () => { @@ -127,10 +129,31 @@ export default function createErrorLink({ store }) { // Capture all other errors in Sentry const errorMessage = `apollo error: ${getErrorMessage(error)}`; - Sentry.captureException(new Error(errorMessage), { - extra: { - errorObject: error, + + const authState = getAuthState(); + const sessionState = getSessionState() || {}; + + // Keep Sentry context useful but avoid high-volume/PII payloads. + // - Don't attach the raw Apollo error object (can contain request details) + // - Don't attach identifiers (userId/deviceId) + // - Keep role info since it's relevant to the incident class + const safeExtras = { + operationName: operation.operationName, + statusCode, + reloadId: authState?.reloadId, + hasUserToken: !!authState?.userToken, + authLoading: !!authState?.loading, + session: { + initialized: !!sessionState.initialized, + defaultRole: sessionState.defaultRole, + allowedRolesCount: Array.isArray(sessionState.allowedRoles) + ? sessionState.allowedRoles.length + : 0, }, + }; + + Sentry.captureException(new Error(errorMessage), { + extra: safeExtras, }); } }); diff --git a/src/stores/auth.js b/src/stores/auth.js index 1739ac2..654cef5 100644 --- a/src/stores/auth.js +++ b/src/stores/auth.js @@ -202,6 +202,7 @@ export default createAtom(({ get, merge, getActions }) => { }; const confirmLoginRequest = async ({ authTokenJwt, isConnected }) => { authLogger.info("Confirming login request", { isConnected }); + const reloadId = Date.now(); if (!isConnected) { // backup anon tokens const [anonAuthToken, anonUserToken] = await Promise.all([ @@ -213,7 +214,7 @@ export default createAtom(({ get, merge, getActions }) => { secureStore.setItemAsync(STORAGE_KEYS.ANON_USER_TOKEN, anonUserToken), ]); } - merge({ onReloadAuthToken: authTokenJwt }); + merge({ onReloadAuthToken: authTokenJwt, reloadId }); triggerReload(); }; @@ -308,6 +309,7 @@ export default createAtom(({ get, merge, getActions }) => { initialized: false, onReload: false, onReloadAuthToken: null, + reloadId: null, userOffMode: false, isReloading: false, lastReloadTime: 0, diff --git a/src/stores/tree.js b/src/stores/tree.js index e5f2058..1d84313 100644 --- a/src/stores/tree.js +++ b/src/stores/tree.js @@ -1,5 +1,13 @@ import { createAtom } from "~/lib/atomic-zustand"; +import { createLogger } from "~/lib/logger"; +import { SYSTEM_SCOPES } from "~/lib/logger/scopes"; + +const treeLogger = createLogger({ + module: SYSTEM_SCOPES.APP, + feature: "tree-reload", +}); + const reloadCallbacks = []; export default createAtom(({ merge, getActions }) => { @@ -24,12 +32,14 @@ export default createAtom(({ merge, getActions }) => { if (callback) { reloadCallbacks.push(callback); } - networkActions.triggerReload(); + // Clear session/store state first to stop user-level queries/subscriptions + // while we swap identity tokens. sessionActions.clear(); resetStores(); merge({ triggerReload: true, - suspend: false, + // Keep the tree suspended until we've run reload callbacks. + suspend: true, }); }; @@ -37,12 +47,26 @@ export default createAtom(({ merge, getActions }) => { merge({ triggerReload: false, }); + + // Run all reload callbacks sequentially and await them. + // This ensures auth identity swap completes BEFORE the network layer is recreated. while (reloadCallbacks.length > 0) { let callback = reloadCallbacks.shift(); if (callback) { - callback(); + try { + await Promise.resolve(callback()); + } catch (error) { + treeLogger.error("Reload callback threw", { + error: error?.message, + }); + } } } + + networkActions.triggerReload(); + + // Allow tree to render again; NetworkProviders will show its loader until ready. + merge({ suspend: false }); }; const suspendTree = () => {