akka / akka-grpc

A platform to build and run apps that are elastic, agile, and resilient. SDK, libraries, and hosted environments.
https://doc.akka.io/libraries/akka-grpc/current/
Other
431 stars 124 forks source link

Add Trailers.apply to construct from code, message, and details #1933

Closed alexklibisz closed 6 months ago

alexklibisz commented 7 months ago

The handler generated by akka-grpc takes an error handler in the form of eHandler: ActorSystem => PartialFunction[Throwable, Trailers].

This change just adds a method to make it easier to construct an instance of Trailers containing rich error details, so that we can enrich the error response from within the error handler.

Couple things I'm not sure about in the initial implementation:

  1. This implementation delegates to the analogous GrpcServiceException.apply method. Is there a preferred way to would abstract out the common code? I.e., create a Utils object somewhere.
  2. The runtime/test submodule does not seem to import any instances of GeneratedMessage, so I'm not sure how to test that we're encoding details correctly. Should we modify the subproject so that it imports LocalizedMessage? Should we put the tests in another submodule.
alexklibisz commented 7 months ago

I added the create method in https://github.com/akka/akka-grpc/pull/1933/commits/ed7a91359de5b3a28db1a639834ac5e0abf1699e

And added an example to the LoggingErrorHandlingGreeterServer in https://github.com/akka/akka-grpc/pull/1933/commits/18856521d354a8e9030b9d2d26485bd95fb44340.

If that example looks good, I can do the same on the Java side.

johanandren commented 7 months ago

That example looks good, please add for Java as well 👍

alexklibisz commented 7 months ago

Ran into an issue in the Java example. Commented inline.

alexklibisz commented 6 months ago

I'm seeing an error in the Gradle build: https://github.com/akka/akka-grpc/actions/runs/8851059859/job/24306757026?pr=1933#step:9:131

Error:  /home/runner/work/akka-grpc/akka-grpc/plugin-tester-scala/src/main/scala/example/myapp/helloworld/LoggingErrorHandlingGreeterServer.scala:89: class com.google.rpc.LocalizedMessage is not a value

on the line:

Trailers(com.google.rpc.Code.INVALID_ARGUMENT, ex.getMessage, List(LocalizedMessage("en-US", ex.getMessage)))

I guess it's possible that gradle isn't configured to know about the scalapb-generated LocalizedMessage. 🤔

alexklibisz commented 6 months ago

@johanandren This is ready for another look. I might need a hint on how to get the Gradle Scala build working.

johanandren commented 6 months ago

The plugin-tester-scala is used for verifying building Scala projects with gradle and maven in addition to the "normal" sbt, so those two project files needed some ammends, I've pushed fixes that should sort it.