Skip to content

Commit

Permalink
Stop marshalling any.Any types unnecessarily.
Browse files Browse the repository at this point in the history
Fixes #576. Adds a test case for marshalling of details as well.
  • Loading branch information
johanbrandhorst committed Jun 22, 2018
1 parent 9b9677c commit 1665baf
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 19 deletions.
1 change: 1 addition & 0 deletions runtime/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ go_test(
"@com_github_golang_protobuf//ptypes/timestamp:go_default_library",
"@com_github_golang_protobuf//ptypes/wrappers:go_default_library",
"@org_golang_google_genproto//protobuf/field_mask:go_default_library",
"@org_golang_google_genproto//googleapis/rpc/errdetails:go_default_library",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//codes:go_default_library",
"@org_golang_google_grpc//metadata:go_default_library",
Expand Down
17 changes: 3 additions & 14 deletions runtime/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"net/http"

"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/any"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/grpclog"
Expand Down Expand Up @@ -94,19 +93,9 @@ func DefaultHTTPError(ctx context.Context, mux *ServeMux, marshaler Marshaler, w
}

body := &errorBody{
Error: s.Message(),
Code: int32(s.Code()),
}

for _, detail := range s.Details() {
if det, ok := detail.(proto.Message); ok {
a, err := ptypes.MarshalAny(det)
if err != nil {
grpclog.Infof("Failed to marshal any: %v", err)
} else {
body.Details = append(body.Details, a)
}
}
Error: s.Message(),
Code: int32(s.Code()),
Details: s.Proto().GetDetails(),
}

buf, merr := marshaler.Marshal(body)
Expand Down
37 changes: 32 additions & 5 deletions runtime/errors_test.go
Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
package runtime_test

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"context"
"github.com/grpc-ecosystem/grpc-gateway/runtime"
"google.golang.org/genproto/googleapis/rpc/errdetails"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func TestDefaultHTTPError(t *testing.T) {
ctx := context.Background()

statusWithDetails, _ := status.New(codes.FailedPrecondition, "failed precondition").WithDetails(
&errdetails.PreconditionFailure{},
)

for _, spec := range []struct {
err error
status int
msg string
err error
status int
msg string
details string
}{
{
err: fmt.Errorf("example error"),
Expand All @@ -32,10 +38,16 @@ func TestDefaultHTTPError(t *testing.T) {
status: http.StatusNotFound,
msg: "no such resource",
},
{
err: statusWithDetails.Err(),
status: http.StatusPreconditionFailed,
msg: "failed precondition",
details: "type.googleapis.com/google.rpc.PreconditionFailure",
},
} {
w := httptest.NewRecorder()
req, _ := http.NewRequest("", "", nil) // Pass in an empty request to match the signature
runtime.DefaultHTTPError(ctx, &runtime.ServeMux{}, &runtime.JSONBuiltin{}, w, req, spec.err)
runtime.DefaultHTTPError(ctx, &runtime.ServeMux{}, &runtime.JSONPb{}, w, req, spec.err)

if got, want := w.Header().Get("Content-Type"), "application/json"; got != want {
t.Errorf(`w.Header().Get("Content-Type") = %q; want %q; on spec.err=%v`, got, want, spec.err)
Expand All @@ -53,5 +65,20 @@ func TestDefaultHTTPError(t *testing.T) {
if got, want := body["error"].(string), spec.msg; !strings.Contains(got, want) {
t.Errorf(`body["error"] = %q; want %q; on spec.err=%v`, got, want, spec.err)
}

if spec.details != "" {
details, ok := body["details"].([]interface{})
if !ok {
t.Errorf(`body["details"] = %T; want %T`, body["details"], []interface{}{})
continue
}
if len(details) != 1 {
t.Errorf(`len(body["details"]) = %v; want 1`, len(details))
continue
}
if details[0].(map[string]interface{})["@type"] != spec.details {
t.Errorf(`details.@type = %s; want %s`, details[0].(map[string]interface{})["@type"], spec.details)
}
}
}
}

0 comments on commit 1665baf

Please sign in to comment.