Merge pull request #1345 from square/ray/more-exception-detangling

Exception reporter disentangling for Android UI code

Author: rjrjr

workflow-runtime/api/workflow-runtime.api

@@ -116,3 +116,12 @@ public final class com/squareup/workflow1/WorkflowInterceptor$WorkflowSession$De
 	public static fun isRootWorkflow (Lcom/squareup/workflow1/WorkflowInterceptor$WorkflowSession;)Z
 }
 
+public final class com/squareup/workflow1/internal/ThrowablesKt {
+	public static final fun requireNotNullWithKey (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function0;)Ljava/lang/Object;
+	public static synthetic fun requireNotNullWithKey$default (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function0;ILjava/lang/Object;)Ljava/lang/Object;
+}
+
+public final class com/squareup/workflow1/internal/Throwables_jvmKt {
+	public static final fun withKey (Ljava/lang/Throwable;Ljava/lang/Object;)Ljava/lang/Throwable;
+}
+

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/Throwables.kt

@@ -3,9 +3,36 @@ package com.squareup.workflow1.internal
 import kotlin.contracts.ExperimentalContracts
 import kotlin.contracts.contract
 
+/**
+ * Like Kotlin's [requireNotNull], but uses [stackTraceKey] to create a fake top element
+ * on the stack trace, ensuring that a crash reporter's default grouping will create unique
+ * groups for unique keys.
+ *
+ * @see [withKey]
+ *
+ * @throws IllegalArgumentException if the [value] is false.
+ */
+@OptIn(ExperimentalContracts::class)
+inline fun <T : Any> requireNotNullWithKey(
+  value: T?,
+  stackTraceKey: Any,
+  lazyMessage: () -> Any = { "Required value was null." }
+): T {
+  contract {
+    returns() implies (value != null)
+  }
+  if (value == null) {
+    val message = lazyMessage()
+    val exception: Throwable = IllegalArgumentException(message.toString())
+    throw exception.withKey(stackTraceKey)
+  } else {
+    return value
+  }
+}
+
 /**
  * Like Kotlin's [require], but uses [stackTraceKey] to create a fake top element
- * on the stack trace, ensuring that crash reporter's default grouping will create unique
+ * on the stack trace, ensuring that a crash reporter's default grouping will create unique
  * groups for unique keys.
  *
  * So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
@@ -36,7 +63,7 @@ internal inline fun requireWithKey(
 
 /**
  * Like Kotlin's [check], but uses [stackTraceKey] to create a fake top element
- * on the stack trace, ensuring that crash reporter's default grouping will create unique
+ * on the stack trace, ensuring that a crash reporter's default grouping will create unique
  * groups for unique keys.
  *
  * So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
@@ -67,12 +94,12 @@ internal inline fun checkWithKey(
 
 /**
  * Uses [stackTraceKey] to create a fake top element on the stack trace, ensuring
- * that crash reporter's default grouping will create unique groups for unique keys.
+ * that a crash reporter's default grouping will create unique groups for unique keys.
  *
  * So far only effective on JVM, this is a pass through in other languages.
  *
  * @param stackTraceKey an object whose [toString] method will serve as a grouping key
  * for crash reporters. It is important that keys are stable across processes,
  * avoid system hashes.
  */
-internal expect fun <T : Throwable> T.withKey(stackTraceKey: Any): T
+expect fun <T : Throwable> T.withKey(stackTraceKey: Any): T

workflow-runtime/src/jvmMain/kotlin/com/squareup/workflow1/internal/Throwables.jvm.kt

@@ -1,6 +1,6 @@
 package com.squareup.workflow1.internal
 
-internal actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = apply {
+actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = apply {
   val realTop = stackTrace[0]
   val fakeTop = StackTraceElement(
     // Real class name to ensure that we are still "in project".

workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/ScreenComposableFactoryFinder.kt

@@ -2,6 +2,8 @@ package com.squareup.workflow1.ui.compose
 
 import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.remember
+import com.squareup.workflow1.internal.withKey
+import com.squareup.workflow1.ui.Compatible.Companion.keyFor
 import com.squareup.workflow1.ui.EnvironmentScreen
 import com.squareup.workflow1.ui.NamedScreen
 import com.squareup.workflow1.ui.Screen
@@ -72,5 +74,5 @@ public fun <ScreenT : Screen> ScreenComposableFactoryFinder.requireComposableFac
           environment[ViewRegistry]
             .getEntryFor(Key(rendering::class, ScreenComposableFactory::class))
         }."
-    )
+    ).withKey(keyFor(rendering))
 }

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactoryFinder.kt

@@ -1,5 +1,7 @@
 package com.squareup.workflow1.ui
 
+import com.squareup.workflow1.internal.withKey
+import com.squareup.workflow1.ui.Compatible.Companion.keyFor
 import com.squareup.workflow1.ui.ScreenViewFactory.Companion.forWrapper
 import com.squareup.workflow1.ui.ViewRegistry.Key
 import com.squareup.workflow1.ui.navigation.BackStackScreen
@@ -98,5 +100,5 @@ public fun <ScreenT : Screen> ScreenViewFactoryFinder.requireViewFactoryForRende
         "ViewEnvironment.withComposeInteropSupport() " +
         "from module com.squareup.workflow1:workflow-ui-compose at the top " +
         "of your Android view hierarchy."
-    )
+    ).withKey(keyFor(rendering))
 }

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowSavedStateRegistryAggregator.kt

@@ -11,6 +11,8 @@ import androidx.lifecycle.findViewTreeLifecycleOwner
 import androidx.savedstate.SavedStateRegistryOwner
 import androidx.savedstate.findViewTreeSavedStateRegistryOwner
 import androidx.savedstate.setViewTreeSavedStateRegistryOwner
+import com.squareup.workflow1.internal.requireNotNullWithKey
+import com.squareup.workflow1.internal.withKey
 
 /**
  * Manages a group of [SavedStateRegistryOwner]s that are all saved to
@@ -106,6 +108,7 @@ public class WorkflowSavedStateRegistryAggregator {
       } catch (e: IllegalStateException) {
         // Exception thrown by SavedStateRegistryOwner is pretty useless.
         throw IllegalStateException("Error consuming $parentKey from $parentRegistryOwner", e)
+          .withKey(parentKey.orEmpty())
       }
       restoreFromBundle(restoredState)
     }
@@ -163,7 +166,7 @@ public class WorkflowSavedStateRegistryAggregator {
           "Compatible.compatibilityKey -- note the name fields on BodyAndOverlaysScreen " +
           "and BackStackScreen.",
         e
-      )
+      ).withKey(key)
     }
 
     // Even if the parent lifecycle is in a state further than CREATED, new observers are sent all
@@ -223,20 +226,21 @@ public class WorkflowSavedStateRegistryAggregator {
     key: String,
     force: Boolean = false
   ) {
-    val lifecycleOwner = requireNotNull(view.findViewTreeLifecycleOwner()) {
+    val lifecycleOwner = requireNotNullWithKey(view.findViewTreeLifecycleOwner(), key) {
       "Expected $view($key) to have a ViewTreeLifecycleOwner. " +
         "Use WorkflowLifecycleOwner to fix that."
     }
     val registryOwner = KeyedSavedStateRegistryOwner(key, lifecycleOwner)
     children.put(key, registryOwner)?.let {
       throw IllegalArgumentException("$key is already in use, it cannot be used to register $view")
+        .withKey(key)
     }
     view.findViewTreeSavedStateRegistryOwner()
       ?.takeIf { !force || it is KeyedSavedStateRegistryOwner }
       ?.let {
         throw IllegalArgumentException(
           "Using $key to register $view, but it already has SavedStateRegistryOwner: $it"
-        )
+        ).withKey(key)
       }
     view.setViewTreeSavedStateRegistryOwner(registryOwner)
     restoreIfOwnerReady(registryOwner)
@@ -253,6 +257,7 @@ public class WorkflowSavedStateRegistryAggregator {
   public fun saveAndPruneChildRegistryOwner(key: String) {
     children.remove(key)?.let { saveIfOwnerReady(it) }
       ?: throw IllegalArgumentException("No such child: $key, on parent $parentKey")
+        .withKey(key)
   }
 
   private fun saveIfOwnerReady(child: KeyedSavedStateRegistryOwner) {

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/navigation/OverlayDialogFactoryFinder.kt

@@ -1,5 +1,7 @@
 package com.squareup.workflow1.ui.navigation
 
+import com.squareup.workflow1.internal.withKey
+import com.squareup.workflow1.ui.Compatible.Companion.keyFor
 import com.squareup.workflow1.ui.Screen
 import com.squareup.workflow1.ui.ViewEnvironment
 import com.squareup.workflow1.ui.ViewEnvironmentKey
@@ -31,7 +33,7 @@ public interface OverlayDialogFactoryFinder {
       ?: throw IllegalArgumentException(
         "An OverlayDialogFactory should have been registered to display $rendering, " +
           "or that class should implement AndroidOverlay. Instead found $entry."
-      )
+      ).withKey(keyFor(rendering))
   }
 
   public companion object : ViewEnvironmentKey<OverlayDialogFactoryFinder>() {