Add details to stream error response #561
Conversation
|
Probably easiest to merge this and close #560 if we want both of these changes. |
runtime/handler.go
Outdated
| grpcCode = s.Code() | ||
| } | ||
| httpCode := HTTPStatusFromCode(grpcCode) | ||
| s, _ := status.FromError(err) |
There was a problem hiding this comment.
Hmm. I know it should never happen, but seeing the second return ignored is weird, too. Any thoughts about something like
s, ok := status.FromError(err)
if !ok {
panic("err is no status")
}There was a problem hiding this comment.
I wanted to make use of the new status handling in gRPC-Go 1.10 which is equivalent to the old handling, but I accept gRPC-Gateway can't enforce that users have the latest gRPC version. I will change it back.
There was a problem hiding this comment.
Oh that's a lovely function. Thanks for the pointer.
johanbrandhorst
force-pushed
the
add-details-to-stream-error-response
branch
from
cd4034d to
345e924
Compare
johanbrandhorst
force-pushed
the
add-details-to-stream-error-response
branch
from
345e924 to
16c3cee
Compare
|
I hate to bump this but I think it's a pretty straightforward, backwards compatible change with a new test case and I'd really like to have this merged ASAP. Please advise if I can speed things up somehow. |
|
I just reviewed this and I think it is good to go. Marking as approved. Can you please close any other PRs that this obviates and then I'll merge it (probably tomorrow morning/early afternoon US time) |
|
Today was a disaster. Sorry for the slow merge. Merging now |
|
Any chance we could have a new release with this? 1.4.0? |
…ctionById fix address bug, address in index is hex
* Use the status Message when possible in errors * Add details to stream error response.
Add the gRPC status details to the streamed response.
This is built on top of #560, but I can close that one if we want to merge these together instead.