observableEither.tryCatch

[waynevanson]

Is there any purpose for this not being in the library for ObservableEither?

Would you accept a PR?

docs to help: https://rxjs.dev/api/operators/catchError

[waynevanson]

I reallly don't know much about RXJS and the whole porting process, but here is a non tested implementaiton:

import { either as E } from "fp-ts";
import { observable as OB, observableEither as OBE } from "fp-ts-rxjs";
import { catchError } from "rxjs/operators";
import { Lazy, pipe } from "fp-ts/lib/function";
import { Observable } from "rxjs";

export const tryCatch = <E, A>(
  lfa: Lazy<Observable<A>>,
  onErr: (e: unknown) => E
): OBE.ObservableEither<E, A> =>
  pipe(
    lfa(),
    OB.map(E.right),
    catchError((err, caught) =>
      pipe(
        caught,
        OB.map(() => pipe(err, onErr, E.left))
      )
    )
  );

[waynevanson]

An implementation that actually catches lol

import { either as E } from "fp-ts";
import { observable as OB, observableEither as OBE } from "fp-ts-rxjs";
import { catchError } from "rxjs/operators";
import { Lazy, pipe } from "fp-ts/lib/function";
import { Observable } from "rxjs";

export const tryCatchObservable = <E, A>(
  lfa: Lazy<Observable<A>>,
  onErr: (e: unknown) => E
): OBE.ObservableEither<E, A> =>
  pipe(
    lfa(),
    OB.map(E.right),
    catchError((err) => pipe(err, onErr, E.left, OB.of))
  );

[OliverJAsh]

For reference, this would mirror the behaviour we have in TaskEither: https://gcanti.github.io/fp-ts/modules/TaskEither.ts.html#trycatch.

I don't think the observable parameter needs to be lazy. Observables are already lazy until you subscribe to them. How about this signature then?

export declare function tryCatch<E, A>(f: Observable<A>, onError: (error: unknown) => E): ObservableEither<E, A>

I wonder if tryCatch should be curried and receive the Observable last, similar to ObservableEither.fromObservable. Although if we do that, we should also update TaskEither.tryCatch to use the same format.

FWIW, here's a version I wrote. It's slightly more complicated because we need to be careful not to catch errors that happen during the map:

import * as E from 'fp-ts/lib/Either';
import { flow, pipe } from 'fp-ts/lib/function';
import * as RxJS from 'rxjs';

/**
 * Like `promise.then(onFulfilled, onRejected)`. This is *not* the same as
 * `map(onNext).catchError(onError)`, because this operator will only catch errors resulting from
 * the original observable—not errors resulting from `onNext`.
 */
export const mapOrCatchError = <T, B>(
  onNext: (value: T) => B,
  onError: (value: unknown) => B,
): RxJS.OperatorFunction<T, B> => (ob$) =>
  new RxJS.Observable<B>((observer) =>
    ob$.subscribe({
      next: (t) => {
        // eslint-disable-next-line fp/no-let
        let next: B;
        try {
          next = onNext(t);
        } catch (error) {
          observer.error(error);
          return;
        }
        observer.next(next);
      },
      error: (error) => {
        // eslint-disable-next-line fp/no-let
        let next: B;
        try {
          next = onError(error);
        } catch (newError) {
          observer.error(newError);
          return;
        }
        observer.next(next);
        observer.complete();
      },
      complete: () => {
        observer.complete();
      },
    }),
  );

/**
 * Similar to https://gcanti.github.io/fp-ts/modules/TaskEither.ts.html#trycatch
 * https://github.com/gcanti/fp-ts-rxjs/issues/31
 */
export const tryCatch = <E, A>(
  onError: (e: unknown) => E,
): RxJS.OperatorFunction<A, E.Either<E, A>> => (ob$) =>
  pipe(
    ob$,
    mapOrCatchError(
      E.right,
      flow(
        onError,
        // We must wrap this to avoid inference issues
        (e) => E.left(e),
      ),
    ),
  );

[waynevanson]

I definitely prefer the piped API.

You are right with not needing lazy observable.

[anthonyjoeseph]

I don't understand the need for a mapOrCatchError function. It doesn't seem necessary for implementing OBE.tryCatch b/c E.right can never fail. Is there a case where I would usemapOrCatchError instead of E.tryCatch?

I love the piped OBE.tryCatch btw!

[Edit: here's an example, assuming a simple catchError version of OBE.tryCatch :

import { pipe } from 'fp-ts/function'
import * as E from 'fp-ts/Either'
import * as rf from 'rxjs/fetch'
import * as OB from 'fp-ts-rxjs/lib/Observable'
import * as OBE from 'fp-ts-rxjs/lib/ObservableEither'

const divByZero = (a: number) => a / 0 // will throw an exception
const handledError$: OBE.ObservableEither<Error, number> = pipe(
  rf.fromFetch('https://jsonplaceholder.typicode.com/todos/1'),
  OBE.tryCatch(E.toError),
  OB.map(E.chain((a) => E.tryCatch(() => divByZero(a.status), E.toError))),
)

My question is: Is there an advantage to mapOrCatchError over this solution?]

[OliverJAsh]

@anthonyjoeseph Imagine something like:

ob$.tryCatch(doSomethingWithError)

Now imagine that, for some reason, doSomethingWithError threw an exception E, unexpectedly.

If we didn't use mapOrCatchError, we would end up with an Observable of Either.left(E). This isn't ideal—we only wanted to catch errors originating from ob$, not errors originating from doSomethingWithError.

catchError would catch errors in ob$ and doSomethingWithError. We only want to catch errors in ob$.

[anthonyjoeseph]

Ah, ok I understand, thanks for explaining.

I'm not yet convinced that's desired behavior. In the case you're describing, there are now three rail-lines of output - an error and a success represented by Either, and an implicit error implied by Observable.

Perhaps a different type definition for mapOrCatchError could represent its failure cases explicitly with an Either:

export const mapOrCatchError = <NextError, ErrorError, T, B>(
  onNext: (value: T) => E.Either<NextError, B>,
  onError: (value: unknown) => E.Either<ErrorError, B>,
): RxJS.OperatorFunction<T, Either<NextError | ErrorError, B>>

Which would turn tryCatch into:

export const tryCatch = <ErrorError, E, A>(
  onError: (e: unknown) => E.Either<ErrorError, E>,
): RxJS.OperatorFunction<A, E.Either<ErrorError | E, A>>

But now, what if onError still throws an error? Should we wrap it in another Either? It's fail-able all the way down.

To me, the problem is that an Either will always be a leaky abstraction in languages with unchecked exceptions - you can never be 100% sure that you've captured all of your errors. Some people argue that this is a reason not to use Either. I disagree with that - I think it's best to make a hard rule of where the error boundary is so that we can push errors to the boundaries of the system.

I think it makes more sense to model it after Task and TaskEither - Task is treated as though it can never fail (source).

I believe TE.tryCatch intentionally models the behavior that mapOrCatchError avoids - a Task generated by TE.tryCatch can truly never fail.

I suppose the trade off is certainty - with mapOrCatchError, we are 100% sure that our ObservableEither's types match its values, but without it, we can be certain that our Observable will never fail, which I believe is the more useful certainty insofar as preventing runtime crashes is the goal.

I'm certainly open to debate here, though - is there a use case where it makes sense to propagate those errors separately?

[Edit: incorrect Either reference url]

[OliverJAsh]

a Task generated by TE.tryCatch can truly never fail.

Hmm, I'm not sure that's true. Consider this:

const lazyPromise = () => Promise.reject(1);
const doSomethingWithError = error => {
  // oops, unexpected exception
  throw new Error('foo');
};
const run = TaskEither.tryCatch(lazyPromise, doSomethingWithError)
run();
// => UnhandledPromiseRejectionWarning: Error: foo

If you wanted this to return a resolved promise of Either.left instead, you would have to rewrite TaskEither.tryCatch to use promise.then(a).catch(b) instead of promise.then(a, b).

My version of tryCatch that uses mapOrCatchError simply gives us parity with TaskEither.tryCatch, i.e. ob.mapOrCatchError(a, b) is identical to promise.then(a, b).

I think it's a good discussion to have, whether or not this is the right behaviour (for both ObservableEither and TaskEither), but before we do that I want to make sure we're on the same page.

[anthonyjoeseph]

Oh! You are totally right. Please forgive my rant - I hadn't even tried running it yet.

Thanks for the additional explanation - promise.then(a, b) vs promise.then(a).catch(b) is very clear to me now.

I totally agree with the need for a mapOrCatchError-like behavior. I have one last question - looking at the rxjs implementation & empirically from my own experiments, catchError already does something similar. Is there something I'm missing here?

[OliverJAsh]

You are right. Now I'm the one getting confused 😆

Now I remember, the reason I used mapOrCatchError was to give parity with promise.then(a, b), so that if a threw unexpectedly, we wouldn't catch the error.

As you said, a should never fail because (in this case) it's just E.right. In which case, these should be identical:

• promise.then(a, b) / ob.mapOrCatchError(a, b)
• promise.then(a).catch(b) / ob.map(a).catchError(b)

… which is to say: we shouldn't need to use mapOrCatchError, but doing so means that the code is consistent with TaskEither.tryCatch.

If we choose not to use mapOrCatchError, perhaps we could update TaskEither.tryCatch to use promise.then(a).catch(b) just so we have parity.

[anthonyjoeseph]

Sounds like they are logically equivalent, so either way makes sense to me!

Should this be a pr by now? Or is it generally best to get the approval of a maintainer first?

[OliverJAsh]

I think we can probably send a PR for it now it has been scoped out.

[anthonyjoeseph]

PR