Merge pull request #2384 from digma-ai/improve-error-handling-in-AggressiveUpdateService

better error handling in AggressiveUpdateService Closes #2383

Author: shalom938

ide-common/src/main/java/org/digma/intellij/plugin/analytics/AnalyticsService.java

@@ -514,6 +514,11 @@ public void dispose() {
      * All calls to the AnalyticsProvider proxy must be wrapped by a call to this method.
      * this is purely catch exceptions and rethrow AnalyticsServiceException.
      */
+    /*
+    Note: this method should throw only AnalyticsServiceException.
+     the only case it will throw something else is on EDTAccessException or ReadAccessException which are severe errors
+     that should turn a red error to user, and we must catch those during development or testing before release.
+    */
     private <T> T executeCatching(Supplier<T> tSupplier) throws AnalyticsServiceException {
         try {
             return tSupplier.get();

ide-common/src/main/java/org/digma/intellij/plugin/common/ExceptionUtils.java

@@ -117,12 +117,7 @@ public static boolean isAnyConnectionException(@NotNull Throwable e) {
     }
 
     public static boolean isConnectionException(@NotNull Throwable e) {
-
-        var ex = e.getCause();
-        while (ex != null && !(isConnectionUnavailableException(ex))) {
-            ex = ex.getCause();
-        }
-        return ex != null;
+        return findConnectException(e) != null;
     }
 
     public static boolean isConnectionUnavailableException(@NotNull Throwable exception) {
@@ -186,8 +181,7 @@ private static boolean is404PageNotFound(@NotNull Throwable exception) {
 
 
     public static boolean isSslConnectionException(@NotNull Throwable e) {
-        var cause = findAssignableCause(SSLException.class, e);
-        return cause != null;
+        return findSslException(e) != null;
     }
 
     @Nullable

ide-common/src/main/kotlin/org/digma/intellij/plugin/updates/AggressiveUpdateService.kt

@@ -10,14 +10,12 @@ import org.apache.maven.artifact.versioning.ComparableVersion
 import org.digma.intellij.plugin.analytics.AnalyticsService
 import org.digma.intellij.plugin.analytics.AnalyticsServiceConnectionEvent
 import org.digma.intellij.plugin.analytics.ApiClientChangedEvent
-import org.digma.intellij.plugin.analytics.BackendInfoHolder
 import org.digma.intellij.plugin.common.DisposableAdaptor
 import org.digma.intellij.plugin.common.ExceptionUtils
 import org.digma.intellij.plugin.common.buildVersionRequest
 import org.digma.intellij.plugin.common.getPluginVersion
 import org.digma.intellij.plugin.common.isProjectValid
 import org.digma.intellij.plugin.common.newerThan
-import org.digma.intellij.plugin.common.runWIthRetry
 import org.digma.intellij.plugin.errorreporting.ErrorReporter
 import org.digma.intellij.plugin.log.Log
 import org.digma.intellij.plugin.model.rest.AboutResult
@@ -118,7 +116,9 @@ class AggressiveUpdateService(val project: Project) : DisposableAdaptor {
     }
 
 
-    @Synchronized
+    //startMonitoring is called from init while the object is constructed.
+    //it should never be called concurrently because this is a project service and the platform guards against
+    // concurrent creation of services.
     private fun startMonitoring() {
 
         Log.log(logger::debug, "startMonitoring called")
@@ -128,7 +128,12 @@ class AggressiveUpdateService(val project: Project) : DisposableAdaptor {
             return
         }
 
-        disposingPeriodicTask("AggressiveUpdate.periodic", delayBetweenUpdatesSeconds.inWholeMilliseconds, true) {
+        //this task is paused by the scheduler if there is no connection. It's useless to try to update without backend because there is no way to
+        // know the backend version.
+        //but because this service starts on project startup,and actually IDE startup, it may run once before ApiErrorHandler had the chance
+        // to mark connection lost. if it will run when backend is not running there will be too many error reports in posthog,
+        // so it should ignore no connection errors.
+        disposingPeriodicTask("AggressiveUpdate.periodic", 10.seconds.inWholeMilliseconds, delayBetweenUpdatesSeconds.inWholeMilliseconds, true) {
             update()
         }
     }
@@ -140,42 +145,45 @@ class AggressiveUpdateService(val project: Project) : DisposableAdaptor {
             updateState()
             Log.log(logger::trace, "updating state on startup completed successfully")
         } catch (e: Throwable) {
-            val message = ExceptionUtils.getNonEmptyMessage(e)
-            Log.debugWithException(logger, e, "error in updateStateNow {}", message)
-            errorReporter.reportError("AggressiveUpdateService.updateStateNow", e)
+            //don't report connection errors , its useless and will report too many errors to posthog and the log
+            if (!ExceptionUtils.isAnyConnectionException(e)) {
+                val message = ExceptionUtils.getNonEmptyMessage(e)
+                Log.debugWithException(logger, e, "error in update {}", message)
+                errorReporter.reportError("AggressiveUpdateService.update", e)
+            }
         }
     }
 
 
     private fun updateState() {
-
-        runWIthRetry({
-            Log.log(logger::debug, "loading versions")
-            val versions = buildVersions()
-            Log.log(logger::debug, "loaded versions {}", versions)
-            Log.log(logger::debug, "updating state")
-            val prevUpdateState = updateStateRef.get().copy()
-            update(versions)
-            Log.log(logger::debug, "state updated. prev state: {}, new state: {}", prevUpdateState, updateStateRef)
-        }, backOffMillis = 2000, maxRetries = 5)
+        Log.log(logger::debug, "loading versions")
+        val versions = buildVersions()
+        Log.log(logger::debug, "loaded versions {}", versions)
+        Log.log(logger::debug, "updating state")
+        val prevUpdateState = updateStateRef.get().copy()
+        update(versions)
+        Log.log(logger::debug, "state updated. prev state: {}, new state: {}", prevUpdateState, updateStateRef)
     }
 
 
+    //if there is no backend running this method will throw an AnalyticsServiceException
     private fun buildVersions(): Versions {
-        try {
+        return try {
             val versionsResponse = AnalyticsService.getInstance(project).getVersions(buildVersionRequest())
             reportVersionsErrorsIfNecessary(versionsResponse.errors)
-            return Versions.fromVersionsResponse(versionsResponse)
+            Versions.fromVersionsResponse(versionsResponse)
         } catch (e: Throwable) {
-            val message = ExceptionUtils.getNonEmptyMessage(e)
-            Log.debugWithException(logger, e, "error in buildVersions {}", message)
-            errorReporter.reportError("AggressiveUpdateService.buildVersions", e)
-
-            //if getVersions failed try to get server version from getAbout
-            val about = BackendInfoHolder.getInstance(project).getAbout()
-            about?.let {
-                return Versions.fromAboutResponse(it)
-            } ?: throw RuntimeException("could not get backend info from getVersions or getAbout")
+            //don't report connection errors , its useless and will report too many errors to posthog and the log
+            if (!ExceptionUtils.isAnyConnectionException(e)) {
+                val message = ExceptionUtils.getNonEmptyMessage(e)
+                Log.debugWithException(logger, e, "error in buildVersions {}", message)
+                errorReporter.reportError("AggressiveUpdateService.buildVersions", e)
+            }
+
+            //if getVersions failed try to get server version from getAbout.
+            //if there is no connection getAbout will throw an AnalyticsServiceException
+            val about = AnalyticsService.getInstance(project).about
+            Versions.fromAboutResponse(about)
         }
     }