Prevent either block nesting to avoid type/try-catch mismatches

[lbialy]

Either blocks, based on boundaries, provide implicit evidence for safe jumps. One problematic case is when .ok() combinator is called on a fork (or inside of exception capturing scope that can swallow a Break), another is when nested boundaries provide evidences for mismatched cases:

val effectMatchingOuterEither: Either[Throwable, Unit] = Left(Exception("oh no"))
val effectMatchingInnerEither: Either[String, Int] = Right(3)

val outer: Either[Throwable, Unit] = either {
  val inner: Either[String, Int] = either {
    effectMatchingOuterEither.ok()
    effectMatchingInnerEither.ok()
  }

  ()
}

Notice that first Either value evaluated with .ok() matches outer either block while being evaluated in the inner block. This might very well be considered a feature - types allow us to decide where to jump - but in my short but stressful experience with this kind of combinators - when this is used with type inference, results can be surprising. In my case the inner block was the body of Tapir handler while outer either block was in main function. The result was, unsurprisingly, a thread killed with Break. I would get a compilation error if nested either blocks were not allowed because without the outer block I'd be informed that it's impossible to use .ok() as either block in tapir handler is inferred to be Either[Unit, String]. I know this is a rather heavy-handed change and there are possible use cases for nested either blocks (tagged jumps for example) but this is confusing enough imho to warrant such a change. I can easily imagine such a change to occur during refactoring of code where inner either block stops matching one of the eithers with .ok() in it's scope and outer either providing evidence where it's no longer handled correctly.

[Ichoran]

In my experience, once one is aware that this can happen, it's actually a nice feature. Normally, yes, you jump to the closest block. But you can get out farther if you need to. It's unusual--at least for me--to be so confused that I have both the type wrong and where to jump to wrong, but they accidentally align and work despite not being desired behavior.

The problem comes when you try to jump thread boundaries. So, from my experience at least, I think this avoidance of nesting is not desirable.

[lbialy]

In simplistic examples, sure. The problem I encountered was in a block in which tapir endpoints were also declared, either block was opened on top for Either[Throwable, Unit] and inferred type of endpoint handler was Either[Unit, String]. I had a function that had Either[Throwable, String] that I .ok()ed in handler and it caught the top-level either handler instead of the one inside of the handler so two things happened simultaneously: either blocks overlapped and there was a thread boundary that won't be covered by #147. Now disallowing nested either blocks won't fix this case completely and neither will #147 because you can always do:

either:
  val t = Thread(() => e.ok())
  t.start()

but I still feel it would prevent bugs coming from refactoring that are possible due to the automatic application of implicits.

[adamw]

Maybe reformulate a bit? Sth like:

Nesting of either blocks is not allowed as it's error prone, due to type inference. Consider extracting the nested either block to a separate function.

[adamw]

Looks good - let's try and see when we get complaints :) Do you think we should mention this in the docs?

[lbialy]

I guess we should with an example on how to refactor.

[adamw]

@lbialy Would you be able to add the docs as well? :)

[lbialy]

Yeah. Btw: I was thinking whether there should be a either.nested: block that would allow for nested either blocks (maybe either.unsafe?).

[adamw]

Yeah. Btw: I was thinking whether there should be a either.nested: block that would allow for nested either blocks (maybe either.unsafe?).

Let's be lazy here and wiat for the first user complaint that they would like such behavior ;)

[lbialy]

Ok, I did add docs and changed the error message to your suggestion.

[adamw]

this does not compile (as indicated by the CI failure) - you can't .ok() here as the boundary is String-error-typed, and returnsEither is Exception-error-typed.

[adamw]

Ok, one merged, couple more to go :) Thanks :)