Skip to content

Commit 8431165

Browse files
MakMukhiMakMukhi
authored
Revert "Server shouldn't Fatalf in case it fails to encode. (#1251)" (#1274)
This reverts commit d5bc85c.
1 parent d5bc85c commit 8431165

5 files changed

Lines changed: 13 additions & 50 deletions

File tree

‎call.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func sendRequest(ctx context.Context, dopts dialOptions, compressor Compressor,
119119
}
120120
outBuf, err := encode(dopts.codec, args, compressor, cbuf, outPayload)
121121
if err != nil {
122-
return err
122+
return Errorf(codes.Internal, "grpc: %v", err)
123123
}
124124
if c.maxSendMessageSize == nil {
125125
return Errorf(codes.Internal, "callInfo maxSendMessageSize field uninitialized(nil)")

‎rpc_util.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func encode(c Codec, msg interface{}, cp Compressor, cbuf *bytes.Buffer, outPayl
314314
// TODO(zhaoq): optimize to reduce memory alloc and copying.
315315
b, err = c.Marshal(msg)
316316
if err != nil {
317-
return nil, Errorf(codes.Internal, "grpc: error while marshaling: %v", err.Error())
317+
return nil, err
318318
}
319319
if outPayload != nil {
320320
outPayload.Payload = msg
@@ -324,7 +324,7 @@ func encode(c Codec, msg interface{}, cp Compressor, cbuf *bytes.Buffer, outPayl
324324
}
325325
if cp != nil {
326326
if err := cp.Do(cbuf, b); err != nil {
327-
return nil, Errorf(codes.Internal, "grpc: error while compressing: %v", err.Error())
327+
return nil, err
328328
}
329329
b = cbuf.Bytes()
330330
}

‎server.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,14 @@ func (s *Server) sendResponse(t transport.ServerTransport, stream *transport.Str
664664
}
665665
p, err := encode(s.opts.codec, msg, cp, cbuf, outPayload)
666666
if err != nil {
667-
grpclog.Println("grpc: server failed to encode response: ", err)
668-
return err
667+
// This typically indicates a fatal issue (e.g., memory
668+
// corruption or hardware faults) the application program
669+
// cannot handle.
670+
//
671+
// TODO(zhaoq): There exist other options also such as only closing the
672+
// faulty stream locally and remotely (Other streams can keep going). Find
673+
// the optimal option.
674+
grpclog.Fatalf("grpc: Server failed to encode response %v", err)
669675
}
670676
if len(p) > s.opts.maxSendMessageSize {
671677
return status.Errorf(codes.ResourceExhausted, "grpc: trying to send message larger than max (%d vs. %d)", len(p), s.opts.maxSendMessageSize)

‎stream.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ func (cs *clientStream) SendMsg(m interface{}) (err error) {
364364
}
365365
}()
366366
if err != nil {
367-
return err
367+
return Errorf(codes.Internal, "grpc: %v", err)
368368
}
369369
if cs.c.maxSendMessageSize == nil {
370370
return Errorf(codes.Internal, "callInfo maxSendMessageSize field uninitialized(nil)")
@@ -606,6 +606,7 @@ func (ss *serverStream) SendMsg(m interface{}) (err error) {
606606
}
607607
}()
608608
if err != nil {
609+
err = Errorf(codes.Internal, "grpc: %v", err)
609610
return err
610611
}
611612
if len(out) > ss.maxSendMessageSize {

‎test/end2end_test.go‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,6 @@ type test struct {
454454
clientInitialWindowSize int32
455455
clientInitialConnWindowSize int32
456456
perRPCCreds credentials.PerRPCCredentials
457-
customCodec grpc.Codec
458457

459458
// srv and srvAddr are set once startServer is called.
460459
srv *grpc.Server
@@ -4796,46 +4795,3 @@ func testPerRPCCredentialsViaDialOptionsAndCallOptions(t *testing.T, e env) {
47964795
t.Fatalf("Test failed. Reason: %v", err)
47974796
}
47984797
}
4799-
4800-
type errCodec struct {
4801-
noError bool
4802-
}
4803-
4804-
func (c *errCodec) Marshal(v interface{}) ([]byte, error) {
4805-
if c.noError {
4806-
return []byte{}, nil
4807-
}
4808-
return nil, fmt.Errorf("3987^12 + 4365^12 = 4472^12")
4809-
}
4810-
4811-
func (c *errCodec) Unmarshal(data []byte, v interface{}) error {
4812-
return nil
4813-
}
4814-
4815-
func (c *errCodec) String() string {
4816-
return "Fermat's near-miss."
4817-
}
4818-
4819-
func TestEncodeDoesntPanic(t *testing.T) {
4820-
defer leakCheck(t)()
4821-
for _, e := range listTestEnv() {
4822-
testEncodeDoesntPanic(t, e)
4823-
}
4824-
}
4825-
4826-
func testEncodeDoesntPanic(t *testing.T, e env) {
4827-
te := newTest(t, e)
4828-
erc := &errCodec{}
4829-
te.customCodec = erc
4830-
te.startServer(&testServer{security: e.security})
4831-
defer te.tearDown()
4832-
te.customCodec = nil
4833-
tc := testpb.NewTestServiceClient(te.clientConn())
4834-
// Failure case, should not panic.
4835-
tc.EmptyCall(context.Background(), &testpb.Empty{})
4836-
erc.noError = true
4837-
// Passing case.
4838-
if _, err := tc.EmptyCall(context.Background(), &testpb.Empty{}); err != nil {
4839-
t.Fatalf("EmptyCall(_, _) = _, %v, want _, <nil>", err)
4840-
}
4841-
}

0 commit comments

Comments
 (0)