Merge pull request #2711 from devtron-labs/fix/external-app-polling-cleanup

fix: polling cleanup in external apps

Author: AbhishekA1509

src/Pages/App/Details/ExternalFlux/ExternalFluxAppDetails.tsx

@@ -69,9 +69,12 @@ const ExternalFluxAppDetails = () => {
         setAppDetailsError(null)
     }
 
+    /**
+     * Throws error in case request is aborted
+     */
     const handleFetchExternalFluxCDAppDetails = () =>
         // NOTE: returning a promise so that we can trigger the next timeout after this api call completes
-        new Promise<void>((resolve) => {
+        new Promise<void>((resolve, reject) => {
             setIsReloadResourceTreeInProgress(true)
 
             abortPreviousRequests(
@@ -87,6 +90,8 @@ const ExternalFluxAppDetails = () => {
                         } else {
                             setAppDetailsError(error)
                         }
+                    } else {
+                        reject(error)
                     }
                 })
                 .finally(() => {
@@ -107,7 +112,11 @@ const ExternalFluxAppDetails = () => {
     }
 
     const handleReloadResourceTree = async () => {
-        await handleFetchExternalFluxCDAppDetails()
+        try {
+            await handleFetchExternalFluxCDAppDetails()
+        } catch {
+            // do nothing
+        }
     }
 
     useEffect(() => {

src/components/externalArgoApps/ExternalArgoAppDetail.tsx

@@ -32,14 +32,15 @@ import { AppDetails, AppType } from '../v2/appDetails/appDetails.type'
 import IndexStore from '../v2/appDetails/index.store'
 import { ExternalArgoAppDetailType } from './externalArgoApp.type'
 
+let initTimer = null
+
 const ExternalArgoAppDetail = ({ appName, clusterId, isExternalApp, namespace }: ExternalArgoAppDetailType) => {
     const location = useLocation()
     const history = useHistory()
     const [isLoading, setIsLoading] = useState(true)
     const [isReloadResourceTreeInProgress, setIsReloadResourceTreeInProgress] = useState(false)
     const [errorResponseCode, setErrorResponseCode] = useState(undefined)
 
-    let initTimer = null
     let isAPICallInProgress = false
 
     const abortControllerRef = useRef<AbortController>(new AbortController())
@@ -68,11 +69,21 @@ const ExternalArgoAppDetail = ({ appName, clusterId, isExternalApp, namespace }:
 
     const _init = () => {
         if (!isAPICallInProgress) {
-            _getAndSetAppDetail(true)
+            _getAndSetAppDetail()
         }
     }
 
-    const _getAndSetAppDetail = async (shouldTriggerPolling: boolean = false) => {
+    const handleInitiatePolling = () => {
+        if (initTimer) {
+            clearTimeout(initTimer)
+        }
+
+        initTimer = setTimeout(() => {
+            _init()
+        }, window._env_.EA_APP_DETAILS_POLLING_INTERVAL || 30000)
+    }
+
+    const _getAndSetAppDetail = async () => {
         isAPICallInProgress = true
         setIsReloadResourceTreeInProgress(true)
 
@@ -85,25 +96,25 @@ const ExternalArgoAppDetail = ({ appName, clusterId, isExternalApp, namespace }:
                     ...appDetailResponse.result,
                     deploymentAppType: DeploymentAppTypes.GITOPS,
                 }
+
+                isAPICallInProgress = false
+                handleInitiatePolling()
+
                 IndexStore.publishAppDetails(genericAppDetail, AppType.EXTERNAL_ARGO_APP)
                 setErrorResponseCode(undefined)
             })
             .catch((errors: ServerErrors) => {
                 if (!getIsRequestAborted(errors)) {
                     showError(errors)
                     setErrorResponseCode(errors.code)
+                    isAPICallInProgress = false
+                    handleInitiatePolling()
                 }
             })
             .finally(() => {
                 setIsLoading(false)
                 isAPICallInProgress = false
                 setIsReloadResourceTreeInProgress(false)
-
-                if (shouldTriggerPolling) {
-                    initTimer = setTimeout(() => {
-                        _init()
-                    }, window._env_.EA_APP_DETAILS_POLLING_INTERVAL || 30000)
-                }
             })
     }
 

src/components/v2/appDetails/ea/EAAppDetail.component.tsx

@@ -40,6 +40,8 @@ import { getExternalLinks } from '../../../externalLinks/ExternalLinks.service'
 import { ExternalLinkIdentifierType, ExternalLinksAndToolsType } from '../../../externalLinks/ExternalLinks.type'
 import { sortByUpdatedOn } from '../../../externalLinks/ExternalLinks.utils'
 
+let initTimer = null
+
 const ExternalAppDetail = ({ appId, appName, isExternalApp }) => {
     const location = useLocation()
     const history = useHistory()
@@ -53,7 +55,6 @@ const ExternalAppDetail = ({ appId, appName, isExternalApp }) => {
 
     const abortControllerRef = useRef<AbortController>(new AbortController())
 
-    let initTimer = null
     let isAPICallInProgress = false
 
     // component load
@@ -80,7 +81,7 @@ const ExternalAppDetail = ({ appId, appName, isExternalApp }) => {
 
     const _init = () => {
         if (!isAPICallInProgress) {
-            _getAndSetAppDetail(true)
+            _getAndSetAppDetail()
         }
     }
 
@@ -120,7 +121,17 @@ const ExternalAppDetail = ({ appId, appName, isExternalApp }) => {
         return genericAppDetail
     }
 
-    const _getAndSetAppDetail = (shouldTriggerPolling: boolean = false) => {
+    const handleInitiatePolling = () => {
+        if (initTimer) {
+            clearTimeout(initTimer)
+        }
+
+        initTimer = setTimeout(() => {
+            _init()
+        }, window._env_.EA_APP_DETAILS_POLLING_INTERVAL || 30000)
+    }
+
+    const _getAndSetAppDetail = () => {
         isAPICallInProgress = true
         setIsReloadResourceTreeInProgress(true)
 
@@ -161,24 +172,21 @@ const ExternalAppDetail = ({ appId, appName, isExternalApp }) => {
                 }
 
                 setErrorResponseCode(undefined)
+
+                handleInitiatePolling()
             })
             .catch((errors: ServerErrors) => {
+                setIsLoading(false)
+                isAPICallInProgress = false
+
                 if (!getIsRequestAborted(errors)) {
                     showError(errors)
                     setErrorResponseCode(errors.code)
+                    handleInitiatePolling()
                 }
-
-                setIsLoading(false)
-                isAPICallInProgress = false
             })
             .finally(() => {
                 setIsReloadResourceTreeInProgress(false)
-
-                if (shouldTriggerPolling) {
-                    initTimer = setTimeout(() => {
-                        _init()
-                    }, window._env_.EA_APP_DETAILS_POLLING_INTERVAL || 30000)
-                }
             })
     }
 

src/components/v2/index.tsx

@@ -116,7 +116,7 @@ const RouterComponent = ({ envType }) => {
     }, [location.search])
 
     const _init = (fetchExternalLinks?: boolean) => {
-        abortPreviousRequests(() => _getAndSetAppDetail(fetchExternalLinks, true), abortControllerRef)
+        abortPreviousRequests(() => _getAndSetAppDetail(fetchExternalLinks), abortControllerRef)
     }
 
     const handleAppDetailsCallError = (e: any) => {
@@ -141,12 +141,16 @@ const RouterComponent = ({ envType }) => {
     }
 
     const handleInitiatePolling = () => {
+        if (initTimer) {
+            clearTimeout(initTimer)
+        }
+
         initTimer = setTimeout(() => {
             _init()
         }, window._env_.HELM_APP_DETAILS_POLLING_INTERVAL || 30000)
     }
 
-    const handleFetchAppDetails = async (fetchExternalLinks: boolean) => {
+    const handleFetchAppDetails = async (fetchExternalLinks: boolean): Promise<{ isAborted: boolean }> => {
         try {
             const response = await getInstalledChartDetail(+params.appId, +params.envId, abortControllerRef)
             handlePublishAppDetails(response)
@@ -155,32 +159,49 @@ const RouterComponent = ({ envType }) => {
             if (fetchExternalLinks) {
                 getExternalLinksAndTools(response.result?.clusterId)
             }
+            setLoadingDetails(false)
         } catch (error) {
-            handleAppDetailsCallError(error)
-        } finally {
+            const isAborted = getIsRequestAborted(error)
+            
+            if (!isAborted) {
+                handleAppDetailsCallError(error)
+            }
             setLoadingDetails(false)
+
+            return { isAborted }
         }
+
+        return { isAborted: false }
     }
 
     const handleFetchResourceTree = async () => {
         try {
             const response = await getInstalledChartResourceTree(+params.appId, +params.envId, abortControllerRef)
             handlePublishAppDetails(response)
+            setLoadingResourceTree(false)
         } catch (error) {
-            handleAppDetailsCallError(error)
-        } finally {
+            const isAborted = getIsRequestAborted(error)
+
+            if (!isAborted) {
+                handleAppDetailsCallError(error)
+            }
             setLoadingResourceTree(false)
+
+            return { isAborted }
         }
+
+        return { isAborted: false }
     }
 
-    const _getAndSetAppDetail = async (fetchExternalLinks: boolean, shouldTriggerPolling: boolean = false) => {
+    const _getAndSetAppDetail = async (fetchExternalLinks: boolean) => {
         if (envType === EnvType.CHART) {
             // Intentionally not setting await since it was not awaited earlier when in thens as well
             Promise.allSettled([
                 handleFetchAppDetails(fetchExternalLinks),
                 handleFetchResourceTree(),
-            ]).finally(() => {
-                if (shouldTriggerPolling) {
+            ]).then((results) => {
+                const isAborted = results.some((result) => result.status === 'fulfilled' && result.value.isAborted)
+                if (!isAborted) {
                     handleInitiatePolling()
                 }
             })
@@ -195,9 +216,8 @@ const RouterComponent = ({ envType }) => {
                     setErrorResponseCode(e.code)
                 }
             } finally {
-                if (shouldTriggerPolling) {
-                    handleInitiatePolling()
-                }
+                // BTW This is a dead block BTW should be removed
+                handleInitiatePolling()
             }
         }
     }