Skip to content

Exception reporter disentangling for Android UI code #1345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions workflow-runtime/api/workflow-runtime.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,33 @@ 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 BugSnag'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
Expand Down Expand Up @@ -75,4 +102,4 @@ internal inline fun checkWithKey(
* 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
public expect fun <T : Throwable> T.withKey(stackTraceKey: Any): T
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package com.squareup.workflow1.internal

actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = this
public actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = this
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package com.squareup.workflow1.internal

actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = this
public actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = this
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.squareup.workflow1.internal

internal actual fun <T : Throwable> T.withKey(stackTraceKey: Any): T = apply {
public 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".
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -72,5 +74,5 @@ public fun <ScreenT : Screen> ScreenComposableFactoryFinder.requireComposableFac
environment[ViewRegistry]
.getEntryFor(Key(rendering::class, ScreenComposableFactory::class))
}."
)
).withKey(keyFor(rendering))
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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))
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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>() {
Expand Down