Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: switch to vertx server backend #223

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

geminicaprograms
Copy link
Contributor

@geminicaprograms geminicaprograms commented Oct 12, 2022

There issue with non-deterministic EOF is observable by others but so far there is no good explanation (or test method) given (see #668) therefore for the time being we are switching to vertx http server.

TODO:

  • OPTIONS request
    curl -v 'http://localhost:9090/api/v1/content' \
    -X 'OPTIONS' \
    -H 'Accept: */*' \
    -H 'Accept-Language: en-GB,en-US;q=0.9,en;q=0.8' \
    -H 'Access-Control-Request-Headers: content-type' \
    -H 'Access-Control-Request-Method: POST' \
    -H 'Connection: keep-alive' \
    -H 'Origin: http://localhost:3000' \
    -H 'Referer: http://localhost:3000/' \
    -H 'Sec-Fetch-Dest: empty' \
    -H 'Sec-Fetch-Mode: cors' \
    -H 'Sec-Fetch-Site: same-site' \
    -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36' \
    --compressed
    is not accepted and results in 405 Method Not Allowed therefore UI is not able to talk to the server but calling curl
    curl -v 'http://localhost:9090/api/v1/content' \
      -H 'Referer: http://localhost:3000/' \
      -H 'Sec-Fetch-Dest: empty' \
      -H 'Sec-Fetch-Mode: cors' \
      -H 'Sec-Fetch-Site: same-site' \
      -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36' \
      -H 'sec-ch-ua: "Chromium";v="106", "Google Chrome";v="106", "Not;A=Brand";v="99"' \
      --data-raw '{"addDocumentation":false,"addMetrics":false,"projectName":"clear-buzzard","groupId":"com.softwaremill","json":"No","scalaVersion":"Scala3","builder":"Sbt","effect":"IOEffect","implementation":"Http4s"}' \
      --compressed
  • at the moment one cannot customise the logs hence they are not displayed when the server is started

Verifies #49

There issue with non-deterministic EOF is observable by others but so far
there is no good explanation (or test method) given (see [1]) therefore
for the time being we are switching to vertx http server.

TODO:
* `OPTIONS` request is not accepted and results in `405 Method Not
  Allowed` therefore UI is not able to talk to the server but calling
  `curl` works
* at the moment one cannot customise the logs hence they are not
  displayed when the server is started

Verifies #49

[1] http4s/blaze#668
Comment on lines +18 to 32
Dispatcher[IO]
.use { dispatcher =>
{
Dependencies
.wire(config)
.use { case Dependencies(httpApi) =>
/*
allocates the http api resource, and never releases it (so that the http server is available
as long as our application runs)
*/
httpApi.resource(dispatcher).useForever
}
}
}
.unsafeRunSync()
Copy link
Contributor Author

@geminicaprograms geminicaprograms Oct 12, 2022

Choose a reason for hiding this comment

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

Current approach is based on Vert.x example
Alternatively for comprehension can be used but it is hard to tell if it is more readable ;)

  (for {
      dispatcher <- Dispatcher[IO]
      servers <- Dependencies
        .wire(config)
        .map { case Dependencies(httpApi) =>
          /*
        allocates the http api resource, and never releases it (so that the http server is available
        as long as our application runs)
           */
          httpApi.resource(dispatcher).useForever.unsafeRunSync()
        }
    } yield (dispatcher, servers)).useForever.unsafeRunSync()

@adamw WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can make it more readable :)

for {
  dispatcher <- Dispatcher[IO]
  httpApi <- Dependencies.wire(config)
  servers <- httpApi.resource(dispatcher)
  _ <- Resource.never
} yield whatever

.prependInterceptor(CorrelationIdInterceptor)
// all errors are formatted as json, and there are no other additional http4s routes
.defaultHandlers(msg => ValuedEndpointOutput(http.jsonErrorOutOutput, Error_OUT(msg)), notFoundWhenRejected = true)
// TODO customise the serverLog when available
Copy link
Member

Choose a reason for hiding this comment

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

fixed in latest tapir

def resource(dispatcher: Dispatcher[IO], endpoints: List[ServerEndpoint[Any with Fs2Streams[IO], IO]], port: Int) = {
Resource.make(
IO.delay {
val vertx = Vertx.vertx()
Copy link
Member

Choose a reason for hiding this comment

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

I think we should crate a single shared Vertx

@adamw
Copy link
Member

adamw commented Oct 17, 2022

We should check the OPTIONS request using a test in tapir itself - and if it's an issue, fix it there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants