[Pitch] Typed throws in the Concurrency module

Hey everyone,

In the latest typed throws pitch, the adoption of typed throws in the standard library was removed due to the pitch growing quite large by itself and the large surface area of both the Swift and Concurrency module that might adopt typed throws.

This pitch covers the adoption of typed throws inside the Concurrency module. There are quite a few nice improvements that typed throws enable w.r.t the expressivity of task groups and tasks.

You can find the full proposal here.

Looking forward to comments and questions!

Franz

28 Likes

Hi Franz, looks great overall! A few comments below:

  1. For AsyncSequences, I really like that this would be coupled with adding primary associated types – I think many people are waiting for those :+1:
  2. Adding the next2 method, and defaulting both next and next2 to call the other unfortunately would mean that you could implement an async sequence without either, and it'd compile. Furthermore, the compiler wouldn't produce errors + fixits, so it won't be as easy to just let Xcode help you fill in the missing methods. Not sure what the solution is here, without some special magic that would make next2 the new required method, but the compiler would synthesize the next shim only if next2 is implemented (but not if next2 isn't implemented, then you'd still get nice fixits and couldn't write a type without at least one next method).
  3. Since the transformation methods on AsyncSequence force the same error type as the root sequence, should we add mapError that allows you to map the type + value of the error as well? I imagine it'd be hard to pass around async sequences in a diverse codebase without it, it's unlikely all the async sequences you want to eventually chain would share the same error type.
  4. I also really like adding the @discardableResult attribute only on the specific Task types, definitely a nice quality-of-life improvement.
6 Likes

Not being able to backport the typed transformations is unfortunate. Is there any thing the user could do to help fill there? Would they be able to shadow the declarations locally? Anything swift-async-algorithms could do?

On this note:

The newly proposed transformational AsyncSequence methods restrict the thrown typed of the closures to the Failure type of the base AsyncSequence . The closures could throw different error types; however, that would require error a union like type for the error.

This may deserve more exploration, as this marks AsyncSequence's move into full parity with older streaming libraries. When using Combine, for example, we often used an operator built on setFailureType(to:) operator (such as eraseError()) to erase the error type in a stream. Perhaps something like that could be added? Additionally, it might be beneficial to add a mapError transform to allow recovery of error types from within the stream itself, as well as manual production of the union type mentioned, if the user wants. I think both of these are doable as part of this proposal. A third addition, somewhat larger, is to offer overloads of the base transforms that allow you to transform any incoming error value and type alongside the value transformation.

4 Likes

Yes, that's something we have to figure out. We need a two-way default implementation here since we need existing code to bridge into the new method and the other way around as well. We have to look if there is something we can do in the compiler for this.

This has come up during the discussions of this proposal and it is definitely something that is important. For now we had deferred it for the future but it might be something worth pulling into this proposal.

This is interesting but it brings up a larger problem and I personally don't want swift-async-algorithms to become a back deploy polypill for some of the standard library types but I understand the concern.

AsyncSequence iterators can throw during iteration, as described by the throws on the next()operation on async iterators:

public protocol AsyncIteratorProtocol {
  associatedtype Element mutating func next() async throws -> Element?
}

Currently, untyped throws allows an AsyncIterator to throw CancellationError from next(). Is that now specifically disallowed?

3 Likes

The implication of typed throws on AsyncSequence are quite big when it comes to the throwing errors and it will require some work in the current AsyncSequence implementations. This is mostly a concern for the current generic source asynchronous sequences such as AsyncStream and AsyncChannel. Does should not throw a CancellationError and just return nil.

It does bring up an important point though which is that as a user you cannot differentiate any more between an AsyncSequence terminating normally i.e. returning nil or terminating due to cancellation. Since both now return nil. Even checking for Task.isCancelled after the next() method returned nil won't give you a definitive answer if the nil was due to cancellation since cancellation is happening from a different thread.

If it is important for users to know the difference between normal termination and task cancellation then the source asynchronous sequence should surface the CancellationError this can be done by either using any Error as the Failure type or by creating a use-case specific failure type.

FWIW, both AsyncStream and AsyncChannel are currently not throwing CancellationErrors. The only places in the Concurrency module that throw CancellationErrors are checkCancellation() and the sleep() methods.

1 Like

Yeah I think it should happen together with adding typed throws to AsyncSequence, i.e. in this proposal. Without it the ergonomics of using multiple sequences with different error types get pretty bad.

3 Likes

If you have an AsyncSequence chained to another AsyncSequence, then that intermediate sequence must also propagate the CancellationError, or else the tail sequence will not report cancellation.

It seems like this formalization is exposing a corner case of the existing API design.

I think the better alternative would be to add a wasCancelled property to AsyncSequence and have its iterator’s next() method set that flag to true when Task.isCancelled returns true, and/or in the case of a chained sequence, when the previous sequence’s wasCancelled property returns true. This would require some way for the iterator to communicate cancellation back to the sequence that outside parties could not also use.

1 Like

This propagation is mostly automatic since the transformation AsyncSequences such as map are just calling next on the base. If that base is throwing a CancellationError they will automatically propagate it. I don't see any reason for adding wasCancelled to the iterator. Could you please expand on why you see a necessity for it.

Because once next() becomes typed, all sequences in the chain must have typealias Failure = any Error or else any one of them will swallow the cancellation and turn it into nil.

The fact that this happens today is an artifact of AsyncStream’s implementation, but with this change it would become enforced by the type system.

1 Like

Most transformative AsyncSequences are just going to forward the error that they receive from their base e.g. their Failure type will be constrained to Base.Failure. Which means they will just forward CancellationErrors. Now there is a set of transformative AsyncSequences that do change the error e.g. a potential mapError. Those need to decide what they do if they get a CancellationError and for probably all of them this is up to the user.

I don't expect that we would have any AsyncSequence in either the Concurrency module nor swift-async-algorithms that would just swallow CancellationErrors.

I don't think there are any explicitly throwing CancellationError instances in AsyncStream or even AsyncThrowingStream today.

1 Like

On a completely different topic: if AsyncThrowingStream is subsuming the role of AsyncStream, is it possible to hide the old AsyncStream and replace it with AsyncThrowingStream in the surface language? Something like:

#if $TypedThrows
typealias AsyncStream = AsyncThrowingStream where Failure == Never

@available(*, unavailable)
protocol AsyncStream {
  …
}
2 Likes

We can’t drop the current AsyncStream type since it is already baked into API and ABI. Furthermore, it also has a different signature than AsyncThrowingStream which has two generic parameters. What we can probably do is deprecate AsyncStream in the long run. I personally think we should do this in stages and not with this pitch.

That’s why I’m suggesting marking it unavailable (and I guess deprecated) and providing a type alias. That should only be visible to the client without affecting the ABI.

Can?

Thanks for the call out. I meant to write can’t.

It's not obvious to me, and I didn't see it discussed in the proposal, why we need next2() on AsyncIteratorProtocol? Why is adding the typed throw incompatible with the previous @rethrows behavior?

It would because we do handle such situations already in the compiler. The Executor/enqueue method is a recent example of that, and Hashable also. The introduction of a new next method can be made pretty seamless if we needed it to be. I.e. deprecating the old and recommending the new etc.

@ktoso Nice! Is that special cased in the compiler for each type, or is that another use of @_disfavoredOverload?