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

Exception on unsafe cast in writeResponse #1229

Closed
senia-psm opened this issue Apr 27, 2022 · 7 comments · Fixed by #1244
Closed

Exception on unsafe cast in writeResponse #1229

senia-psm opened this issue Apr 27, 2022 · 7 comments · Fixed by #1244
Labels
bug Something isn't working

Comments

@senia-psm
Copy link
Member

senia-psm commented Apr 27, 2022

A simple test that starts echo Server and performs a single Client call runs forever.

This looks like regression: runs forever on 2.0.0-RC7, works as expected on 2.0.0-RC4 (with ZManaged instead of Scope).

To Reproduce
See this project with a single test: https://github.com/senia-psm/zhttp-bug

This test runs forever:

test("Server should be able to start on an unoccupied port") {

  val dummyRoute = Http.collect[Request] { case req @ Method.POST -> !! / "echo" =>
    Response(status = Status.Ok, data = req.data)
  }

  val layer: ZLayer[ServerChannelFactory & EventLoopGroup, Nothing, Server.Start] = ZLayer.scoped {
    Server(dummyRoute).withBinding("localhost", 0).make.orDie
  }
  val env                                                                         = ZLayer
    .make[Server.Start & EventLoopGroup & ChannelFactory](
      layer,
      EventLoopGroup.auto(0),
      ServerChannelFactory.auto,
      ChannelFactory.auto,
    )

  val action = for {
    port           <- ZIO.serviceWith[Server.Start](_.port)
    resp           <- Client.request(
                        s"http://localhost:$port/echo",
                        method = Method.POST,
                        content = HttpData.fromString("echo string"),
                      )
    responseString <- resp.bodyAsString
  } yield assertTrue(port > 0, responseString == "echo string")

  action.provideLayer(env)
},

Expected behaviour
Test above should complete in finite amount of time (< 1 second).

Desktop:

  • OS: Linux
  • Version: 5.4.179-gentoo

Additional context
Might be related to zio/interop-cats#533

@senia-psm senia-psm added the bug Something isn't working label Apr 27, 2022
@senia-psm
Copy link
Member Author

Replacing Response(status = Status.Ok, data = req.data) with req.data.toByteBuf.map(bb => Response(status = Status.Ok, data = HttpData.fromByteBuf(bb))) solves an issue, though it prevents streaming. Can be used as a workaround.

@senia-psm
Copy link
Member Author

@adamgfraser tracked down the issue:
The reason is unsafe cast: msg.data.asInstanceOf[HttpData.Complete] in ServerResponseHandler::writeResponse.

@senia-psm senia-psm changed the title Possible regression in ZManaged to scoped ZIO migration Exception on unsafe cast in writeResponse Apr 28, 2022
@tusharmath
Copy link
Collaborator

Yeah this is a regression. Will take this up. Let me know if you want to give it a shot. I can point you to the exact place where changes are required.

@senia-psm
Copy link
Member Author

senia-psm commented May 2, 2022

@tusharmath I can try to fix it if you give me a hint on how to process UnsafeAsync in writeResponse. Should I convert it to Complete with something like BinaryStream(unsafeAsync.toByteBufStream)?

@tusharmath
Copy link
Collaborator

@senia-psm Basically you just want to read data piece by piece and write it on the channel.
Look at the code here — https://github.com/dream11/zio-http/blob/main/zio-http/src/main/scala/zhttp/http/HttpData.scala#L160

In our case, instead of creating a stream we will write it on the channel. Hope this helps.

@senia-psm
Copy link
Member Author

@tusharmath it seems like I don't understand zio-http internals good enough to fix it - even the most obvious and straight forward fix fails for me. See #1236 - fromStream test fails and I have no idea how to fix it.

@senia-psm
Copy link
Member Author

@tusharmath actually it seems like there is another bug - see #1237

senia-psm added a commit to senia-psm/zio-http that referenced this issue May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants