Introduce migration path for long-standing issue of withTimeout

[qwwdfsad]

The current state of things:

• kotlinx.coroutines.withTimeout is considered harmful -- it throws CE where it should throw TE. Enough has been written on this topic, so I won't elaborate much. Our own book, Kotlin in Action, has a dedicated note (15.2.2) about this gotcha, and it already should be enough. Would be nice to avoid a situation like this
• kotlinx.coroutines.withTimeoutOrNull is perfectly fine.
• kotlinx.coroutines.time.withTimeout suffers from the same decease. The time subpackage is an unfortunate pick indicating that the package (previously -- separate JAR) is interoperable with java.time entities.

Additional data:

• On GH, withTimeout is twice as popular as withTimeoutOrNull (1 and 2)
• Also, it is not helping: the flow.timeout() operator (luckily, under @FlowPreview), but it is also harmful when played with coroutine-launching operators (basically, anything useable).

What are our options?

1. Breaking behavioural change. Out of the question.

2. Come up with a new name.
   This one comes with its own advantages and disadvantages, namely:
   Pros:

   • Much easier to migrate and distinguish for readers
   • We can safely change the semantics (i.e. introduce non-atomic cancellation)
   • Can play out nicely with the new withContext if we will introduce one

   Cons:

   • A weird state of withTimeoutOrNull. Either we have semantically-similar withTimeoutOrNull and __withDeadline or we sacrifice perfectly fine withTimeoutOrNull for __withDeadlineOrNull
     • Seven+ years of common knowledge ("recognition") down the drain

3. Reuse the same name. Originally, I had a plethora of sub-options, but the most reasonable seem to be the following:

   • Introduce kotlinx.coroutines.timeout.withTimeout version (yes, the package name for withTimeoutOrNull and withTimeout will be different)
   • Introduce @LowPriorityInOverloadResolution on the previous version(s) of withTimeout, pass it through the Beta/RC cycle. Consider marking it as Delicate?
   • Pull the trick similar to soft-deprecation of Enum.values() -- purely IDE-assisted replacement. Also, one more argument for KT-54106
   • When soft-deprecation is rolled into the major IJ/AS version, consider introducing real deprecation or @DelicateCoroutinesApi.

====

I picked the third option, and here is the draft.

If we agree that this is the way, I'll properly follow up on the PR -- documentation, guide, tests, flow operators, time subpackage, as well as proper migration path in the ticket with versioning (of coroutines and IDEA).

I would love to hear both @dkhalanskyjb stance on that as the maintainer and @SebastianAigner as the one who teaches people how to use coroutines.

[qwwdfsad]

One thing that is worth tweaking around is the interaction between the new and old API.

I.e. right now nothing prevents users from writing something like this:

try {
    kotlinx.coroutines.timeout.withTimeout(...) {}
} catch (e: TimeoutCancellationException) {

}

and it seems to be quite a pattern. Probably we should mark it as Delicate and eventually deprecate

[qwwdfsad]

Note to self: revisit #3716 when moving forward

[qwwdfsad]

Apart from the "reuse the same name" path, one of the other options I implicitly picked here is to start with LowPriority... and IDEA migration (in the next versions) with the potential @DelicateCoroutinesApi instead of the deprecation right off the bat. Here I'm overcautious about being too disruptive with the change, but I can probably be easily convinced otherwise.

[kevincianfarini]

Introduce kotlinx.coroutines.timeout.withTimeout version (yes, the package name for withTimeoutOrNull and withTimeout will be different)

I think it would be better to be working towards the next breaking change opportunity where there exists two versions of each function -- withTimeout and withTimeoutOrNull -- the original versions are deprecated and the new versions live in the same timeout package.

Yes, this is of course not strictly necessary for withTimeoutOrNull, but having withTimeout and withTimeoutOrNull living in separate packages whenever coroutines 2.0.0 occurs would be a weird vestige.

Considering withTimeout is 2x as popular as withTimeoutOrNull, this is likely to be disruptive change regardless. What do you think?

[qwwdfsad]

Yes, we can disrupt both withTimeout and withTimeoutOrNull simultaneously.

On the one hand, it's indeed more sane and convenient to have them in the same package. On the other, it's an IDE alt+enter-driven matter, and it feels really off to convey a message "this function is perfectly fine, but we would like to disturb you here for the sake of (minor?) inconsistency". I find it hard to justify such a disruption (note that it is different from withTimeout where the change is justified by a fact that we prevent the whole class of bugs); I'm open to alternative views on a problem though!

[kevincianfarini]

with the potential @DelicateCoroutinesApi instead of the deprecation right off the bat. Here I'm overcautious about being too disruptive with the change, but I can probably be easily convinced otherwise.

As a consumer of kotlinx-coroutines I'd like to attempt to convince you otherwise. My original comment in #1374 regarding marking the API as delicate was motivated by the lack of a proper solution. In essence, I was advocating that the maintainers should have guided people to use withTimeoutOrNull instead of withTimeout.

Since this change fixes the longstanding withTimeout issue, I think deprecation makes more sense. Opt-ins convey that something can be solved in a better way in many use cases, but there exist a few very small use cases where an escape hatch is required (eg. GlobalScope).

In contrast, the old withTimeout function isn't operating as an escape hatch -- it's flawed and is being replaced by something that doesn't experience the same flaw. For newly written code there exists no use case where someone should use the legacy withTimeout function over the kotlinx.coroutines.timeout.withTimeout function, and that seems like a pretty clear indicator that it should be a deprecation instead of an opt-in.

I'd also be worried that the legacy withTimeout function being guarded by an opt-in conveys the wrong message to library consumers -- that this function would be maintained longer than it's intended to. Deprecation properly conveys that this is flawed, is no longer maintained, and will be deleted at some point in the future. Don't use it.

Finally, I worry that it's pretty common for library consumers to do a blanket opt-in @DelicateCoroutinesApi for end uses like applications in KGP. In that scenario those library consumers wouldn't be notified of this change and the migration pathway wouldn't be as clear. Deprecation doesn't suffer from the same drawback.

[kevincianfarini]

On the other, it's an IDE alt+enter-driven matter, and it feels really off to convey a message "this function is perfectly fine, but we would like to disturb you here for the sake of (minor?) inconsistency".

This may not be a very strong argument per say, but can't the same alt+enter argument be made for replacing withTimeoutOrNull with kotlinx.coroutines.timeout.withTimeoutOrNull?

In the case of replacing withTimout, the functionality is actually different and rectifying that requires a possibly substantial PR. In contrast, replacing withTimeoutOrNull would be changing the import statement and that's it. How disruptive is that really when it won't be actually broken until 2.0.0? Presumably there will be a handful of actually substantive breaking changes in 2.0.0, like withTimeout and possibly reworking Deferred and async. In light of this, is changing the namespace of a function while also offering a migration path considered disruptive enough to prevent this?

I don't feel super strongly one way or the other, but just something I'd raise my eyebrow at when reviewing import statements in PRs.

[dkhalanskyjb]

Thoughts in no particular order.

• The details of how the IDE would be of help here are unclear to me. withTimeout(d. c) should be replaced with try { kotlinx.coroutines.timeout.withTimeout(d, c) } catch (e: TimeoutException) { throw TimeoutCancellationException(e) } to preserve the behavior, which should be achievable with the deprecation machinery, right? What more can the IDE do?
• "Years of recognition down the drain" for a new name can be a good thing, because this function has already discredited itself. We may want to start anew, with a clean reputation.
• If we keep the same name, add LowPriority to the old one, and don't deprecate it, we encounter this problem: once someone adds (copy-pastes from another file, accidentally adds an import, whatever) some code using the new withTimeout, the old code in the same file will silently start behaving differently (or the IDE will crash because it won't know how to keep both withTimeout versions). This is especially alarming because the change in behavior will be essentially impossible to spot during code review. Maybe the OptIns will become gray in the IDE, but that's all.

[joffrey-bion]

Thanks for this issue! It's great to see progress here 😍

I really like option 3, and I second @kevincianfarini regarding the deprecation being more appropriate than marking it as delicate, for exactly the reasons described.

I also believe aligning the package for withTimeoutOrNull would be worth the disruption (as a user). Maybe the deprecation for the old one can be soft for much longer, though.

"Years of recognition down the drain" for a new name can be a good thing, because this function has already discredited itself. We may want to start anew, with a clean reputation.

I see your point, and I agree that there are probably a bunch of materials out there warning people about withTimeout, which might make users wary about using the new withTimeout if we keep the name. I'm not too worried about it, though. Given the numbers we see - people are still using the broken function much more despite the warning materials. And at worst, they will use withTimeoutOrNull instead, which is not a bad thing.
On the other hand, the name really fits the purpose, and inventing a new name is likely to be slightly off / unsatisfying / incorrect (for instance, I saw something like "deadline" in the text, which is wrong if we're passing a Duration and not an Instant).

[dkhalanskyjb]

On the other hand, the name really fits the purpose, and inventing a new name is likely to be slightly off / unsatisfying / incorrect

We can't know that without considering other options. ChatGPT immediately proposed withTimeLimit, withMaxDuration, withTimeBound, and executeWithin, and I didn't even deliberate on the prompt at all. I don't see problems with withTimeLimit, for example, and executeWithin seems surprising but fresh: executeWithin(2.seconds) { } is unusual, yet makes sense.

So, I wouldn't reject the idea of a new name without giving it a fair try.

EDIT: timingOutAfter(2.seconds) { ... } came to my mind after a bit of thought. I don't like the with part anyway.

[SebastianAigner]

Just a few thoughts from my POV:

• I agree with Dima's point on a new name providing a fresh new start. It also prevents any confusion where people look for coroutine pitfalls, find some blog posts that mention the current behavior of withTimeout, and then get needlessly scared that they shouldn't write withTimeout in their own code, even after a fix was deployed (after all, people rarely pay attention to imports).

• It seems important that we end up with a consistent pairing of functions a la Xxx and XxxOrNull -- because this allows people to pattern match, and they'll discover the existence of either variant just via autocomplete. Having withTimeoutOrNull paired with a new withDeadline or something akin to it prevents that, so imo should be avoided.

• AI agents are going to use generate code using the existing withTimeout for the foreseeable future, and I'm inclined to believe they will also most likely continue to generate code to catch TimeoutCancellationException (even if they figure out that withTimeout was relocated to a different package). In the case of a soft-deprecation, out-of-band AI agents that generate entire PRs may also simply continue to use the old import if we were to simply rename it, and to a reviewer outside of the IDE (e.g. in GitHub code reviews), the mistake may be practically undiscoverable. A distinct new name and real deprecation of the withTimeout would help here.

All of this makes me lean towards introducing a new pair of functions, name tbd, and deprecating what we have so far.

[kevincianfarini]

Just to add to the chorus -- @dkhalanskyjb and @SebastianAigner have made great points regarding preserving the old withTimeout[OrNull] names. I think two new names would be more clear to library consumers. Might I suggest completeWithin(Duration) and completeWithinOrNull(Duration)?

[qwwdfsad]

The details of how the IDE would be of help here are unclear to me.

@dkhalanskyjb The key observation here was that IDE could do it in a less intrusive way -- instead of all withTimeout producing compilation warnings and being crossed out (as well as interfering with linters), IDE could help with the migration in a less intrusive manner.

Thanks for all the arguments!
Seems like introducing the new name for both would be a more reasonable and sound approach. The naming is (as usual) hard, so here we can start our contest for a new name for withTimeout! All ideas are welcome, so grab ChatGPT, tell your friends, shitpost on Twitter/Bsky and bring it here!

On a more serious note, I'll give the naming problem a go later this week

[SebastianAigner]

Out of the ones mentioned so far, withTimeLimit does resonate with me, but I'm certain there are other naming schemes that still unambiguously communicate what the function does (especially if we look beyond the withXxx naming convention -- like potentially runWithTimeout?).

[matt-ramotar]

Came here to suggest completeWithin. Seeing it has already been suggested. +1 to @kevincianfarini

[joffrey-bion]

We can't know that without considering other options. [...] So, I wouldn't reject the idea of a new name without giving it a fair try.

Fair enough, we can't really know without trying.

ChatGPT immediately proposed withTimeLimit, withMaxDuration, withTimeBound

All 3 of these IMO just feel like a convoluted way to talk about what we all know as a timeout, which was my point. Using other words to describe it is not wrong in itself, but when there is an established way to refer to something, it's more satisfying to use it. Maybe I'm overestimating how widespread the word timeout is, though, as this is just an intuition and nothing measured.

I have to admit that there are actually interesting options, though, especially if we explore the name from other angles. I mean other than "with + something that conveys the timeout idea worse than the word timeout".

I like how executeWithin(2.seconds) { ... } reads (although I would tend to prefer the verb "run" since we have it in other places in the stdlib). That said, I'm concerned that the absence of time notion in the name might make the function less discoverable.

I don't like the with part anyway.

I agree that with as a prefix doesn't seem to fit the general "bring into scope" idea of the with() or withContext() functions.
Wouldn't hurt as a middle man like in runWithTimeout, though.

I actually like runWithTimeout as it doesn't have this slightly weird with prefix, and is more aligned with the runBlocking name. We could go even further with runTimingOutAfter.

Anyway, all this to say that if we do find a better name than withTimeout, then we should jump on the opportunity, but if we don't, I think we shouldn't settle for a lesser name just to change the name.

[alexvanyo]

A slightly different path than the discussion so far: what if the newly introduced function is called withTimeoutOrThrow?

Then the existing withTimeout could be cleanly deprecated, and withTimeoutOrNull wouldn't have to change.

It does make all usages slightly more verbose, but perhaps that verbosity ends up being more explicit: suffixing with OrThrow helps indicate that there will be a thrown exception that either needs to be handled in some way or it will propagate, which could help reveal a logic error like the example in #1374. It allows keeping the timeout naming without needing to find a similar-meaning replacement, and it also avoids the issue of having multiple withTimeout functions that do different things.

[qwwdfsad]

@alexvanyo that's a fresh line of thinking, thanks!

withTimeoutOrThrow does not seem to be aligned with the overall naming scheme in Kotlin, because it's usually "the action OR throw if the action failed" -- getOrThrow, decodeOrThrow, maxOrThrow etc. withTimeoutOrThrow might convey slightly different message here, but I might misunderstand the nuances of the (English) language here.

[alexvanyo]

Given withTimeoutOrNull already exists for the case of trying something, or returning null, I don't think that a withTimeoutOrThrow which throws instead of returning null would be out of place.

This form is sort of inspired by the Result methods - maybe there would be a good way to use those directly by having an intermediate Result? Maybe something like runSuspendCatching that takes a timeout duration? That then directly overlaps with #1814 which would make the solution more complicated.

[SebastianAigner]

A summary of views we've collected across various social media posts (Twitter, Bluesky, LinkedIn):

Potential names:

• runWithTimeout
• withTimeLimit / runWithLimit / runWithTimeLimit
• withTimeoutException
• withExceptionOnTimeout
• timeBox / timeBound / boundToTimeLimit
• runTimeout
• limitTime/ timeLimited
• runFor / runUntil / runAtMost
• autoCancel / cancelAt / cancelAfter / cancelingAfter
• withDeadline
• withDuration / withExpiration
• expiringIn / expireIn / expireAfter

And, for some light hearted personal favorites: gottaGoFasterThan, everythingMustGoInTheNext, withTimeIsMoney.

Someone also pointed out correctly that most likely, the naming for the new exception should follow a consistent naming scheme (i.e. withTimeLimit should throw a TimeLimitException), so just throwing that in here so that we keep it in mind.

(From these, my personal favorite remains runWithTimeout)

[joffrey-bion]

Thanks for this recap @SebastianAigner !

Also, I'd like to reiterate that IMO withDeadline would be an incorrect name if the argument is a duration. A deadline is a moment in time, not a duration. If we have a timeout of 10s, the deadline is "the instant 10s from now".

[qwwdfsad]

Tentatively, we have a winner!
withTimeoutOrThrow is what we decided to use as our primary name for the prototype, so later I'll create a PR with that and with the reasoning