fix: fix time conrols on timeline when eventID is set (#438)

Fixes #436 Also enforces use of `AbortController` for connect requests to avoid hangs
block · Oct 4, 2023 · b724ff9 · b724ff9
1 parent 1d4c4d9
commit b724ff9
Show file tree

Hide file tree

Showing 9 changed files with 120 additions and 44 deletions.
diff --git a/console/client/src/features/modules/ModulePage.tsx b/console/client/src/features/modules/ModulePage.tsx
@@ -23,13 +23,18 @@ export const ModulePage = () => {
   }, [modules, moduleName])
 
   React.useEffect(() => {
+    const abortController = new AbortController()
     if (!module) return
 
     const fetchCalls = async () => {
-      const calls = await getCalls(module.name)
+      const calls = await getCalls({ abortControllerSignal: abortController.signal, destModule: module.name })
       setCalls(calls)
     }
     fetchCalls()
+
+    return () => {
+      abortController.abort()
+    }
   }, [module])
 
   return (

diff --git a/console/client/src/features/timeline/Timeline.tsx b/console/client/src/features/timeline/Timeline.tsx
@@ -33,7 +33,6 @@ export const Timeline = ({ timeSettings, filters }: Props) => {
   React.useEffect(() => {
     const eventId = searchParams.get('id')
     const abortController = new AbortController()
-    abortController.signal
 
     const fetchEvents = async () => {
       let eventFilters = filters
@@ -45,7 +44,7 @@ export const Timeline = ({ timeSettings, filters }: Props) => {
         const id = BigInt(eventId)
         eventFilters = [eventIdFilter({ higherThan: id }), ...filters]
       }
-      const events = await getEvents({ filters: eventFilters })
+      const events = await getEvents({ abortControllerSignal: abortController.signal, filters: eventFilters })
       setEntries(events)
 
       if (eventId) {
@@ -81,28 +80,34 @@ export const Timeline = ({ timeSettings, filters }: Props) => {
     }
   }, [isOpen])
 
+  const handlePanelClosed = () => {
+    const newParams = new URLSearchParams(searchParams.toString())
+    newParams.delete('id')
+    setSearchParams(newParams)
+    setSelectedEntry(null)
+  }
+
   const handleEntryClicked = (entry: Event) => {
     if (selectedEntry === entry) {
-      setSelectedEntry(null)
       closePanel()
-      const newParams = new URLSearchParams(searchParams.toString())
-      newParams.delete('id')
-      setSearchParams(newParams)
       return
     }
 
     switch (entry.entry?.case) {
       case 'call':
-        openPanel(<TimelineCallDetails timestamp={entry.timeStamp as Timestamp} call={entry.entry.value} />)
+        openPanel(
+          <TimelineCallDetails timestamp={entry.timeStamp as Timestamp} call={entry.entry.value} />,
+          handlePanelClosed,
+        )
         break
       case 'log':
-        openPanel(<TimelineLogDetails event={entry} log={entry.entry.value} />)
+        openPanel(<TimelineLogDetails event={entry} log={entry.entry.value} />, handlePanelClosed)
         break
       case 'deploymentCreated':
-        openPanel(<TimelineDeploymentCreatedDetails event={entry} deployment={entry.entry.value} />)
+        openPanel(<TimelineDeploymentCreatedDetails event={entry} deployment={entry.entry.value} />, handlePanelClosed)
         break
       case 'deploymentUpdated':
-        openPanel(<TimelineDeploymentUpdatedDetails event={entry} deployment={entry.entry.value} />)
+        openPanel(<TimelineDeploymentUpdatedDetails event={entry} deployment={entry.entry.value} />, handlePanelClosed)
         break
       default:
         break

diff --git a/console/client/src/features/timeline/details/TimelineCallDetails.tsx b/console/client/src/features/timeline/details/TimelineCallDetails.tsx
@@ -29,15 +29,23 @@ export const TimelineCallDetails = ({ timestamp, call }: Props) => {
   }, [call])
 
   useEffect(() => {
+    const abortController = new AbortController()
     const fetchRequestCalls = async () => {
       if (selectedCall.requestName === undefined) {
         return
       }
-      const calls = await getRequestCalls(selectedCall.requestName)
+      const calls = await getRequestCalls({
+        abortControllerSignal: abortController.signal,
+        requestKey: selectedCall.requestName,
+      })
       setRequestCalls(calls.reverse())
     }
 
     fetchRequestCalls()
+
+    return () => {
+      abortController.abort()
+    }
   }, [client, selectedCall])
 
   return (

diff --git a/console/client/src/features/timeline/filters/TimelineTimeControls.tsx b/console/client/src/features/timeline/filters/TimelineTimeControls.tsx
@@ -54,7 +54,7 @@ export const TimelineTimeControls = ({ onTimeSettingsChange, selectedTimeRange,
   const isTailing = selected.value === TIME_RANGES['tail'].value
 
   React.useEffect(() => {
-    setSelected(selectedTimeRange)
+    handleRangeChanged(selectedTimeRange)
     setIsPaused(isTimelinePaused)
   }, [selectedTimeRange, isTimelinePaused])
 

diff --git a/console/client/src/features/verbs/VerbCalls.tsx b/console/client/src/features/verbs/VerbCalls.tsx
@@ -19,16 +19,25 @@ export const VerbCalls = ({ module, verb }: Props) => {
   const { openPanel } = React.useContext(SidePanelContext)
 
   React.useEffect(() => {
+    const abortController = new AbortController()
     const fetchCalls = async () => {
       if (module === undefined) {
         return
       }
 
-      const calls = await getCalls(module.name, verb?.verb?.name)
+      const calls = await getCalls({
+        abortControllerSignal: abortController.signal,
+        destModule: module.name,
+        destVerb: verb?.verb?.name,
+      })
       setCalls(calls)
     }
 
     fetchCalls()
+
+    return () => {
+      abortController.abort()
+    }
   }, [client, module, verb])
 
   const handleClick = (call: CallEvent) => {

diff --git a/console/client/src/features/verbs/VerbPage.tsx b/console/client/src/features/verbs/VerbPage.tsx
@@ -30,13 +30,22 @@ export const VerbPage = () => {
   }, [modules, moduleName])
 
   React.useEffect(() => {
+    const abortController = new AbortController()
     if (!module) return
 
     const fetchCalls = async () => {
-      const calls = await getCalls(module.name, verb?.verb?.name)
+      const calls = await getCalls({
+        abortControllerSignal: abortController.signal,
+        destModule: module.name,
+        destVerb: verb?.verb?.name,
+      })
       setCalls(calls)
     }
     fetchCalls()
+
+    return () => {
+      abortController.abort()
+    }
   }, [module])
 
   return (

diff --git a/console/client/src/providers/modules-provider.tsx b/console/client/src/providers/modules-provider.tsx
@@ -1,3 +1,4 @@
+import { Code, ConnectError } from '@bufbuild/connect'
 import { PropsWithChildren, createContext, useContext, useEffect, useState } from 'react'
 import { useClient } from '../hooks/use-client'
 import { ConsoleService } from '../protos/xyz/block/ftl/v1/console/console_connect'
@@ -12,14 +13,28 @@ export const ModulesProvider = (props: PropsWithChildren) => {
   const [modules, setModules] = useState<GetModulesResponse>(new GetModulesResponse())
 
   useEffect(() => {
+    const abortController = new AbortController()
     const fetchModules = async () => {
-      const modules = await client.getModules({})
-      setModules(modules ?? [])
+      try {
+        const modules = await client.getModules({}, { signal: abortController.signal })
+        setModules(modules ?? [])
+      } catch (error) {
+        if (error instanceof ConnectError) {
+          if (error.code !== Code.Canceled) {
+            console.error('ModulesProvider - Connect error:', error)
+          }
+        } else {
+          console.error('ModulesProvider:', error)
+        }
+      }
 
       return
     }
 
     fetchModules()
+    return () => {
+      abortController.abort()
+    }
   }, [client, schema])
 
   return <modulesContext.Provider value={modules}>{props.children}</modulesContext.Provider>

diff --git a/console/client/src/providers/side-panel-provider.tsx b/console/client/src/providers/side-panel-provider.tsx
@@ -3,7 +3,7 @@ import React, { PropsWithChildren } from 'react'
 interface SidePanelContextType {
   isOpen: boolean
   component: React.ReactNode
-  openPanel: (component: React.ReactNode) => void
+  openPanel: (component: React.ReactNode, onClose?: () => void) => void
   closePanel: () => void
 }
 
@@ -19,16 +19,24 @@ export const SidePanelContext = React.createContext<SidePanelContextType>(defaul
 export const SidePanelProvider = ({ children }: PropsWithChildren) => {
   const [isOpen, setIsOpen] = React.useState(false)
   const [component, setComponent] = React.useState<React.ReactNode>()
+  const [onCloseCallback, setOnCloseCallback] = React.useState<(() => void) | null>(null)
 
-  const openPanel = (comp: React.ReactNode) => {
+  const openPanel = React.useCallback((comp: React.ReactNode, onClose?: () => void) => {
     setIsOpen(true)
     setComponent(comp)
-  }
+    if (onClose) {
+      setOnCloseCallback(() => onClose)
+    }
+  }, [])
 
-  const closePanel = () => {
+  const closePanel = React.useCallback(() => {
     setIsOpen(false)
     setComponent(undefined)
-  }
+    if (onCloseCallback) {
+      onCloseCallback()
+    }
+    setOnCloseCallback(null)
+  }, [onCloseCallback])
 
   return (
     <SidePanelContext.Provider value={{ isOpen, openPanel, closePanel, component }}>

diff --git a/console/client/src/services/console.service.ts b/console/client/src/services/console.service.ts
@@ -93,12 +93,13 @@ export const timeFilter = (olderThan: Timestamp | undefined, newerThan: Timestam
   return filter
 }
 
-interface IDFilterParams {
+export const eventIdFilter = ({
+  lowerThan,
+  higherThan,
+}: {
   lowerThan?: bigint
   higherThan?: bigint
-}
-
-export const eventIdFilter = ({ lowerThan, higherThan }: IDFilterParams): EventsQuery_Filter => {
+}): EventsQuery_Filter => {
   const filter = new EventsQuery_Filter()
   const idFilter = new EventsQuery_IDFilter()
   idFilter.lowerThan = lowerThan
@@ -110,46 +111,62 @@ export const eventIdFilter = ({ lowerThan, higherThan }: IDFilterParams): Events
   return filter
 }
 
-export const getRequestCalls = async (requestKey: string): Promise<CallEvent[]> => {
+export const getRequestCalls = async ({
+  abortControllerSignal,
+  requestKey,
+}: {
+  abortControllerSignal: AbortSignal
+  requestKey: string
+}): Promise<CallEvent[]> => {
   const allEvents = await getEvents({
+    abortControllerSignal,
     filters: [requestKeysFilter([requestKey]), eventTypesFilter([EventType.CALL])],
   })
   return allEvents.map((e) => e.entry.value) as CallEvent[]
 }
 
-export const getCalls = async (
-  destModule: string,
-  destVerb: string | undefined = undefined,
-  sourceModule: string | undefined = undefined,
-): Promise<CallEvent[]> => {
+export const getCalls = async ({
+  abortControllerSignal,
+  destModule,
+  destVerb,
+  sourceModule,
+}: {
+  abortControllerSignal: AbortSignal
+  destModule: string
+  destVerb?: string
+  sourceModule?: string
+}): Promise<CallEvent[]> => {
   const allEvents = await getEvents({
+    abortControllerSignal,
     filters: [callFilter(destModule, destVerb, sourceModule), eventTypesFilter([EventType.CALL])],
   })
   return allEvents.map((e) => e.entry.value) as CallEvent[]
 }
 
-interface GetEventsParams {
-  limit?: number
-  order?: EventsQuery_Order
-  filters?: EventsQuery_Filter[]
-}
-
 export const getEvents = async ({
+  abortControllerSignal,
   limit = 1000,
   order = EventsQuery_Order.DESC,
   filters = [],
-}: GetEventsParams): Promise<Event[]> => {
-  const response = await client.getEvents({ filters, limit, order })
+}: {
+  abortControllerSignal: AbortSignal
+  limit?: number
+  order?: EventsQuery_Order
+  filters?: EventsQuery_Filter[]
+}): Promise<Event[]> => {
+  const response = await client.getEvents({ filters, limit, order }, { signal: abortControllerSignal })
   return response.events
 }
 
-export interface StreamEventsParams {
+export const streamEvents = async ({
+  abortControllerSignal,
+  filters,
+  onEventReceived,
+}: {
   abortControllerSignal: AbortSignal
   filters: EventsQuery_Filter[]
   onEventReceived: (event: Event) => void
-}
-
-export const streamEvents = async ({ abortControllerSignal, filters, onEventReceived }: StreamEventsParams) => {
+}) => {
   try {
     for await (const response of client.streamEvents(
       { updateInterval: { seconds: BigInt(1) }, query: { limit: 1000, filters } },