fix: Avoid NPE in GrpcMetadataImpl #1854 #1855
Conversation
| new GrpcServiceException( | ||
| ex.getStatus, | ||
| new RichGrpcMetadataImpl( | ||
| ex.getStatus, |
There was a problem hiding this comment.
Unrelated to this PR, but why we use RichGrpcMetadataImpl, but above we just use GrpcMetadataImpl?
There was a problem hiding this comment.
We want to propagate the structured error details if present, but maybe cleaner and more correct to select between GrpcMetadataImpl and RichGrpcMedataImpl on the presence of trailers instead of giving an empty metadata like I did. I'll do that instead.
There was a problem hiding this comment.
Hmm, looking at it some more, actually, I'm not so sure why we are not passing RichGrpcMetadataImpl above, that looks like the right thing to do.
There was a problem hiding this comment.
312a11a should do the trick
Also, in the case of going from proto details we always have trailers
| new GrpcMetadataImpl( | ||
| // might not be present | ||
| Option(statusRuntimeException.getTrailers).getOrElse(new io.grpc.Metadata()))) | ||
| new RichGrpcMetadataImpl(statusRuntimeException.getStatus, statusRuntimeException.getTrailers)) |
There was a problem hiding this comment.
we just converted, trailers always present here
Exception may not have any trailers. Also, fail fast if created with null delegate in some place we don't expected.
References #1854