chore: upgrade tapir to 1.2.10

[frekw]

Tests are currently failing with:

      Exception in thread "zio-fiber-25" java.util.concurrent.RejectedExecutionException: Unable to run zio.internal.FiberRuntime@16eeabe0
        at zio.Executor.submitOrThrow(Executor.scala:79)
        at zio.test.TestClock.default(TestClock.scala:408)
5 tests passed. 0 tests failed. 0 tests ignored.


    - ZIO Http - ZHttpAdapterSpec
      Exception in thread "zio-fiber-25" java.util.concurrent.RejectedExecutionException: Unable to run zio.internal.FiberRuntime@16eeabe0
        at zio.Executor.submitOrThrow(Executor.scala:79)
        at zio.test.TestClock.default(TestClock.scala:408)
Executed in 14 s 109 ms

[frekw]

Seems to happen whenever a test uses the SttpInterpreter. I'm at a loss and not quite sure how to continue debugging this. 🫣

[ghostdogpr]

By the way we're stuck from upgrading zio, see #1513 (which is why test3_js fails).

That error looks super weird indeed 🤔 We didn't have them in the other PR for zio 2.0.4 so it must be an issue either in tapir or in zio-http.

[frekw]

I suspect it's an issue in tapir/how tapir uses sttp. All tests pass, but then the suite itself fails. And it only happens if the SttpInterpreter has been used... But I suppose it could be something internal in zio-test as well.

Regardless, we'll have to wait for the zio 2.0.4-bug to be fixed due to an incompatibility in zio-test that seems to exist between the versions.

[ghostdogpr]

Why no scala 2.12? It seems supported: https://mvnrepository.com/artifact/dev.zio/zio-http

[frekw]

@ghostdogpr yup, but had to drop it on the Tapir side due to some visibility issues that only seems to affect 2.12 (and that I couldn't find any way around): softwaremill/tapir#2576

[frekw]

However, we might be able to re-add 2.12 support in Tapir once zio/zio-http#1859 is merged. 🤞

[frekw]

I guess we also need to disable the examples for 2.12 :/

[frekw]

I've been banging my head against the wall on this one again. An interesting finding is that test interceptor failure seems to work, any other test seems to result in the weird clock failures. So it seems like failure only happens when we progress further into the interpretor.

Also tried upgrading sttp to 3.8.5, but that forces us to move away from async-http-client (marked as deprecated due to lack of maintainer, although that seems to have changed since) to the native HttpClient in JDK 11, but doing so seems breaks all the tests with java.lang.IllegalArgumentException: Cannot decode: Missing 🥺

Right now I'm kind of hoping for a new release of sttp built towards 2.0.5 since there was some izumi-reflect issues with 2.0.4. But honestly it feels like a stretch that it would help :/

I'll try to get async-http-client undeprecated in sttp in the meanwhile.

[ghostdogpr]

Hmm I wanted to have a look but the project refuses to load in IntelliJ with this change. A simple sbt compile also fails. That is because the project defaults to 2.12 and zio-http is not available. The problem is that I think we can't change that because the sbt plugin requires 2.12. Not sure what's the way out of this 🤔

[ghostdogpr]

Maybe there should be 2 root modules, the regular one without the sbt plugin and we default to 2.13, and another one just for the sbt plugin (most people who load the project don't care about the plugin). But we need to make sure we publish everything exactly once.

[ghostdogpr]

Oh but I guess we can simply wait on the fix for zio/zio-http#1813 being released and tapir as well...

[ghostdogpr]

I debugged the test failure: you can understand what is going on by adding a breakpoint on the RejectedExecutionException constructor. What happens is that zio-http makes its own executor and calls onExecutor (see usingSharedThreadPool in NettyRuntime.scala). The problem is that somehow this executor gets closed at the end of the test, and the zio runtime attempts at running more things (some interruption in TestClock) on that executor, triggering the RejectedExecutionException.

[frekw]

Amazing find! I was unable to get debugging to work myself (in VSCode) but this definitely explains it.

Perhaps it's possible to work around that by extending the scope of the server?

OTOH, it seems a bit weird to me that what I guess is essentially work happening "outside" a request would get scheduled on zio-https executor. I wonder if that's intended behavior 🤔

I agree on 2.12, it would be better if we could get support for that across the board, so perhaps wait for a new version of zio-http and then I can see if I can readd support for 2.12 to Tapir.

[ghostdogpr]

Perhaps it's possible to work around that by extending the scope of the server?

OTOH, it seems a bit weird to me that what I guess is essentially work happening "outside" a request would get scheduled on zio-https executor. I wonder if that's intended behavior 🤔

Yeah that seems like a bug to me. Wrote this in Discord, not sure anyone will respond to that. I tried a couple things on our side but it didn't change anything.

[ghostdogpr]

Opened zio/zio-http#1896

[frekw]

Am still working on this, right now awaiting the next tapir release.

Unfortunately the changes in zio-http 0.0.4 means that we'll probably see a somewhat significant performance hit (more info here: softwaremill/tapir#2718) due to some differences in behavior between the request execution models of Tapir and zio-http 0.0.4. Theres an open issue in zio-http's issues, but depending on how it's resolved and how big of a difference it will make we might want to reevaluate how we integrate with zio-http going forward, given that it's likely the top option for ZIO users using Caliban... 😕

[frekw]

Added a ticket in zio-http to track the 2.12 support: zio/zio-http#2006

[frekw]

@ghostdogpr I've disabled examples/compilation for 2.12 until the issue above is resolved ☝️

While far from ideal my proposal would be that we go ahead and merge in that state since this (finally) fixes FiberRefs (and hence session) for zio-http on ZIO & Caliban 2.X, which is a significant blocker to upgrade to upgrade from 1.x right now, but then work to get 2.12 support back as soon as possible when the above issue is resolved.

[frekw]

The JDK8 failures is coming from Sttp using HttpTimeoutException introduced in Java 11. Not sure what to make of that.

https://github.com/softwaremill/sttp/blob/b901e9b1194650be36eaf9ea34452c2309ceba27/core/src/main/scalajvm/sttp/client3/SttpClientExceptionExtensions.scala#L21

[frekw]

That seems to have been added as part of this commit: softwaremill/sttp@21393d2

I suspect it's one of the test dependencies that's pulling it in.

[frekw]

So, I think the failing tests is due to Tapir pulling in sttp 2.8.9 in the Tapir sttp client. That version is incompatible with JDK8 due to its usage of HttpTimeoutException introduced in JDK11.

I think we should just drop JDK8. Oracle's active support was dropped a year ago.

[ghostdogpr]

Shall we keep that?

[frekw]

I really like to have it available, and since it ships with sbt it doesn't introduce any new deps either!