clean up tests, log failure to marshal

Author: joybestourous

internal/transport/controlbuf.go

@@ -32,6 +32,7 @@ import (
 	"golang.org/x/net/http2/hpack"
 	"google.golang.org/grpc/internal/grpclog"
 	"google.golang.org/grpc/internal/grpcutil"
+	"google.golang.org/grpc/internal/pretty"
 	istatus "google.golang.org/grpc/internal/status"
 	"google.golang.org/grpc/mem"
 	"google.golang.org/grpc/status"
@@ -858,7 +859,9 @@ func (l *loopyWriter) earlyAbortStreamHandler(eas *earlyAbortStream) error {
 
 	if p := istatus.RawStatusProto(eas.status); len(p.GetDetails()) > 0 {
 		stBytes, err := proto.Marshal(p)
-		if err == nil {
+		if err != nil {
+			l.logger.Errorf("Failed to marshal rpc status: %s, error: %v", pretty.ToJSON(p), err)
+		} else {
 			headerFields = append(headerFields, hpack.HeaderField{Name: grpcStatusDetailsBinHeader, Value: encodeBinHeader(stBytes)})
 		}
 	}

test/end2end_test.go

@@ -2155,44 +2155,38 @@ func testTap(t *testing.T, e env) {
 }
 
 func (s) TestTapStatusDetails(t *testing.T) {
-	wantDetails := &testpb.Empty{}
-	st := status.New(codes.ResourceExhausted, "rate limit exceeded")
-	st, err := st.WithDetails(wantDetails)
-	if err != nil {
-		t.Fatalf("status.WithDetails() failed: %v", err)
-	}
-
-	tapHandler := func(_ context.Context, _ *tap.Info) (context.Context, error) {
+	tapHandler := func(context.Context, *tap.Info) (context.Context, error) {
 		// Return error with details for all RPCs.
+		wantDetails := &testpb.Empty{}
+		st := status.New(codes.ResourceExhausted, "rate limit exceeded")
+		st, err := st.WithDetails(wantDetails)
+		if err != nil {
+			t.Fatalf("status.WithDetails() failed: %v", err)
+		}
 		return nil, st.Err()
 	}
 
-	ss := &stubserver.StubServer{
-		EmptyCallF: func(_ context.Context, _ *testpb.Empty) (*testpb.Empty, error) {
-			// This should never be called since TAP handler rejects the RPC.
-			return &testpb.Empty{}, nil
-		},
-	}
-	sopts := []grpc.ServerOption{grpc.InTapHandle(tapHandler)}
-	if err := ss.Start(sopts); err != nil {
-		t.Fatalf("Error starting server: %v", err)
-	}
+	ss := stubserver.StartTestService(t, nil, grpc.InTapHandle(tapHandler))
 	defer ss.Stop()
 
+	if err := ss.StartClient(); err != nil {
+		t.Fatalf("ss.StartClient() failed: %v", err)
+	}
+
 	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
 	defer cancel()
 
-	_, err = ss.Client.EmptyCall(ctx, &testpb.Empty{})
+	_, err := ss.Client.EmptyCall(ctx, &testpb.Empty{})
 	if err == nil {
 		t.Fatal("EmptyCall() succeeded; want error")
 	}
 
 	gotStatus := status.Convert(err)
 	if gotStatus.Code() != codes.ResourceExhausted {
-		t.Fatalf("EmptyCall() returned code %v; want %v", gotStatus.Code(), codes.ResourceExhausted)
+		t.Errorf("EmptyCall() returned code %v; want %v", gotStatus.Code(), codes.ResourceExhausted)
 	}
 	if gotStatus.Message() != "rate limit exceeded" {
-		t.Fatalf("EmptyCall() returned message %q; want %q", gotStatus.Message(), "rate limit exceeded")
+		t.Errorf("EmptyCall() returned message %q; want %q", gotStatus.Message(), "rate limit exceeded")
 	}
 
 	details := gotStatus.Details()