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

Add StatusException trailers to response metadata #548

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Gregory-Berkman-Imprivata

Modified test unary request (common) - returns correct error response to check if trailers in the StatusException are propagated from server to client.
Modified ListenerDriver.scala to add the trailers from the StatusException to the response metadata

The modified test passed but other tests in MetadataSpec failed for unknown reasons

#547

@Gregory-Berkman-Imprivata Gregory-Berkman-Imprivata marked this pull request as draft August 22, 2023 20:34
@Gregory-Berkman-Imprivata
Copy link
Author

@thesamet I modified a test to check if trailers are propagated from the server to the client. That test passed in my PR but other MetadataSpec tests started failing. I also did not modify any other request type except for unary.

@thesamet
Copy link
Contributor

Looks like the change isn't compatible with Scala 2.12: https://github.com/scalapb/zio-grpc/actions/runs/5943784958/job/16119751686?pr=548#step:7:27

@Gregory-Berkman-Imprivata
Copy link
Author

Looks like the change isn't compatible with Scala 2.12: https://github.com/scalapb/zio-grpc/actions/runs/5943784958/job/16119751686?pr=548#step:7:27

I used import scala.jdk.CollectionConverters.CollectionHasAsScala to convert the set of Keys in the metadata to scala Iterable but it has not been introduced in Scala 2.12. Switch to import scala.collection.JavaConverters._ which is deprecated in 2.13

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