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

Request hanging on multipartBody file uploads with zio-http #2518

Closed
aleksandr-vin opened this issue Oct 18, 2022 · 16 comments · Fixed by #3690
Closed

Request hanging on multipartBody file uploads with zio-http #2518

aleksandr-vin opened this issue Oct 18, 2022 · 16 comments · Fixed by #3690

Comments

@aleksandr-vin
Copy link

aleksandr-vin commented Oct 18, 2022

Tapir version: 1.1.3

Scala version: 2.13.10

I'm defining the endpoint and server logic like this:

  import sttp.tapir.ztapir._
  import sttp.tapir.generic.auto._
  import io.circe.generic.auto._
  import sttp.tapir.json.circe._

  case class UploadForm(file: Part[TapirFile])

  val uploadEndpoint: PublicEndpoint[
    UploadForm,
    String,
    String,
    Any
  ] = endpoint
    .post
    .in("files")
    .in(multipartBody[UploadForm])
    .errorOut(stringBody)
    .out(stringBody)
    .name("test files endpoint here")

  val uploadServerEndpoint =
    uploadEndpoint
      .zServerLogic[Any] { x =>
        (for {
          _ <- ZIO.succeed(logger.info(s">>>>>>>>>>>>> $x"))
        } yield "ooooops")
      }

then making zio-http server:

    val zserverLog: ServerLog[Task] = DefaultServerLog(
        doLogWhenReceived = { s => ZIO.succeed(logger.info(s">>> $s")) },
        doLogWhenHandled = { case (s, e) => ZIO.succeed(logger.info(s">>> $s: $e")) },
        doLogAllDecodeFailures = { case (s, e) => ZIO.succeed(logger.info(s">>> $s: $e")) },
        doLogExceptions = { case (s, e) => ZIO.succeed(logger.info(s">>> $s: $e")) },
        noLog = ZIO.succeed(logger.info(".........................")),
        logWhenReceived = true,
        logWhenHandled = true,
        logAllDecodeFailures = true
      )

      val zserverOpts = ZioHttpServerOptions
        .customiseInterceptors
        .serverLog(log = zserverLog)
        .options

      val fsR = ZioHttpInterpreter(zserverOpts).toHttp(List(uploadServerEndpoint))

     Unsafe.unsafe(implicit u =>
        runtime
          .unsafe
          .runToFuture(
            Server.start(8081, fsR)
          )
      )

Then starting the server and uploading a file with curl command:

% curl http://localhost:8081/files \
     -X POST \
     -F '[email protected]' \
     -v --max-time 10 --no-keepalive
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:8081...
* Connected to localhost (127.0.0.1) port 8081 (#0)
> POST /files HTTP/1.1
> Host: localhost:8081
> User-Agent: curl/7.79.1
> Accept: */*
> Content-Length: 12006
> Content-Type: multipart/form-data; boundary=------------------------7022269a28a25ad5
>
* We are completely uploaded and fine
* Operation timed out after 10005 milliseconds with 0 bytes received
* Closing connection 0
curl: (28) Operation timed out after 10005 milliseconds with 0 bytes received
%

Server logs only this:

[info] INFO  [nioEventLoopGroup-2-3] - >>> Request received: POST /files

And curl times out not receiving anything.

@aleksandr-vin
Copy link
Author

Checked with adopt-tapir's generated projects with ZIO and zio-http vs http4s server implementations, replacing Endpoints.scala with:

package com.softwaremill

import sttp.tapir._

import sttp.tapir.ztapir.ZServerEndpoint
import zio.ZIO
import sttp.model.Part
import sttp.tapir.generic.auto._

object Endpoints {
  case class UploadForm(file: Part[TapirFile])
  val helloEndpoint: PublicEndpoint[UploadForm, Unit, String, Any] = endpoint.post
    .in("hello")
    .in(multipartBody[UploadForm])
    .out(stringBody)
  val helloServerEndpoint: ZServerEndpoint[Any, Any] = helloEndpoint.serverLogicSuccess(user => ZIO.succeed(s"Hello $user"))

  val apiEndpoints: List[ZServerEndpoint[Any, Any]] = List(helloServerEndpoint)

  val all: List[ZServerEndpoint[Any, Any]] = apiEndpoints
}

the zio-http version hangs:

% curl -v http://localhost:8080/hello -X POST -F '[email protected]'
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /hello HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.79.1
> Accept: */*
> Content-Length: 869
> Content-Type: multipart/form-data; boundary=------------------------446b122b8c71f149
> 
* We are completely uploaded and fine
* Empty reply from server
* Closing connection 0
curl: (52) Empty reply from server

while http4s version runs ok:

curl -v http://localhost:8080/hello -X POST -F '[email protected]'
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /hello HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.79.1
> Accept: */*
> Content-Length: 995
> Content-Type: multipart/form-data; boundary=------------------------fe5f85d3ef7d8ac5
> 
* We are completely uploaded and fine
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: text/plain; charset=UTF-8
< Date: Tue, 18 Oct 2022 22:15:40 GMT
< Content-Length: 245
< 
* Connection #0 to host localhost left intact
Hello UploadForm(Part(file,/var/folders/tn/grglkq4521sgkw60t257zmw00000gq/T/tapir12134154041590495837tmp,Map(filename -> build.sbt),List(Content-Disposition: form-data; name="file"; filename="build.sbt", Content-Type: application/octet-stream)))%       

@adamw
Copy link
Member

adamw commented Nov 2, 2022

That's probably because multipart isn't supported by the zio-http interpreter: https://github.com/softwaremill/tapir/blob/master/server/zio-http-server/src/main/scala/sttp/tapir/server/ziohttp/ZioHttpRequestBody.scala#L28

I think there was some missing functionality in zio-http which prevented us from implementing this, but maybe it's possible now. Either way, I'll change this to throw an exception instead of silently failing.

@aleksandr-vin
Copy link
Author

Exception is good as we can test for it in our test suites and know when support will be added :)

Thanks

@Pask423 Pask423 self-assigned this Nov 7, 2022
@Pask423
Copy link
Contributor

Pask423 commented Nov 7, 2022

It seems that ZIO-HTTP support for multipart is still work in progress - zio/zio-http#1617, so we will have to wait for now.

@akizminet
Copy link

It seems that ZIO-HTTP supports multipart/form-data in 0.0.5. There is an example in zio/zio-http#2037

@adamw
Copy link
Member

adamw commented Mar 14, 2023

@akizminet Awesome :) Now we need somebody to volunteer to add multipart support to tapir+zio :)

@seakayone
Copy link
Contributor

I would like to volunteer:
feat: Add zio-http multipart body support #3690

Currently this PR is in Draft because:

  1. I have yet to test my code
  2. The PR does not have any tests (yet)

@adamw
Copy link
Member

adamw commented Apr 16, 2024

@seakayone Awesome! There are some multipart tests which can be enabled for the zio http interpreter: https://github.com/softwaremill/tapir/blob/master/server/zio-http-server/src/test/scala/sttp/tapir/server/ziohttp/ZioHttpServerTest.scala#L263, so this might give some guidance

@seakayone
Copy link
Contributor

Thanks @adamw I have activated the tests and am currently struggling with zio-http.

The incoming ServerRequest contains the multiple parts, but when calling zio.http.Request.body.[asMultipartFormStream|asMultiPartForm] they only ever return the single, first part. Will need to look into it further, might take some time.

@ipetkovic
Copy link

@seakayone Any news on this perhaps?
This became relevant for me as well since I would like to switch from play http-server to zio-http.
do you need help maybe?

@seakayone
Copy link
Contributor

My PR for tapir cannot be merged as there is bug in zio-http zio/zio-http#2411 which I haven't been able to fix.

@Pask423 Pask423 removed their assignment May 21, 2024
@godspeedelbow
Copy link

Any news on this? zio/zio-http#2411 got merged

@lmlynik
Copy link

lmlynik commented Jul 4, 2024

Hi, same question, is somebody actively working on this?

@adamw
Copy link
Member

adamw commented Jul 4, 2024

@lmlynik There's an old PR (#3690) but I don't recall other efforts

@seakayone
Copy link
Contributor

seakayone commented Jul 10, 2024

I am planning to finalize the PR (#3690) in the next couple of days hopefully.

@seakayone
Copy link
Contributor

PR (#3690) is ready for review.

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 a pull request may close this issue.

8 participants