Skip to content
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

Handle uncaught exceptions from async calls #1653

Merged
merged 3 commits into from
Nov 1, 2023
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,17 @@ import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.protocol.guest.ProtocolBridge
import app.cash.redwood.protocol.guest.ProtocolMismatchHandler
import app.cash.redwood.treehouse.AppLifecycle.Host
import kotlin.coroutines.CoroutineContext
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.GlobalScope
import kotlinx.serialization.json.Json

@OptIn(DelicateCoroutinesApi::class)
public class StandardAppLifecycle(
internal val protocolBridgeFactory: ProtocolBridge.Factory,
internal val json: Json,
internal val widgetVersion: UInt,
) : AppLifecycle {
private lateinit var host: Host
internal val coroutineScope: CoroutineScope = GlobalScope

private lateinit var broadcastFrameClock: BroadcastFrameClock
internal lateinit var frameClock: MonotonicFrameClock
Expand All @@ -50,6 +48,17 @@ public class StandardAppLifecycle(
}
}

private val coroutineExceptionHandler = object : CoroutineExceptionHandler {
override val key: CoroutineContext.Key<*>
get() = CoroutineExceptionHandler.Key

override fun handleException(context: CoroutineContext, exception: Throwable) {
host.handleUncaughtException(exception)
}
}

internal val coroutineScope = CoroutineScope(coroutineExceptionHandler)

override fun start(host: Host) {
this.host = host
this.broadcastFrameClock = BroadcastFrameClock { host.requestFrame() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package app.cash.redwood.treehouse

import kotlin.coroutines.CoroutineContext
import kotlinx.coroutines.CoroutineExceptionHandler

/** Manages loading and hot-reloading a series of code sessions. */
internal interface CodeHost<A : AppService> {
val stateStore: StateStore
Expand All @@ -24,6 +27,9 @@ internal interface CodeHost<A : AppService> {

fun newServiceScope(): ServiceScope<A>

/** Cancels the current code and propagates [exception] to all listeners. */
fun handleUncaughtException(exception: Throwable)

fun addListener(listener: Listener<A>)

fun removeListener(listener: Listener<A>)
Expand All @@ -46,3 +52,12 @@ internal interface CodeHost<A : AppService> {
fun close()
}
}

internal fun CodeHost<*>.asExceptionHandler() = object : CoroutineExceptionHandler {
override val key: CoroutineContext.Key<*>
get() = CoroutineExceptionHandler.Key

override fun handleException(context: CoroutineContext, exception: Throwable) {
handleUncaughtException(exception)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,20 @@ public open class EventListener {
name: String,
) {
}

/**
* Invoked when [app] has thrown an uncaught exception.
*
* This indicates an unrecoverable software bug. Development implementations should report the
* exception to the developer. Production implementations should post the exception to a bug
* tracking service.
*
* When a Treehouse app fails its current [Zipline] instance is canceled so no further code will
* execute. A new [Zipline] will start when new code available, or when the app is restarted.
*/
public open fun uncaughtException(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For follow-up, I’d like more Zipline-specific context here. The ZiplineManifest has the release version, and that’ll be super valuable for reporting upstream.

app: TreehouseApp<*>,
exception: Throwable,
) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,6 @@ internal interface EventPublisher {
fun onUnknownEvent(widgetTag: WidgetTag, tag: EventTag)

fun onUnknownEventNode(id: Id, tag: EventTag)

fun onUncaughtException(exception: Throwable)
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,8 @@ internal class RealEventPublisher(
override fun onUnknownEventNode(id: Id, tag: EventTag) {
listener.onUnknownEventNode(app, id, tag)
}

override fun onUncaughtException(exception: Throwable) {
listener.uncaughtException(app, exception)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import app.cash.zipline.loader.ZiplineCache
import app.cash.zipline.loader.ZiplineHttpClient
import app.cash.zipline.loader.ZiplineLoader
import app.cash.zipline.withScope
import kotlin.coroutines.CoroutineContext
import kotlin.native.ObjCName
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineExceptionHandler
Expand Down Expand Up @@ -52,7 +51,7 @@ public class TreehouseApp<A : AppService> private constructor(
private val codeHost = ZiplineCodeHost<A>()

public val dispatchers: TreehouseDispatchers =
TreehouseDispatchersWithExceptionHandler(factory.dispatchers, codeHost.exceptionHandler)
TreehouseDispatchersWithExceptionHandler(factory.dispatchers, codeHost.asExceptionHandler())

private val eventPublisher = RealEventPublisher(factory.eventListener, this)

Expand Down Expand Up @@ -205,22 +204,6 @@ public class TreehouseApp<A : AppService> private constructor(

override var session: ZiplineCodeSession<A>? = null

/** Propagates exceptions on the Zipline dispatcher to the listeners. */
val exceptionHandler = object : CoroutineExceptionHandler {
override val key: CoroutineContext.Key<*>
get() = CoroutineExceptionHandler.Key

override fun handleException(context: CoroutineContext, exception: Throwable) {
appScope.launch(dispatchers.ui) {
for (listener in listeners) {
listener.uncaughtException(exception)
}
codeHost.session?.cancel()
codeHost.session = null
}
}
}

override fun newServiceScope(): CodeHost.ServiceScope<A> {
val ziplineScope = ZiplineScope()

Expand All @@ -245,12 +228,25 @@ public class TreehouseApp<A : AppService> private constructor(
listeners -= listener
}

override fun handleUncaughtException(exception: Throwable) {
appScope.launch(dispatchers.ui) {
for (listener in listeners) {
listener.uncaughtException(exception)
}
codeHost.session?.cancel()
codeHost.session = null
}

eventPublisher.onUncaughtException(exception)
}

fun onCodeChanged(zipline: Zipline, appService: A) {
val sessionScope = CoroutineScope(SupervisorJob(appScope.coroutineContext.job))
sessionScope.launch(dispatchers.ui) {
val previous = session

val next = ZiplineCodeSession(
codeHost = this@ZiplineCodeHost,
dispatchers = dispatchers,
eventPublisher = eventPublisher,
appScope = appScope,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import kotlinx.coroutines.launch
import kotlinx.serialization.json.Json

internal class ZiplineCodeSession<A : AppService>(
private val codeHost: CodeHost<*>,
private val dispatchers: TreehouseDispatchers,
private val eventPublisher: EventPublisher,
private val appScope: CoroutineScope,
Expand All @@ -44,7 +45,7 @@ internal class ZiplineCodeSession<A : AppService>(
frameClock.start(sessionScope, dispatchers)
sessionScope.launch(dispatchers.zipline) {
val appLifecycle = appService.withScope(ziplineScope).appLifecycle
val host = RealAppLifecycleHost(appLifecycle, eventPublisher, frameClock)
val host = RealAppLifecycleHost(codeHost, appLifecycle, eventPublisher, frameClock)
appLifecycle.start(host)
}
}
Expand All @@ -60,6 +61,7 @@ internal class ZiplineCodeSession<A : AppService>(

/** Platform features to the guest application. */
private class RealAppLifecycleHost(
val codeHost: CodeHost<*>,
val appLifecycle: AppLifecycle,
val eventPublisher: EventPublisher,
val frameClock: FrameClock,
Expand All @@ -81,4 +83,8 @@ private class RealAppLifecycleHost(
) {
eventPublisher.onUnknownEventNode(id, tag)
}

override fun handleUncaughtException(exception: Throwable) {
codeHost.handleUncaughtException(exception)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal class FakeCodeHost : CodeHost<FakeAppService> {
}
}

fun triggerException(exception: Throwable) {
override fun handleUncaughtException(exception: Throwable) {
for (listener in listeners) {
listener.uncaughtException(exception)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,7 @@ class FakeEventPublisher : EventPublisher {

override fun onUnknownEventNode(id: Id, tag: EventTag) {
}

override fun onUncaughtException(exception: Throwable) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class TreehouseAppContentTest {
content.bind(view1)
eventLog.takeEvent("codeSessionA.app.uis[0].start()")

codeHost.triggerException(Exception("boom!"))
codeHost.handleUncaughtException(Exception("boom!"))
eventLog.takeEventsInAnyOrder(
"codeSessionA.app.uis[0].close()",
"codeListener.onUncaughtException(view1, kotlin.Exception: boom!)",
Expand All @@ -321,7 +321,7 @@ class TreehouseAppContentTest {
codeHost.session = FakeCodeSession("codeSessionA", eventLog)
eventLog.takeEvent("codeSessionA.start()")

codeHost.triggerException(Exception("boom!"))
codeHost.handleUncaughtException(Exception("boom!"))
eventLog.takeEvent("codeSessionA.cancel()")

val view1 = FakeTreehouseView("view1")
Expand All @@ -347,7 +347,7 @@ class TreehouseAppContentTest {
content.bind(view1)
eventLog.takeEvent("codeSessionA.app.uis[0].start()")

codeHost.triggerException(Exception("boom!"))
codeHost.handleUncaughtException(Exception("boom!"))
eventLog.takeEventsInAnyOrder(
"codeSessionA.app.uis[0].close()",
"codeListener.onUncaughtException(view1, kotlin.Exception: boom!)",
Expand Down Expand Up @@ -376,7 +376,7 @@ class TreehouseAppContentTest {
content.preload(FakeOnBackPressedDispatcher(), uiConfiguration)
eventLog.takeEvent("codeSessionA.app.uis[0].start()")

codeHost.triggerException(Exception("boom!"))
codeHost.handleUncaughtException(Exception("boom!"))
eventLog.takeEvent("codeSessionA.app.uis[0].close()")
eventLog.takeEvent("codeSessionA.cancel()")

Expand Down
3 changes: 3 additions & 0 deletions redwood-treehouse/api/zipline-api.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ functions = [
# fun close(): kotlin.Unit
"moYx+T3e",

# fun handleUncaughtException(kotlin.Throwable): kotlin.Unit
"Hls+uhG7",

# fun onUnknownEvent(app.cash.redwood.protocol.WidgetTag, app.cash.redwood.protocol.EventTag): kotlin.Unit
"jmKreoSS",

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@ public interface AppLifecycle : ZiplineService {
public interface Host : ZiplineService {
public fun requestFrame()

/** Notify the host that an event was unrecognized and will be ignored. */
public fun onUnknownEvent(widgetTag: WidgetTag, tag: EventTag)

/**
* Notify the host that an event was received for a node that no longer exists.
* That event will be ignored.
*/
public fun onUnknownEventNode(id: Id, tag: EventTag)

/** Handle an uncaught exception. The app is now in an undefined state and must be stopped. */
public fun handleUncaughtException(exception: Throwable)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateListOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.Saver
import androidx.compose.runtime.saveable.SaverScope
import androidx.compose.runtime.saveable.rememberSaveable
Expand All @@ -44,6 +45,7 @@ import com.example.redwood.emojisearch.compose.Image
import com.example.redwood.emojisearch.compose.Text
import com.example.redwood.emojisearch.compose.TextInput
import example.values.TextFieldState
import kotlinx.coroutines.launch
import kotlinx.serialization.json.Json

data class EmojiImage(
Expand Down Expand Up @@ -85,6 +87,7 @@ private fun LazyColumn(
httpClient: HttpClient,
navigator: Navigator,
) {
val scope = rememberCoroutineScope()
val allEmojis = remember { mutableStateListOf<EmojiImage>() }

// Simple counter that allows us to trigger refreshes by simple incrementing the value
Expand Down Expand Up @@ -139,9 +142,15 @@ private fun LazyColumn(
hint = "Search",
onChange = { textFieldState ->
// Make it easy to trigger a crash to manually test exception handling!
if (textFieldState.text == "crash") {
throw RuntimeException("boom!")
when (textFieldState.text) {
"crash" -> throw RuntimeException("boom!")
"async" -> {
scope.launch {
throw RuntimeException("boom!")
}
}
}

searchTerm = textFieldState
},
)
Expand Down