Skip to content

Commit d5bc85c

Browse files
MakMukhiMakMukhi
authored
Server shouldn't Fatalf in case it fails to encode. (#1251)
* Server shouldn't Fatalf in case it fails to encode. * golint * post-review update
1 parent 1e47334 commit d5bc85c

5 files changed

Lines changed: 50 additions & 13 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 Errorf(codes.Internal, "grpc: %v", err)
122+
return 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, err
317+
return nil, Errorf(codes.Internal, "grpc: error while marshaling: %v", err.Error())
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, err
327+
return nil, Errorf(codes.Internal, "grpc: error while compressing: %v", err.Error())
328328
}
329329
b = cbuf.Bytes()
330330
}

‎server.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -664,14 +664,8 @@ 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-
// 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)
667+
grpclog.Println("grpc: server failed to encode response: ", err)
668+
return err
675669
}
676670
if len(p) > s.opts.maxSendMessageSize {
677671
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: 1 addition & 2 deletions
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 Errorf(codes.Internal, "grpc: %v", err)
367+
return err
368368
}
369369
if cs.c.maxSendMessageSize == nil {
370370
return Errorf(codes.Internal, "callInfo maxSendMessageSize field uninitialized(nil)")
@@ -606,7 +606,6 @@ func (ss *serverStream) SendMsg(m interface{}) (err error) {
606606
}
607607
}()
608608
if err != nil {
609-
err = Errorf(codes.Internal, "grpc: %v", err)
610609
return err
611610
}
612611
if len(out) > ss.maxSendMessageSize {

‎test/end2end_test.go‎

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

458459
// srv and srvAddr are set once startServer is called.
459460
srv *grpc.Server
@@ -4795,3 +4796,46 @@ func testPerRPCCredentialsViaDialOptionsAndCallOptions(t *testing.T, e env) {
47954796
t.Fatalf("Test failed. Reason: %v", err)
47964797
}
47974798
}
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)