Skip to content

Commit

Permalink
Add unit test
Browse files Browse the repository at this point in the history
Signed-off-by: Jason Parraga <[email protected]>
  • Loading branch information
Sovietaced committed Jul 9, 2024
1 parent 38f1a03 commit c6d3139
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@ func (ri *RecoveryInterceptor) UnaryServerInterceptor() grpc.UnaryServerIntercep
defer func() {
if r := recover(); r != nil {
ri.panicCounter.Inc()
logger.Fatalf(ctx, "panic-ed for request: [%+v] to %s with err: %v with Stack: %v", req, info.FullMethod, r, string(debug.Stack()))
logger.Errorf(ctx, "panic-ed for request: [%+v] to %s with err: %v with Stack: %v", req, info.FullMethod, r, string(debug.Stack()))
// Return INTERNAL to client with no info as to not leak implementation details
err = status.Errorf(codes.Internal, "")
}
}()

resp, err := handler(ctx, req)
return resp, err
return handler(ctx, req)
}
}

Expand All @@ -49,13 +48,12 @@ func (ri *RecoveryInterceptor) StreamServerInterceptor() grpc.StreamServerInterc
defer func() {
if r := recover(); r != nil {
ri.panicCounter.Inc()
logger.Fatalf(stream.Context(), "panic-ed for stream to %s with err: %v with Stack: %v", info.FullMethod, r, string(debug.Stack()))
logger.Errorf(stream.Context(), "panic-ed for stream to %s with err: %v with Stack: %v", info.FullMethod, r, string(debug.Stack()))

Check warning on line 51 in flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go#L48-L51

Added lines #L48 - L51 were not covered by tests
// Return INTERNAL to client with no info as to not leak implementation details
err = status.Errorf(codes.Internal, "")

Check warning on line 53 in flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go#L53

Added line #L53 was not covered by tests
}
}()

err = handler(srv, stream)
return err
return handler(srv, stream)

Check warning on line 57 in flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/pkg/rpc/adminservice/middleware/recovery_interceptor.go#L57

Added line #L57 was not covered by tests
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package middleware

import (
"context"
mockScope "github.com/flyteorg/flyte/flytestdlib/promutils"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"testing"
)

func TestRecoveryInterceptor(t *testing.T) {
ctx := context.Background()
testScope := mockScope.NewTestScope()
recoveryInterceptor := NewRecoveryInterceptor(testScope)
unaryInterceptor := recoveryInterceptor.UnaryServerInterceptor()
info := &grpc.UnaryServerInfo{}
req := "test-request"

t.Run("should recover from panic", func(t *testing.T) {
_, err := unaryInterceptor(ctx, req, info, func(ctx context.Context, req any) (any, error) {
panic("synthetic")
})
expectedErr := status.Errorf(codes.Internal, "")
require.Error(t, err)
require.Equal(t, expectedErr, err)
})

t.Run("should plumb response without panic", func(t *testing.T) {
mockedResponse := "test"
resp, err := unaryInterceptor(ctx, req, info, func(ctx context.Context, req any) (any, error) {
return mockedResponse, nil
})
require.NoError(t, err)
require.Equal(t, mockedResponse, resp)
})
}

0 comments on commit c6d3139

Please sign in to comment.