From bb350f9546d97790f2254df644c1eaeee3c3ba3e Mon Sep 17 00:00:00 2001
From: Daniil Ovchinnikov <daniil.ovchinnikov@jetbrains.com>
Date: Mon, 2 Sep 2024 23:37:33 +0200
Subject: [PATCH] IJPL-160526 make service registration resilient, log instead
 of throwing

GitOrigin-RevId: 5c43e5681b2aa90914ee64c93fdfe2d4c2125d64
---
 ...intellij.platform.instanceContainer.tests.iml |  1 +
 .../src/internal/InstanceRegistrarImpl.kt        | 16 +++++++++-------
 .../tests/InstanceContainerTest.kt               |  9 +++++----
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/platform/instanceContainer/intellij.platform.instanceContainer.tests.iml b/platform/instanceContainer/intellij.platform.instanceContainer.tests.iml
index df4bcdd1a8c1..9e2ed69e89e9 100644
--- a/platform/instanceContainer/intellij.platform.instanceContainer.tests.iml
+++ b/platform/instanceContainer/intellij.platform.instanceContainer.tests.iml
@@ -13,5 +13,6 @@
     <orderEntry type="library" scope="TEST" name="JUnit5" level="project" />
     <orderEntry type="module" module-name="intellij.platform.instanceContainer" scope="TEST" />
     <orderEntry type="module" module-name="intellij.platform.testFramework.common" scope="TEST" />
+    <orderEntry type="module" module-name="intellij.platform.tests" scope="TEST" />
   </component>
 </module>
\ No newline at end of file
diff --git a/platform/instanceContainer/src/internal/InstanceRegistrarImpl.kt b/platform/instanceContainer/src/internal/InstanceRegistrarImpl.kt
index 2356090f1fc1..5828baf920af 100644
--- a/platform/instanceContainer/src/internal/InstanceRegistrarImpl.kt
+++ b/platform/instanceContainer/src/internal/InstanceRegistrarImpl.kt
@@ -1,4 +1,4 @@
-// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.platform.instanceContainer.internal
 
 import com.intellij.openapi.diagnostic.trace
@@ -30,19 +30,20 @@ internal class InstanceRegistrarImpl(
     val actions = actions()
     val existingHolder = existingKeys[keyClassName]
     if (existingHolder != null) {
-      throw InstanceAlreadyRegisteredException(
+      LOG.error(InstanceAlreadyRegisteredException(
         keyClassName,
         existingInstanceClassName = existingHolder.instanceClassName(),
         newInstanceClassName = initializer.instanceClassName,
-      )
+      ))
+      return
     }
     when (val existingAction = actions[keyClassName]) {
       null -> actions[keyClassName] = RegistrationAction.Register(initializer)
-      is RegistrationAction.Register -> throw InstanceAlreadyRegisteredException(
+      is RegistrationAction.Register -> LOG.error(InstanceAlreadyRegisteredException(
         keyClassName,
         existingInstanceClassName = existingAction.initializer.instanceClassName,
         newInstanceClassName = initializer.instanceClassName,
-      )
+      ))
       is RegistrationAction.Override -> error("must not happen unless keyClassName is in existingKeys which is false") // sanity check
       RegistrationAction.Remove -> error("must not happen unless keyClassName is in existingKeys which is false") // sanity check
     }
@@ -53,7 +54,8 @@ internal class InstanceRegistrarImpl(
     val newAction = when (val existing = actions[keyClassName]) {
       null -> {
         if (keyClassName !in existingKeys) {
-          throw InstanceNotRegisteredException("$keyClassName -> ${initializer?.instanceClassName ?: "<removed>"}")
+          LOG.error(InstanceNotRegisteredException("$keyClassName -> ${initializer?.instanceClassName ?: "<removed>"}"))
+          return
         }
         if (initializer == null) RegistrationAction.Remove else RegistrationAction.Override(initializer)
       }
@@ -81,7 +83,7 @@ internal class InstanceRegistrarImpl(
       }
       is RegistrationAction.Remove -> {
         check(keyClassName in existingKeys) // sanity check
-        // TODO throw InstanceNotRegisteredException("$keyClassName -> ${initializer?.instanceClassName ?: "<removed>"}")
+        // TODO LOG.error(InstanceNotRegisteredException("$keyClassName -> ${initializer?.instanceClassName ?: "<removed>"}"))
         LOG.trace {
           "$debugString : $keyClassName is removed and overridden again in the same scope " +
           "(<removed> -> ${initializer?.instanceClassName})"
diff --git a/platform/instanceContainer/tests/InstanceContainerTest.kt b/platform/instanceContainer/tests/InstanceContainerTest.kt
index de06ab1dd11f..85dd1523cd3d 100644
--- a/platform/instanceContainer/tests/InstanceContainerTest.kt
+++ b/platform/instanceContainer/tests/InstanceContainerTest.kt
@@ -1,6 +1,7 @@
 // Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.platform.instanceContainer.tests
 
+import com.intellij.openapi.progress.assertLogThrows
 import com.intellij.platform.instanceContainer.CycleInitializationException
 import com.intellij.platform.instanceContainer.InstanceNotRegisteredException
 import com.intellij.platform.instanceContainer.internal.*
@@ -103,7 +104,7 @@ class InstanceContainerTest {
         registerInitializer(keyClassName, ReadyInitializer(instance), override = false)
 
         // re-registration in the same scope
-        assertThrows<InstanceAlreadyRegisteredException> {
+        assertLogThrows<InstanceAlreadyRegisteredException> {
           registerInitializer(keyClassName, ThrowingInitializer, override = false)
         }
 
@@ -114,7 +115,7 @@ class InstanceContainerTest {
 
       container.startRegistration(pluginScope).run {
         // re-registration in a different scope
-        assertThrows<InstanceAlreadyRegisteredException> {
+        assertLogThrows<InstanceAlreadyRegisteredException> {
           registerInitializer(keyClassName, ThrowingInitializer, override = false)
         }
         assertNull(complete())
@@ -135,14 +136,14 @@ class InstanceContainerTest {
     withContainer(testInfo.displayName) { container ->
 
       fun InstanceRegistrar.testOverrideNonExistent() {
-        assertThrows<InstanceNotRegisteredException> {
+        assertLogThrows<InstanceNotRegisteredException> {
           overrideInitializer(keyClassName, ThrowingInitializer)
         }
         assertNull(complete())
       }
 
       fun InstanceRegistrar.testRemoveNonExistent() {
-        assertThrows<InstanceNotRegisteredException> {
+        assertLogThrows<InstanceNotRegisteredException> {
           overrideInitializer(keyClassName, null)
         }
         assertNull(complete())