Extend the KDoc for some channel APIs #4148
Conversation
Before, the description of `SUSPEND` was phrased in terms of what will happen, while the rest of the entries were described in an imperative form, that is, as commands as to what should happen. Now, all entries are clarified using a descriptive form.
Added explanations of what exactly happens on each code path, how these operators ensure that all elements get processed eventually, and provided some usage examples.
Currently, the documentation states that uncaught exceptions will lead to the channel being closed. "Uncaught exceptions" is a special thing in kotlinx.coroutines: <https://kotlinlang.org/docs/exception-handling.html#coroutineexceptionhandler> These are not just exceptions that are not wrapped in a try-catch, these are exceptions that can not be propagated to a root coroutine via structured concurrency. Fixed the wording and added a test that shows that uncaught coroutine exceptions are not handled in any special manner.
Turns out, only a single invocation of either `awaitClose` or `invokeOnClose` is allowed in the lifetime of a channel. Document that.
Instead, use `Channel.RENDEZVOUS` so that a meaningful constant is shown in Dokka's output.
Currently, the docs claim that attempting to receive from a failed channel fails. However, the documentation for `Channel` itself correctly states that before `receive` fails, the elements that were already sent will be processed first. Corrected this and added a test demonstrating the behavior.
| * print("in the producer: ") | ||
| * it.cleanup() | ||
| * } | ||
| * yield() |
There was a problem hiding this comment.
yield is redundant and rather confusing here, let's avoid that?
| * } | ||
| * // Grab the first value and discard everything else | ||
| * launch(Dispatchers.Default) { | ||
| * val firstElement = channel.consumeFirst() |
There was a problem hiding this comment.
Maybe invoke it in-place instead of a single coroutine?
| * } | ||
| * ``` | ||
| * | ||
| * In this example, all ten values created by the producer coroutine will be processed: one by `consumeFirst`, |
There was a problem hiding this comment.
Reading this, it seems like the example is mixing two concepts: one is about consuming, and the other about undelivered clean up (which we don't consider as "processed", it has a different meaning).
Maybe something more straightforward?
Like the following (outline, haven't tried to compile it):
val producer = produce {
var currentElement = 0
try {
while(true) {
send(currentElement++)
}
} finally {
println("Cancelled after: $currentElement")
}
}
consumeFirst { ... }
It's deterministic, more trivial and straightforward. WDYT?
There was a problem hiding this comment.
Yes, good point. The point of consume seems to be just notifying the producer that there are no more consumers. I noticed that consume is a reliable way not to miss any elements and became too fixated on that, but this really is a just side effect.
| * | ||
| * This function will attempt to receive elements and put them into the list until the channel is | ||
| * [closed][SendChannel.close]. | ||
| * Calling [toList] without closing the channel is always incorrect: |
There was a problem hiding this comment.
Let's make it a bit less ambigous? "Calling toList on channels that are not closed" or something like that
| * ``` | ||
| * val values = listOf(1, 5, 2, 9, 3, 3, 1) | ||
| * val channel = Channel<Int>() | ||
| * GlobalScope.launch { |
There was a problem hiding this comment.
Let's use produce for that?
Esp. here, where finally block is required otherwise
There was a problem hiding this comment.
I found produce too high-level and thought that using it could detract from the point of the sample. Is it wrong? Is produce common knowledge and part of any coroutine user's toolkit?
There was a problem hiding this comment.
produce is more high-level, but it's also the idiomatic way to create a producer coroutine, so it's worth mentioning. It's true that it may overwhelm the reader, but we can probably work around this by adding extensive comments.
| * Example of usage: | ||
| * ``` | ||
| * val callbackEventsStream = produce { | ||
| * val disposable = registerChannelInCallback(channel) | ||
| * awaitClose { disposable.dispose() } | ||
| * } | ||
| * ``` | ||
| * | ||
| * Internally, [awaitClose] is implemented using [SendChannel.invokeOnClose]. |
Co-authored-by: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
| * | ||
| * There is no way to recover these lost elements. | ||
| * If this is unsuitable, please create a [Channel] manually and pass the `onUndeliveredElement` callback to the | ||
| * constructor. |
There was a problem hiding this comment.
This example is irrelevant to produce documentation, it's Channel doc that should explain that
There was a problem hiding this comment.
I beg to differ. produce calls the Channel function, passing a null to onUndeliveredElement. This is produce's implementation detail (that we must document) and not the Channel's.
There was a problem hiding this comment.
Or do you mean the snippet below? It's not an example, it's an explanation of how to approximate the produce above with an explicitly created channel. Would it work for you if I explain this in text explicitly?
There was a problem hiding this comment.
I mean the snippet itself, yes. I would say it's sufficient to refer to onUndeliveredElement where it has more comprehensive explanation and self-sufficient example
There was a problem hiding this comment.
Non-obvious things this snippet highlights:
- Where the
onUndeliveredParameteris (you can't refer to it directly, the user has to chase the docs) finallywith theclosein the coroutine
There was a problem hiding this comment.
Where the onUndeliveredParameter is (you can't refer to it directly, the user has to chase the docs)
- "If this is unsuitable, please create a [Channel] manually and pass the
onUndeliveredElementcallback to the constructor: [Channel(onUndeliveredElement = ...)]]Channel]" probably will suffice.
I realize that using it might be less obvious, but still believe it's not the produce's documentation job to explain that
One of the pieces of #4133.
Each file can be reviewed independently (and changes to other files will be added later). I'm keeping this a single pull request to avoid spamming.