Skip to content

Add ApplicativeError#voidError - #4324

Merged
johnynek merged 4 commits into
typelevel:mainfrom
armanbilge:feature/void-error
Oct 31, 2022
Merged

Add ApplicativeError#voidError#4324
johnynek merged 4 commits into
typelevel:mainfrom
armanbilge:feature/void-error

Conversation

@armanbilge

Copy link
Copy Markdown
Member

@armanbilge armanbilge added this to the 2.9.0 milestone Oct 27, 2022
Comment on lines +107 to +112
/**
* Void any error, by mapping it to `Unit`.
*
* @see [[handleError]] to map to an `A` value instead of `Unit`.
*/
def voidError[A](fa: F[A]): F[Unit] = handleError(void(fa))(_ => ())

@armanbilge armanbilge Oct 27, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively this could be def voidError(fu: F[Unit]): F[Unit] which may be used like fa.void.voidError. For some reason I thought that was awkward, but looking at it again it seems reasonable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I guess I should use Function.const here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of like taking F[Unit] since if we have to call void on it then when A = Unit we are doing wasteful work.

It's a kind of false generalization. We can only accept any A by voiding so why not have the caller void.

@johnynek johnynek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems useful but maybe we could expand the comment about the motivation (since ignoring errors is often an anti-pattern).

armanbilge and others added 2 commits October 27, 2022 03:06
Co-authored-by: Daniel Spiewak <djspiewak@gmail.com>
Comment on lines +110 to +121
* This is useful when errors are reported via a side-channel but not directly handled.
* For example in Cats Effect:
*
* {{{
* IO.deferred[OutcomeIO[A]].flatMap { oc =>
* ioa.guaranteeCase(oc.complete(_).void).void.voidError.start
* // ...
* }
* }}}
*
* Without the `.voidError`, the Cats Effect runtime would consider an error in `ioa` to be
* unhandled and elevate it to [[scala.concurrent.ExecutionContext.reportFailure ExecutionContext#reportFailure]].

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnynek wdyt?

def laws: ApplicativeLaws[F]

private def makeEqFUnit[A](a: A)(implicit EqFA: Eq[F[A]]): Eq[F[Unit]] =
protected[discipline] def makeEqFUnit[A](a: A)(implicit EqFA: Eq[F[A]]): Eq[F[Unit]] =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So by changing it from private to protected[discipline] you're making it accessible not only for sub-classes but also for everyone in discipline – is that correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, hm. Maybe that should be private[discipline]. The goal is to have it available for us, but not public.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected[discipline] def makeEqFUnit[A](a: A)(implicit EqFA: Eq[F[A]]): Eq[F[Unit]] =
private[discipline] def makeEqFUnit[A](a: A)(implicit EqFA: Eq[F[A]]): Eq[F[Unit]] =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I have no objections in regards to the PR, but I'm sort of curious about the initial implementation there:

def makeEqFUnit[A](a: A)(implicit EqFA: Eq[F[A]]): Eq[F[Unit]] =
    Eq.by(fa => laws.F.as(fa, a))

What could be a rationale of making Eq[F[Unit]] from an arbitrary a: A value?
Perhaps, I'm missing something, but it seems like for two different a it will produce two F[Unit] (even for the same F type) that would be considered as unequal to each other. Is that correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What could be a rationale of making Eq[F[Unit]] from an arbitrary a: A value?

We would normally request an Eq[F[Unit]] as an implicit parameter, but we cannot do so without breaking binary-compatibility.

Perhaps, I'm missing something, but it seems like for two different a it will produce two F[Unit] (even for the same F type) that would be considered as unequal to each other. Is that correct?

That is correct, this is just a workaround when you have a constant a.

@armanbilge
armanbilge requested a review from djspiewak October 27, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a combinator which eats unhandled errors

3 participants