Update polling and timeout message handling | CTT-188

README.md

@@ -67,6 +67,38 @@ You need to pass an object containing API endpoint callbacks as the `apiValue` p
 8. Update translations and documentation as needed
 9. Open Pull Request
 
+## Troubleshooting Polling and Websockets
+
+When investigating inconsistent outcomes between websocket updates and polling updates, temporarily add the following logs to compare what each source is emitting and what final polling decision is made.
+
+Remove these logs after troubleshooting.
+
+### 1) Polling transport messages
+
+File: src/utilities/transport/MemberUpdateTransport.ts
+
+Add a `console.log` with a `polling` label where member and job are returned for polling updates
+
+### 2) Websocket transport messages
+
+File: src/utilities/transport/MemberUpdateTransport.ts
+
+Add a `console.log` with a `websocket` label where member and job are returned for websocket updates:
+
+### 3) Final polling decision
+
+File: src/hooks/usePollMember.tsx
+
+Add a `console.log` with a `final polling decision` label after finalState is computed:
+
+### Suggested debugging sequence for difficult bugs
+
+1. Reproduce the flow several times.
+2. Compare websocket and polling event order.
+3. The final polling decision determines what the UI does
+
+Edge cases might exist that haven't been handled. For example, it was discovered that the IMPAIRED status for No DDA flows is set _after_ a member is set to CONNECTED first. In very rare cases, websockets were so fast that it was detecting CONNECTED before seeing IMPAIRED and we handled it as a success, incorrectly.
+
 ## Commit Message Requirements
 
 _To make commits that trigger a package release, use `npx cz`, it will launch easy to follow commitizen prompts._

src/hooks/__tests__/usePollMember-test.tsx

@@ -9,6 +9,7 @@ import { Provider } from 'react-redux'
 import { createReduxStore, RootState } from 'src/redux/Store'
 import { member, JOB_DATA } from 'src/services/mockedData'
 import { ReadableStatuses } from 'src/const/Statuses'
+import { VERIFY_MODE } from 'src/const/Connect'
 import { CONNECTING_MESSAGES } from 'src/utilities/pollers'
 import { take } from 'rxjs/operators'
 import { Subject } from 'rxjs'
@@ -88,7 +89,6 @@ describe('usePollMember', () => {
     expect(apiValue.loadJob).toHaveBeenCalledWith(connectedMember.most_recent_job_guid)
     expect(states[0]).toMatchObject({
       isError: false,
-      pollingCount: 1,
       currentResponse: {
         member: connectedMember,
         job: JOB_DATA,
@@ -128,7 +128,6 @@ describe('usePollMember', () => {
 
     expect(states[0]).toMatchObject({
       isError: true,
-      pollingCount: 1,
       pollingIsDone: false,
     })
 
@@ -314,7 +313,7 @@ describe('usePollMember', () => {
     subscription.unsubscribe()
   })
 
-  it('should increment pollingCount on each poll', async () => {
+  it('should emit sequential states on each poll', async () => {
     const member1 = { ...member.member, guid: 'MBR-1', most_recent_job_guid: 'JOB-1' }
     const member2 = { ...member.member, guid: 'MBR-2', most_recent_job_guid: 'JOB-2' }
 
@@ -349,8 +348,14 @@ describe('usePollMember', () => {
       { timeout: 3500 },
     )
 
-    expect(states[0].pollingCount).toBe(1)
-    expect(states[1].pollingCount).toBe(2)
+    expect(states[0].currentResponse).toEqual({
+      member: member1,
+      job: { ...JOB_DATA, guid: 'JOB-1' },
+    })
+    expect(states[1].currentResponse).toEqual({
+      member: member2,
+      job: { ...JOB_DATA, guid: 'JOB-2' },
+    })
 
     subscription.unsubscribe()
   }, 10000)
@@ -871,4 +876,136 @@ describe('usePollMember', () => {
 
     subscription.unsubscribe()
   })
+
+  it('should wait for second CONNECTED update in verify mode before finishing', async () => {
+    const wsMessages$ = new Subject<any>()
+    const mockWS = {
+      isConnected: vi.fn().mockReturnValue(true),
+      webSocketMessages$: wsMessages$.asObservable(),
+    }
+
+    const apiValue = {
+      loadMemberByGuid: vi.fn().mockResolvedValue(member.member),
+      loadJob: vi.fn().mockResolvedValue(JOB_DATA),
+    }
+
+    const preloadedState = {
+      config: {
+        mode: VERIFY_MODE,
+      },
+      experimentalFeatures: {
+        useWebSockets: true,
+        memberPollingMilliseconds: 10000,
+      },
+    } as Partial<RootState>
+
+    const { result } = renderHook(() => usePollMember(), {
+      wrapper: createWrapper(apiValue, preloadedState, mockWS),
+    })
+
+    const pollMember = result.current
+    const states: PollingState[] = []
+
+    const subscription = pollMember('MBR-123').subscribe((state: PollingState) => {
+      states.push(state)
+    })
+
+    wsMessages$.next({
+      event: 'members/updated',
+      payload: {
+        guid: 'MBR-123',
+        most_recent_job_guid: 'JOB-123',
+        connection_status: ReadableStatuses.UPDATED,
+        is_being_aggregated: false,
+      },
+    })
+
+    await waitFor(() => expect(states.length).toBe(1))
+    expect(states[0].pollingIsDone).toBe(false)
+    expect(states[0].userMessage).toBe(CONNECTING_MESSAGES.VERIFYING)
+
+    wsMessages$.next({
+      event: 'members/updated',
+      payload: {
+        guid: 'MBR-123',
+        most_recent_job_guid: 'JOB-123',
+        connection_status: ReadableStatuses.CONNECTED,
+        is_being_aggregated: false,
+      },
+    })
+
+    await waitFor(() => expect(states.length).toBe(2))
+    expect(states[1].pollingIsDone).toBe(false)
+    expect(states[1].userMessage).toBe(CONNECTING_MESSAGES.VERIFYING)
+
+    wsMessages$.next({
+      event: 'members/updated',
+      payload: {
+        guid: 'MBR-123',
+        most_recent_job_guid: 'JOB-123',
+        connection_status: ReadableStatuses.CONNECTED,
+        is_being_aggregated: false,
+      },
+    })
+
+    await waitFor(() => expect(states.length).toBe(3))
+    expect(states[2].pollingIsDone).toBe(true)
+    expect(states[2].userMessage).toBe(CONNECTING_MESSAGES.FINISHING)
+
+    subscription.unsubscribe()
+  })
+
+  it('should eventually stop polling for an oauth member in an error state that was never aggregating', async () => {
+    /**
+     * This tests a specific bug where distinctUntilChanged in the transport layer
+     * blocks the second identical poll from reaching the scan accumulator.
+     *
+     * handlePollingResponse uses isNotAggregatingAtAll:
+     *   previousMember.is_being_aggregated === false && polledMember.is_being_aggregated === false
+     *
+     * Without the fix, the second poll is blocked, previousResponse stays as {} (DEFAULT),
+     * isNotAggregatingAtAll is never true, and the OAuth member polls forever.
+     */
+    const oauthMember = {
+      ...member.member,
+      connection_status: ReadableStatuses.PREVENTED,
+      is_being_aggregated: false,
+      is_oauth: true,
+    }
+
+    const apiValue = {
+      loadMemberByGuid: vi.fn().mockResolvedValue(oauthMember),
+      loadJob: vi.fn().mockResolvedValue(JOB_DATA),
+    }
+
+    const preloadedState = {
+      experimentalFeatures: {
+        memberPollingMilliseconds: 100,
+      },
+    }
+
+    const { result } = renderHook(() => usePollMember(), {
+      wrapper: createWrapper(apiValue, preloadedState),
+    })
+
+    const pollMember = result.current
+    const states: PollingState[] = []
+
+    const subscription = pollMember('MBR-123').subscribe((state: PollingState) => {
+      states.push(state)
+    })
+
+    await waitFor(
+      () => {
+        expect(states.some((s) => s.pollingIsDone === true)).toBe(true)
+      },
+      { timeout: 1000 },
+    )
+
+    expect(states.find((s) => s.pollingIsDone === true)?.userMessage).toBe(
+      CONNECTING_MESSAGES.ERROR,
+    )
+
+    subscription.unsubscribe()
+  })
 })

src/hooks/usePollMember.tsx

@@ -5,15 +5,16 @@ import { useWebSocket } from 'src/context/WebSocketContext'
 import { useSelector } from 'react-redux'
 import { getExperimentalFeatures } from 'src/redux/reducers/experimentalFeaturesSlice'
 
-import { scan } from 'rxjs/operators'
+import { scan, distinctUntilChanged } from 'rxjs/operators'
+import _isEqual from 'lodash/isEqual'
 import {
   createMemberUpdateTransport,
   MemberUpdate,
 } from 'src/utilities/transport/MemberUpdateTransport'
+import type { RootState } from 'src/redux/Store'
 
 export interface PollingState {
   isError: boolean
-  pollingCount: number
   currentResponse?: MemberUpdate | Record<string, never>
   previousResponse?: MemberUpdate | Record<string, never>
   pollingIsDone: boolean
@@ -31,6 +32,7 @@ export function usePollMember() {
 
   const { optOutOfEarlyUserRelease, memberPollingMilliseconds, useWebSockets } =
     useSelector(getExperimentalFeatures)
+  const mode = useSelector((state: RootState) => state.config?.mode)
 
   const pollingInterval = memberPollingMilliseconds || 3000
 
@@ -61,8 +63,6 @@ export function usePollMember() {
           const pollingState: PollingState = {
             // only track if the most recent poll was an error
             isError,
-            // always increase polling count
-            pollingCount: acc.pollingCount + 1,
             // dont update previous response if this is an error
             previousResponse: isError ? acc.previousResponse : acc.currentResponse,
             // dont update current response if this is an error
@@ -82,17 +82,48 @@ export function usePollMember() {
             pollingState.initialDataReady = true
           }
 
-          const [shouldStopPolling, messageKey] = handlePollingResponse(pollingState)
+          const [shouldStopPolling, messageKey] = handlePollingResponse(pollingState, mode)
 
-          return {
+          const finalState = {
             ...pollingState,
             // we should keep polling based on the member
             pollingIsDone: isError ? false : shouldStopPolling,
             userMessage: messageKey,
           }
+
+          return finalState
         },
         { ...DEFAULT_POLLING_STATE } as PollingState,
       ),
+      // Deduplicate consecutive identical polling states to prevent unnecessary re-renders.
+      // This must live here — after the scan — so the scan always sees every update
+      // and can correctly track previousResponse/currentResponse transitions. Placing
+      // distinctUntilChanged earlier (in the transport) caused the scan to miss
+      // identical consecutive polls, breaking isNotAggregatingAtAll detection in
+      // handlePollingResponse and causing OAuth members to poll indefinitely.
+      distinctUntilChanged((prev: PollingState, curr: PollingState) => {
+        if (prev.isError !== curr.isError) return false
+        if (prev.pollingIsDone !== curr.pollingIsDone) return false
+        if (prev.userMessage !== curr.userMessage) return false
+        if (prev.initialDataReady !== curr.initialDataReady) return false
+
+        const prevMember = (prev.currentResponse as MemberUpdate)?.member
+        const currMember = (curr.currentResponse as MemberUpdate)?.member
+        const prevJob = (prev.currentResponse as MemberUpdate)?.job
+        const currJob = (curr.currentResponse as MemberUpdate)?.job
+
+        // Return true to *prevent* emitting the event
+        // Return false to emit the event
+        return (
+          prevMember?.connection_status === currMember?.connection_status &&
+          _isEqual(prevMember?.mfa, currMember?.mfa) &&
+          prevMember?.is_being_aggregated === currMember?.is_being_aggregated &&
+          prevMember?.most_recent_job_detail_code === currMember?.most_recent_job_detail_code &&
+          prevMember?.error?.error_code === currMember?.error?.error_code &&
+          prevJob?.guid === currJob?.guid &&
+          prevJob?.async_account_data_ready === currJob?.async_account_data_ready
+        )
+      }),
     )
   }