Peripheral.observe error propagation

[twyatt]

With the current design, errors related to an observation (Peripheral.observe), are propagated via an exception being thrown in the flow. This allows for handling of failures via flow operators such as catch or retry, but exceptions thrown in a flow are terminal events.

Having the observe flow terminate can be problematic, as the flow is a kind of hybrid cold/hot flow, whereas it reacts (performs actions) based on the number of subscribers. When subscribers goes from 0 to 1, the characteristic notifications/indications will be enabled (and appropriate descriptor written to) and the converse (when subscribers goes from 1 to 0) will perform the inverse (unwinding the notification/indication setup).

observe was designed to be long-living, as in: you can subscribe and it will remain active across disconnects/reconnects (automatically re-configuring any notifications/indications as needed on reconnect). This simplifies its usage considerably but having the flow remain active across some errors states (connection drop) but not others (failure to write to config descriptor) is inconsistent. Combined with the automatic handling of spinning up/down indications/notifications, makes error handling difficult and error prone. Using retry would keep the flow alive for downstream subscribers but would have the adverse side-effect of spinning down/up any notifications/indications (because subscribers will momentarily go from 1 to 0, then back to 1) — these operations can be expensive and are unnecessary.

The design isn't yet solidified but materialize operator might provide a simple error handling mechanism. The challenge is being able to propagate failures without terminating the observe flow. It isn't clear if materialize would offer such functionality.

Rather than wait for materialize operator to be available, the observe flow should propagate a sealed class of event types (either data or error). This will allow the observe flow to remain active when, for example, failures to spin up notifications/indications occur. Instead of throwing the exception in the flow, a Observation.Error (or similarly named class) will be propagated. Down stream subscribers can dematerialize failures if desired, but the flow would remain active and only unexpected ("exceptional") Exceptions would be thrown from an observe flow (i.e. the flow would only terminate via exception due to a bug in Kable).

[twyatt]

Recent changes in #190 and #193 has reduced the number of exceptions that are propagated through observation flows, but some failures remain problematic, especially those that occur due to a connection drop in the middle of an I/O operation (that is being executed to spin up/down observations):

kable/core/src/androidMain/kotlin/Connection.kt

Lines 52 to 73 in 941c8ad

	suspend inline fun <reified T> execute(
	crossinline action: BluetoothGatt.() -> Boolean,
	): T = lock.withLock {
	withContext(dispatcher) {
	action.invoke(bluetoothGatt) || throw GattRequestRejectedException()
	}
	val response = try {
	callback.onResponse.receive()
	} catch (e: ConnectionLostException) {
	throw ConnectionLostException(cause = e)
	}
	if (response.status != GattSuccess) throw GattStatusException(response.toString())
	// `lock` should always enforce a 1:1 matching of request to response, but if an Android `BluetoothGattCallback`
	// method gets called out of order then we'll cast to the wrong response type.
	response as? T
	?: throw OutOfOrderGattCallbackException(
	"Unexpected response type ${response.javaClass.simpleName} received"
	)
	}

• Android may return false on line 56 if the BluetoothGatt has already been invalidated due to a connection drop
• An I/O operation may fail and trigger a callback with a non-success status (line 65)

For a typical I/O operation, where a library consumer is wanting to write or read data, these failures can be easily caught and handled as desired. For observations (which are supposed to remain active across disconnect/reconnect) it becomes a grey area if these failures should be "ignored" or propagated (and terminate) through the observation flow.

In many cases, these failures are accompanied by a connection loss, in which case it makes sense to ignore them as it is expected the library consumer will reconnect and the observation will be re-wired (and everything will carry on). But, in the case where the connection isn't lost, then the library consumer needs to be made aware that their flow observation did not spin up correctly.

The dilemma is that at the time of failure, we don't know with certainty that the connection will soon be dropped — so we can't accurately make a decision to ignore the failure.

Some options are:

1. When observation flow failure occurs, asynchronously wait for a set timeout to see if connection drops: if it drops then ignore failure, if it doesn't then propagate the failure through the flow
   • Timing based solutions feel especially error-prone to me, plus it feels too opinionated for Kable to handle failures in this way
2. Add an observationExceptionHandler option to the Peripheral builder — it would allow library consumers to decide how observation failures are handled
   • e.g. re-throw in the handler if you want the exception to propagate through (and terminate) the observation flow
3. Introduce a sealed class for observe (that carries either the data or the failure)
   • This is pretty invasive on the API (breaking change) and introduces complexity to the API just for handling this edge case
4. Other?

[twyatt]

Settled on option 2 in #254.