scalapb / zio-grpc

ScalaPB meets ZIO: write purely functional gRPC services and clients using ZIO
Apache License 2.0
261 stars 82 forks source link

StatusException metadata trailers not propagated to client #547

Open Gregory-Berkman-Imprivata opened 1 year ago

Gregory-Berkman-Imprivata commented 1 year ago

Metadata trailers are not being received when a request fails with a StatusException in zio-grpc 0.6.0 .

I have replicated this in the HelloWorldServer and HelloWorldClient

HelloWorldServer:

package zio_grpc.examples.helloworld

import io.grpc.Metadata.BinaryMarshaller
import io.grpc.{Metadata, Status, StatusException}
import scalapb.zio_grpc.ServerMain
import scalapb.zio_grpc.ServiceList
import zio._
import zio.Console._
import io.grpc.examples.helloworld.helloworld.ZioHelloworld.Greeter
import io.grpc.examples.helloworld.helloworld.{HelloReply, HelloRequest}

object GreeterImpl extends Greeter {
  def sayHello(
      request: HelloRequest
  ): ZIO[Any, StatusException, HelloReply] = {

    val metadataKey = Metadata.Key.of("foo-bin", new BinaryMarshaller[String] {
      override def toBytes(value: String): Array[Byte] = value.getBytes

      override def parseBytes(serialized: Array[Byte]): String = new String(serialized)
    })
    val metadata = new Metadata()
    metadata.put(metadataKey, "bar")

    (printLine(s"Got request: $request").orDie zipRight
      ZIO.fail(new StatusException(Status.INTERNAL, metadata)))
      .tapError(ex => printLine(ex.getTrailers.get(metadataKey)).orDie)
  }
}

object HelloWorldServer extends ServerMain {
  def services: ServiceList[Any] = ServiceList.add(GreeterImpl)
}

HelloWorldClient:

package zio_grpc.examples.helloworld

import io.grpc.examples.helloworld.helloworld.ZioHelloworld.GreeterClient
import io.grpc.examples.helloworld.helloworld.HelloRequest
import io.grpc.{ManagedChannelBuilder, Metadata}
import io.grpc.Metadata.BinaryMarshaller
import zio.Console._
import scalapb.zio_grpc.ZManagedChannel
import zio._

object HelloWorldClient extends zio.ZIOAppDefault {
  val clientLayer: Layer[Throwable, GreeterClient] =
    GreeterClient.live(
      ZManagedChannel(
        ManagedChannelBuilder.forAddress("localhost", 9000).usePlaintext()
      )
    )

  def myAppLogic = {
    val metadataKey = Metadata.Key.of("foo-bin", new BinaryMarshaller[String] {
      override def toBytes(value: String): Array[Byte] = value.getBytes

      override def parseBytes(serialized: Array[Byte]): String = new String(serialized)
    })

    for {
      r <- GreeterClient.sayHello(HelloRequest("World")).either
      _ <- r.fold(ex => printLine(s"${ex.getTrailers.get(metadataKey)}-${ex.getStatus}"), s => printLine(s.message))
    } yield ()
  }

  final def run =
    myAppLogic.provideLayer(clientLayer).exitCode
}

Server output:

Server is running on port 9000. Press Ctrl-C to stop.
Got request: HelloRequest(World,UnknownFieldSet(Map()))
bar

Expected Client output:

bar-Status{code=INTERNAL, description=null, cause=null}

Actual Client output:

null-Status{code=INTERNAL, description=null, cause=null}

As you can see the client receives null within the exception trailer metadata.

thesamet commented 1 year ago

Thanks for reporting! Will you be able to work on a PR to fix this?

On Tue, Aug 22, 2023 at 10:32 AM Gregory-Berkman-Imprivata < @.***> wrote:

Metadata trailers are not being received when a request fails with a StatusException in zio-grpc 0.6.0 .

I have replicated this in the HelloWorldServer and HelloWorldClient

HelloWorldServer:

package zio_grpc.examples.helloworld

import io.grpc.Metadata.BinaryMarshaller import io.grpc.{Metadata, Status, StatusException} import scalapb.zio_grpc.ServerMain import scalapb.ziogrpc.ServiceList import zio. import zio.Console._ import io.grpc.examples.helloworld.helloworld.ZioHelloworld.Greeter import io.grpc.examples.helloworld.helloworld.{HelloReply, HelloRequest}

object GreeterImpl extends Greeter { def sayHello( request: HelloRequest ): ZIO[Any, StatusException, HelloReply] = {

val metadataKey = Metadata.Key.of("foo-bin", new BinaryMarshaller[String] {
  override def toBytes(value: String): Array[Byte] = value.getBytes

  override def parseBytes(serialized: Array[Byte]): String = new String(serialized)
})
val metadata = new Metadata()
metadata.put(metadataKey, "bar")

(printLine(s"Got request: $request").orDie zipRight
  ZIO.fail(new StatusException(Status.INTERNAL, metadata)))
  .tapError(ex => printLine(ex.getTrailers.get(metadataKey)).orDie)

} }

object HelloWorldServer extends ServerMain { def services: ServiceList[Any] = ServiceList.add(GreeterImpl) }

HelloWorldClient:

package zio_grpc.examples.helloworld

import io.grpc.examples.helloworld.helloworld.ZioHelloworld.GreeterClient import io.grpc.examples.helloworld.helloworld.HelloRequest import io.grpc.{ManagedChannelBuilder, Metadata} import io.grpc.Metadata.BinaryMarshaller import zio.Console._ import scalapb.ziogrpc.ZManagedChannel import zio.

object HelloWorldClient extends zio.ZIOAppDefault { val clientLayer: Layer[Throwable, GreeterClient] = GreeterClient.live( ZManagedChannel( ManagedChannelBuilder.forAddress("localhost", 9000).usePlaintext() ) )

def myAppLogic = { val metadataKey = Metadata.Key.of("foo-bin", new BinaryMarshaller[String] { override def toBytes(value: String): Array[Byte] = value.getBytes

  override def parseBytes(serialized: Array[Byte]): String = new String(serialized)
})

for {
  r <- GreeterClient.sayHello(HelloRequest("World")).either
  _ <- r.fold(ex => printLine(s"${ex.getTrailers.get(metadataKey)}-${ex.getStatus}"), s => printLine(s.message))
} yield ()

}

final def run = myAppLogic.provideLayer(clientLayer).exitCode }

Server output:

Server is running on port 9000. Press Ctrl-C to stop. Got request: HelloRequest(World,UnknownFieldSet(Map())) bar

Expected Client output:

bar-Status{code=INTERNAL, description=null, cause=null}

Actual Client output:

null-Status{code=INTERNAL, description=null, cause=null}

As you can see the client receives null within the exception trailer metadata.

— Reply to this email directly, view it on GitHub https://github.com/scalapb/zio-grpc/issues/547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLBLPQ2UOFNZ24IYOPNATXWTULHANCNFSM6AAAAAA32IK3OU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- -Nadav

Gregory-Berkman-Imprivata commented 1 year ago

i can try. I havent contributed yet and am not too familiar with the code. I really need this feature though.

thesamet commented 1 year ago

Thanks - the first step would be adding a failing test under the e2e directory, probably under https://github.com/scalapb/zio-grpc/blob/master/e2e/src/test/scalajvm/scalapb/zio_grpc/TestServiceSpec.scala

On Tue, Aug 22, 2023 at 10:45 AM Gregory-Berkman-Imprivata < @.***> wrote:

i can try. I havent contributed yet and am not too familiar with the code. I really need this feature though.

— Reply to this email directly, view it on GitHub https://github.com/scalapb/zio-grpc/issues/547#issuecomment-1688644912, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLBLOEI47ZYYFFCYKYGE3XWTVZ7ANCNFSM6AAAAAA32IK3OU . You are receiving this because you commented.Message ID: @.***>

-- -Nadav

Gregory-Berkman-Imprivata commented 1 year ago

I think I may have found a way to add the trailers to the metadata. I went to put up a PR to discuss but I do not have access permissions to push a branch.

thesamet commented 1 year ago

You can send a PR by pushing a branch to your personal fork of this project.

thesamet commented 1 year ago

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

Gregory-Berkman-Imprivata commented 1 year ago

I figured out how to do this in my application so I am no longer blocked.

class PropagatingMetadataTrailersTransformer extends GTransform[RequestContext, StatusException, RequestContext, StatusException] {

    def effect[A](
      io: RequestContext => ZIO[Any, StatusException, A]
    ): (RequestContext => ZIO[Any, StatusException, A]) = { context => 
        io(context).tapError(ex => context.responseMetadata.wrap(_.merge(ex.getTrailers)))
    }

    ...
} 

I attempted to implement a similar solution in the PR but tests were failing. Any reason a solution like this could cause issues?

thesamet commented 1 year ago

Not seeing an obvious issue with the workaround.