Add gRPC status codes #137
Conversation
codegen/testing/src/test/scala/hello/world/TestServiceSuite.scala
Outdated
Show resolved
Hide resolved
| def asStatusRuntimeException(message: Option[String] = None): StatusRuntimeException = | ||
| StatusRuntimeException(this, message) |
There was a problem hiding this comment.
Is it legit for the Ok status to be considered as StatusRuntimeException?
There was a problem hiding this comment.
Yeah, it feels weird. We could further divide codes into success and errors.
There was a problem hiding this comment.
Just passing by, but I think we should take gRPC richer error model into consideration for error handling design because there are more and more gRPC implementations that can send extra error details in trailers header（e.g. golang, Java or Rust implementation).
See
- https://grpc.io/docs/guides/error/#richer-error-model
- https://github.com/googleapis/googleapis/blob/e67f6bea8425bbc3a6352ffea8ce2bc6dab45454/google/rpc/status.proto#L35
for more details.
There was a problem hiding this comment.
@danicheg How about making the status Ok package private, since user code should never need it anyways?
There was a problem hiding this comment.
Then we have to drop it from the codeValues list which is kind of weird.
Do we really need to care if users want to shoot themselves in the foot?
There was a problem hiding this comment.
@hamnis good point. If we need to be cautious, I would include an assertion in asStatusRuntimeException to prevent Ok from being treated as an exception.
There was a problem hiding this comment.
if you want to do that go ahead.
An attempt at solving #135.