Fix #123: introduce Async.asyncF

[alexandru]

Fixes #123 — This PR adds a variant of the async builder that suspends side effects in the provided callback.

This is useful for eliminating the need for Effect in many cases where currently it would be required. As an example given in the ScalaDocs, currently this isn't possible without Effect:

  import cats.effect.concurrent.Ref

  type Callback[-] = Either[Throwable, A] => Unit

  class PurePromise[F[_], A](ref: Ref[F, Either[List[Callback[A]], A]])
    (implicit F: Async[F]) {

    def get: F[A] = F.asyncF { cb =>
      ref.modify {
        case current @ Right(result) =>
          (current, F.delay(cb(Right(result))))
        case Left(list) =>
          (Left(cb :: list), F.unit)
      }
    }

    def complete(value: A): F[Unit] =
      F.flatten(ref.modify {
        case Left(list) =>
          (Right(value), F.delay(list.foreach(_(Right(value)))))
        case right =>
          (right, F.unit)
      })
  }

This works for cancelable stuff too, behaving like F.suspend actually, except that the result needs to be signaled via the provided callback:

def delayed[F[_], A](thunk: => A)(implicit timer: Timer[F])(implicit F: Async[A]): F[A] =
  F.asyncF { cb =>
    timer.sleep(1.second) *> F.delay(cb(
      try Right(thunk) 
      catch { case NonFatal(e) => Left(e) }
    ))
  }

Gotcha: the returned F[Unit] cannot throw errors. If it does, the behavior is undefined — because the only way to signal errors is via the provided callback and we cannot call the callback without risking a contract violation or without expensive synchronization.

For IO the implementation will log the error via the global Logger and other implementations are supposed to log the error somewhere too. For IO in particular, we could signal the error via the provided callback, however we don't know if that piece of synchronization will stay there forever, plus I don't want to encourage unhealthy practices because IO behaves in a certain way. The callback provided by Async is not supposed to be called concurrently from multiple places, if it gets called like that then the behavior is undefined.

---

Other fixes:

• removed Async#shift completely, it was deprecated in 0.10 and now we are retiring it
• fixed obsolete cats.effect.implicits for getting syntax for our type classes (not sure if I did the right thing)

[codecov-io]

Codecov Report

Merging #248 into master will increase coverage by 0.13%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #248      +/-   ##
==========================================
+ Coverage   89.18%   89.31%   +0.13%     
==========================================
  Files          58       57       -1     
  Lines        1525     1544      +19     
  Branches      157      153       -4     
==========================================
+ Hits         1360     1379      +19     
  Misses        165      165

[SystemFw]

Nice :)

One question, that pure Promise above still can't be cancellable, right? We'd need a cancelableF for that.

[durban]

I'm sorry, I lost track of whether we allow auto-cancellable flatMap, but if yes, I'm not sure that bracket is correct. Namely, I think if it gets cancelled after the ref is set, but before use, then release doesn't run.

[alexandru]

@SystemFw for one cancelableF is hard to implement, because in case of asyncF we can simply ignore the generate value.

We can afford to do that, because in case of error we only need to report it, or otherwise we don't care about the Unit. This is used in the instances of the monad transformers as well, we ignore the outcome otherwise it would get really complicated with StateT, EitherT and so on, so for example we simply go from F[(S, Unit)] to F[Unit]or fromF[Either[L, Unit]]toF[Unit]. Which would no longer work for a cancelableF`.

Plus if we try really hard, we might discover that it isn't needed.

The def bracket example I gave is cancelable (and I just did a change to ensure that it is). The returned F[Unit] of the registration function can be asynchronous and for Concurrent it can be cancelable as well — I just pushed a law in ConcurrentLaws for it.

[alexandru]

@durban indeed it doesn't work, but not for the stated reason — what you're saying would work due to onCancelRaiseError. The problem is what happens afterwards.

I don't want to fix that now, so I've replaced the sample.

@SystemFw the new def delayed should give you a hint that the returned F[Unit] hooks into the internal cancelation mechanism, so if cancelable then the result can be canceled. As said, it's basically F.suspend, but with a callback.

[SystemFw]

@alexandru Nice, thank you :)

Might be worth a mention in the scaladoc perhaps?

[johnynek]

This looks nice.

[oleg-py]

I'd rather not have referential transparency-breaking methods as a syntax extension, at least as a default one.

On the other hand, I would definitely like utilities we have - like timeoutTo, not sure if there are many of those - available as syntax extensions.

And if I can ask for more, writing Sync[F].delay { ... } is a bit mouthful, I would very much appreciate having delay[F] { ... } as a global symbol (maybe under a separate import).

And ToBracketOps, we should definitely have that here-ish :)

[alexandru]

@oleg-py what syntax do we have in there that is breaking RT?

I’m not interested in adding extension methods that aren’t on these type-classes. The reason is that they are hard to maintain.

Notice that we did not exposed these until now and nobody cared 😉

Also Bracket isn’t a @typeclass so it doesn’t have syntax.

[oleg-py]

Err, sorry, I meant to say methods intended to be used near the "end of the world", like runCancelable.

[alexandru]

What’s wrong with it? 🙂

[oleg-py]

There's nothing wrong except for IDE autocomplete pollution :) (Concurrent)Effect are useful in advanced interop scenarios, and I tend to use them way less than delay or async, esp. since we won't even need it in upcoming fs2 version.

Cats is very good at giving a la carte imports for stuff that I want :)

---

Right, Bracket can't have @typeclass due to having more than one type parameter. I still believe the syntax for Bracket is quite important, more so than for ConcurrentEffect 🙂

[alexandru]

@SystemFw I've added mentions in ScalaDoc too.

[alexandru]

@oleg-py I cannot address your complaints on implicits in this PR. I don't know what the best practices are. Cats has cats.syntax for a la carte imports. Maybe we should do a cats.effect.syntax too.

But I don't want to bother with that and we can add it later.
The cats.effect.implicits package object has to contain everything anyway.

So either I fix the cats.effect.implicits package object right now, to include everything, or we drop it completely, because I personally don't want to get into syntax.

[oleg-py]

@alexandru fine by me. If we merge this soon, I can roll out a syntax PR/proposal quickly enough :)

[gvolpe]

LGTM 👍

Gotcha: the returned F[Unit] cannot throw errors

The only thing I failed to see is whether we want to reflect this in the laws?

[alexandru]

We can’t add laws for the behavior of that F[Unit] because it’s provided by the user and what happens next, the error reporting or signaling, will be implementation specific.