CatchIf - predicate that is always true does not Exclude<...> error types correctly

[akomm]

What version of Effect is running?

3.11.6

What steps can reproduce the bug?

Code is based on Effect-TS Example for catchif

Open the code below in TS-Playground

import { Effect, Random } from "effect"

class HttpError {
  readonly _tag = "HttpError"
}

class ValidationError {
  readonly _tag = "ValidationError"
}

//      ┌─── Effect<string, HttpError | ValidationError, never>
//      ▼
const program = Effect.gen(function* () {
  const n1 = yield* Random.next
  const n2 = yield* Random.next
  if (n1 < 0.5) {
    yield* Effect.fail(new HttpError())
  }
  if (n2 < 0.5) {
    yield* Effect.fail(new ValidationError())
  }
  return "some result"
})

//      ┌─── should be Effect<string, never, never> but is Effect<string, HttpError | ValidationError, never>
//      ▼
const recovered = program.pipe(
//    ^?
  Effect.catchIf(
    (error) => error._tag === "HttpError" || error._tag === "ValidationError",
    () => Effect.succeed("Recovering from HttpError")
  )
)

What is the expected behavior?

The inferred type of const recovered should be Effect<string, never, never>

What do you see instead?

Effect<string, HttpError | ValidationError, never>

Additional information

When a function does not narrow down the input, its not inferred as a type predicate: microsoft/TypeScript#60741 (comment)

This cause non-linear behavior in catchIf.

I know that if statically known at implementation time, I could use catchTags or catchAll.

But when the predicate changes based on external input and the implementation is independent of it, the input change affecting the predicate will require a change in the implementation. It is unexpected behavior and need to be tracked down.

Here is a suggestion adding an overload for this case: microsoft/TypeScript#60741 (comment)

[akomm]

Based on some new finding, it might actually be impossible to reliable solve this on Effect-TS side: microsoft/TypeScript#60741 (comment)

[mikearnaldi]

Unfortunately in cases where the check doesn't reduce the input TS doesn't infer a type predicate, unsure about the choice on the TS side but we have no control over it

[mikearnaldi]

The only thing we could do is to not use predicates in the first place:

export const skip: unique symbol = Symbol.for("....")
export type skip = typeof skip

export declare const catchIf2: {
  <E, X, A2, E2, R2>(
    refinement: (e: E) => X | skip,
    f: (e: Exclude<NoInfer<X>, skip>) => Effect<A2, E2, R2>
  ): <A, R>(self: Effect<A, E, R>) => Effect<A2 | A, E2 | Exclude<X, skip>, R2 | R>
}

used like:

const recovered = program.pipe(
  Effect.catchIf2(
    (error) => error._tag === "HttpError" || error._tag === "ValidationError" ? Effect.skip : error,
    () => Effect.succeed("Recovering from HttpError")
  )
)

would do the job.

This is somehow related to #3318

[akomm]

I've been using this skip approach in own code, outside of effect-ts, for a while. But it was always a bit odd. It is basically a filter & map in one function. Making restricting on type level felt a bit silly, because a filter and filterMap would be the same code, with a different signature. And using constraints like <A, B extends A> would only make sense if A is a union and B is a subset of that union, but the relation B extends A can be outside just union and then its a map again.

[mikearnaldi]

I've been using this skip approach in own code, outside of effect-ts, for a while. But it was always a bit odd. It is basically a filter & map in one function. Making restricting on type level felt a bit silly, because a filter and filterMap would be the same code, with a different signature. And using constraints like <A, B extends A> would only make sense if A is a union and B is a subset of that union, but the relation B extends A can be outside just union and then its a map again.

unifying filter & filterMap is probably a pro not a cons, even in this case we could filter&map the error if we want to, not sure how I feel about it we need to give it a proper test in 4.0's early releases to gather feedback