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

Scanner throws exception from advertisements Flow on premature closure #67

Closed
twyatt opened this issue Feb 10, 2021 · 1 comment · Fixed by #68
Closed

Scanner throws exception from advertisements Flow on premature closure #67

twyatt opened this issue Feb 10, 2021 · 1 comment · Fixed by #68
Assignees
Milestone

Comments

@twyatt
Copy link
Member

twyatt commented Feb 10, 2021

Issue/question raised on StackOverflow.

I believe the issue is with Kable's internals not properly handling the premature closure in the callbackFlow.

As per the callbackFlow KDoc, we should be catching exceptions when trying to emit within the callbackFlow (emphasis shown in green):

  fun flowFrom(api: CallbackBasedApi): Flow<T> = callbackFlow {
      val callback = object : Callback { // Implementation of some callback interface
          override fun onNextValue(value: T) {
              // To avoid blocking you can configure channel capacity using
              // either buffer(Channel.CONFLATED) or buffer(Channel.UNLIMITED) to avoid overfill
+             try {
                  sendBlocking(value)
+             } catch (e: Exception) {
+                 // Handle exception from the channel: failure in flow or premature closing
+             }
          }
          override fun onApiError(cause: Throwable) {
              cancel(CancellationException("API Error", cause))
          }
          override fun onCompleted() = channel.close()
      }
      api.register(callback)
      /*
       * Suspends until either 'onCompleted'/'onApiError' from the callback is invoked
       * or flow collector is cancelled (e.g. by 'take(1)' or because a collector's coroutine was cancelled).
       * In both cases, callback will be properly unregistered.
       */
          awaitClose { api.unregister(callback) }
      }
@twyatt twyatt self-assigned this Feb 10, 2021
@twyatt twyatt added this to the 0.3.0 milestone Feb 10, 2021
@twyatt
Copy link
Member Author

twyatt commented Feb 10, 2021

Fix can be tried/tested via:

import java.net.URI

repositories {
    maven { url = URI("https://oss.sonatype.org/content/repositories/snapshots") }
}

dependencies {
    implementation("com.juul.kable:core:0.2.0-issue-67-SNAPSHOT")
}

SNAPSHOT published from #68.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant