Skip to content

Commit

Permalink
Fix crash due to mismatch number of columns and values in a row (#349)
Browse files Browse the repository at this point in the history
* Fix crash due to mismatch number of columns and values in a row

* Fix lint error

* Rename field
  • Loading branch information
tiopramayudi committed Jul 11, 2023
1 parent 4ed1400 commit ca54bfd
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 18 deletions.
2 changes: 1 addition & 1 deletion api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/caraml-dev/mlp v1.8.1-0.20230613010931-dd63f4364a18
github.com/caraml-dev/turing/engines/experiment v0.0.0
github.com/caraml-dev/turing/engines/router v0.0.0
github.com/caraml-dev/universal-prediction-interface v0.3.4
github.com/caraml-dev/universal-prediction-interface v0.3.6
github.com/gavv/httpexpect/v2 v2.15.0
github.com/getkin/kin-openapi v0.76.0
github.com/ghodss/yaml v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ github.com/caraml-dev/merlin/api v0.0.0-20230403075012-795947162429/go.mod h1:BR
github.com/caraml-dev/merlin/python/batch-predictor v0.0.0-20230403075012-795947162429/go.mod h1:jYSIcxx7FDccKSva3mo12YhQ0rYuP4MOEbgSveY82HE=
github.com/caraml-dev/mlp v1.8.1-0.20230613010931-dd63f4364a18 h1:/Rjw7+qVMo+rBaBqzJZrtQ1lKXb3KXeTXP1lJOTT7xI=
github.com/caraml-dev/mlp v1.8.1-0.20230613010931-dd63f4364a18/go.mod h1:dNqC/QnXYpkxWDaV6XU8y1UJTjmKJC3Z6CpW1n9Hjd0=
github.com/caraml-dev/universal-prediction-interface v0.3.4 h1:cPytzpjXE/8RhSw3iS0JFZzNdM3tJ/l8UcHTPrxQWEo=
github.com/caraml-dev/universal-prediction-interface v0.3.4/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/caraml-dev/universal-prediction-interface v0.3.6 h1:G/D4aukfjLECl8armJqFy/R2+0u/f4AiurSFqAo33uQ=
github.com/caraml-dev/universal-prediction-interface v0.3.6/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/casbin/casbin v1.7.0/go.mod h1:c67qKN6Oum3UF5Q1+BByfFxkwKvhwW57ITjqwtzR1KE=
github.com/census-ecosystem/opencensus-go-exporter-aws v0.0.0-20180411051634-41633bc1ff6b/go.mod h1:icwlHTP1AjScKRxD/s/Qinb7mpbcoUPpqaiBvrSS/QI=
github.com/census-instrumentation/opencensus-proto v0.2.0/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
Expand Down
2 changes: 1 addition & 1 deletion engines/experiment/examples/plugins/hardcoded/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/buger/jsonparser v1.1.1 // indirect
github.com/caraml-dev/turing/engines/router v0.0.0 // indirect
github.com/caraml-dev/universal-prediction-interface v0.3.4 // indirect
github.com/caraml-dev/universal-prediction-interface v0.3.6 // indirect
github.com/cespare/xxhash/v2 v2.1.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fatih/color v1.7.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions engines/experiment/examples/plugins/hardcoded/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMU
github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
github.com/caraml-dev/mlp v1.7.6 h1:u3OvbtPY7w/j9z7WO9YwhKJA0Y0Qy6OzanREfeA7os4=
github.com/caraml-dev/mlp v1.7.6/go.mod h1:ahdKpb/SKFEyHE6817AOh0b2fyroTzJmXM8U+2BG75M=
github.com/caraml-dev/universal-prediction-interface v0.3.4 h1:cPytzpjXE/8RhSw3iS0JFZzNdM3tJ/l8UcHTPrxQWEo=
github.com/caraml-dev/universal-prediction-interface v0.3.4/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/caraml-dev/universal-prediction-interface v0.3.6 h1:G/D4aukfjLECl8armJqFy/R2+0u/f4AiurSFqAo33uQ=
github.com/caraml-dev/universal-prediction-interface v0.3.6/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
Expand Down
2 changes: 1 addition & 1 deletion engines/experiment/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/buger/jsonparser v1.1.1
github.com/caraml-dev/mlp v1.7.6
github.com/caraml-dev/turing/engines/router v0.0.0
github.com/caraml-dev/universal-prediction-interface v0.3.4
github.com/caraml-dev/universal-prediction-interface v0.3.6
github.com/go-playground/validator v9.31.0+incompatible
github.com/hashicorp/go-hclog v0.16.0
github.com/hashicorp/go-plugin v1.4.3
Expand Down
4 changes: 2 additions & 2 deletions engines/experiment/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMU
github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
github.com/caraml-dev/mlp v1.7.6 h1:u3OvbtPY7w/j9z7WO9YwhKJA0Y0Qy6OzanREfeA7os4=
github.com/caraml-dev/mlp v1.7.6/go.mod h1:ahdKpb/SKFEyHE6817AOh0b2fyroTzJmXM8U+2BG75M=
github.com/caraml-dev/universal-prediction-interface v0.3.4 h1:cPytzpjXE/8RhSw3iS0JFZzNdM3tJ/l8UcHTPrxQWEo=
github.com/caraml-dev/universal-prediction-interface v0.3.4/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/caraml-dev/universal-prediction-interface v0.3.6 h1:G/D4aukfjLECl8armJqFy/R2+0u/f4AiurSFqAo33uQ=
github.com/caraml-dev/universal-prediction-interface v0.3.6/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
Expand Down
2 changes: 1 addition & 1 deletion engines/router/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/buger/jsonparser v1.1.1
github.com/caraml-dev/mlp v1.7.6
github.com/caraml-dev/turing/engines/experiment v0.0.0
github.com/caraml-dev/universal-prediction-interface v0.3.4
github.com/caraml-dev/universal-prediction-interface v0.3.6
github.com/fluent/fluent-logger-golang v1.5.0
github.com/go-playground/validator/v10 v10.11.1
github.com/gojek/fiber v0.2.1-rc2
Expand Down
4 changes: 2 additions & 2 deletions engines/router/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMU
github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
github.com/caraml-dev/mlp v1.7.6 h1:u3OvbtPY7w/j9z7WO9YwhKJA0Y0Qy6OzanREfeA7os4=
github.com/caraml-dev/mlp v1.7.6/go.mod h1:ahdKpb/SKFEyHE6817AOh0b2fyroTzJmXM8U+2BG75M=
github.com/caraml-dev/universal-prediction-interface v0.3.4 h1:cPytzpjXE/8RhSw3iS0JFZzNdM3tJ/l8UcHTPrxQWEo=
github.com/caraml-dev/universal-prediction-interface v0.3.4/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/caraml-dev/universal-prediction-interface v0.3.6 h1:G/D4aukfjLECl8armJqFy/R2+0u/f4AiurSFqAo33uQ=
github.com/caraml-dev/universal-prediction-interface v0.3.6/go.mod h1:e0qmFOXQxx8HFg5ObYyQO3WVnrqsr5v5JApFmeF7eJo=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40 h1:xvUo53O5MRZhVMJAxWCJcS5HHrqAiAG9SJ1LpMu6aAI=
github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA=
Expand Down
67 changes: 62 additions & 5 deletions engines/router/missionctl/log/resultlog/upi_result_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import (
"google.golang.org/protobuf/types/known/structpb"
"google.golang.org/protobuf/types/known/timestamppb"

upiv1 "github.com/caraml-dev/universal-prediction-interface/gen/go/grpc/caraml/upi/v1"
"github.com/caraml-dev/universal-prediction-interface/pkg/converter"

"github.com/caraml-dev/turing/engines/router/missionctl/config"
"github.com/caraml-dev/turing/engines/router/missionctl/errors"
"github.com/caraml-dev/turing/engines/router/missionctl/log/resultlog/proto/turing"
upiv1 "github.com/caraml-dev/universal-prediction-interface/gen/go/grpc/caraml/upi/v1"
"github.com/caraml-dev/universal-prediction-interface/pkg/converter"

fiberProtocol "github.com/gojek/fiber/protocol"
)
Expand Down Expand Up @@ -193,9 +194,10 @@ func TestUPIResultLogger_LogTuringRouterRequestSummary_logRouterLog(t *testing.T
resultLogger *UPIResultLogger
}
tests := []struct {
name string
args args
want *upiv1.RouterLog
name string
args args
want *upiv1.RouterLog
expectLogError bool
}{
{
name: "empty request and response",
Expand Down Expand Up @@ -317,6 +319,56 @@ func TestUPIResultLogger_LogTuringRouterRequestSummary_logRouterLog(t *testing.T
},
},
},
{
name: "predict request; mismatch number of columns and number of values in a row",
args: args{
reqHeader: metadata.Pairs("k1", "v1"),
upiReq: &upiv1.PredictValuesRequest{
PredictionTable: &upiv1.Table{
Name: "prediction_table",
Columns: []*upiv1.Column{
{
Name: "col1",
Type: upiv1.Type_TYPE_DOUBLE,
},
},
Rows: []*upiv1.Row{
{
RowId: "0",
Values: []*upiv1.Value{
{
DoubleValue: 0.4,
},
},
},
{
RowId: "1",
Values: []*upiv1.Value{
{
DoubleValue: 0.4,
},
{
IntegerValue: 2,
},
},
},
},
},
TransformerInput: &upiv1.TransformerInput{
Tables: []*upiv1.Table{predictionTable},
Variables: []*upiv1.Variable{variable},
},
TargetName: "target-name",
PredictionContext: predictionContext,
Metadata: &upiv1.RequestMetadata{
PredictionId: "123",
RequestTimestamp: time,
},
},
resultLogger: &UPIResultLogger{upiLogger: &mockUPILogger{}, loggerType: config.UPILogger},
},
expectLogError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -327,6 +379,11 @@ func TestUPIResultLogger_LogTuringRouterRequestSummary_logRouterLog(t *testing.T
tt.args.resultLogger.LogTuringRouterRequestSummary(tt.args.reqHeader, tt.args.upiReq, respCh)
mockLogger, ok := tt.args.resultLogger.upiLogger.(*mockUPILogger)
assert.True(t, ok, "mockUPILogger not used")
if tt.expectLogError {
// no calls due to error
assert.Equal(t, int32(0), mockLogger.numOfCalls)
return
}
assert.Equal(t, tt.want, mockLogger.routerLog)
})
}
Expand Down
29 changes: 29 additions & 0 deletions engines/router/missionctl/server/upi/interceptors/recoverer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package interceptors

import (
"context"
"fmt"

"google.golang.org/grpc"
)

// PanicRecoveryInterceptor interceptor to recover after facing panic
func PanicRecoveryInterceptor() grpc.UnaryServerInterceptor {
return func(ctx context.Context,
req interface{},
info *grpc.UnaryServerInfo,
handler grpc.UnaryHandler) (resp interface{}, err error) {
defer func(ctx context.Context) {
if r := recover(); r != nil {
if e, ok := r.(error); ok {
err = e
} else {
err = fmt.Errorf("panic: %s", r)
}
}
}(ctx)

resp, err = handler(ctx, req)
return resp, err
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package interceptors

import (
"context"
"testing"

upiv1 "github.com/caraml-dev/universal-prediction-interface/gen/go/grpc/caraml/upi/v1"
"github.com/stretchr/testify/assert"
)

func TestPanicRecovery(t *testing.T) {
t.Run("Test panic recovery", func(t *testing.T) {
panicIntercep := PanicRecoveryInterceptor()
_, err := panicIntercep(
context.Background(),
&upiv1.PredictValuesRequest{},
nil,
func(ctx context.Context, req interface{}) (interface{}, error) {
panic("something wrong")
})
assert.Equal(t, "panic: something wrong", err.Error())
})

}
4 changes: 3 additions & 1 deletion engines/router/missionctl/server/upi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/caraml-dev/turing/engines/router/missionctl/log"
"github.com/caraml-dev/turing/engines/router/missionctl/log/resultlog"
"github.com/caraml-dev/turing/engines/router/missionctl/server/constant"
"github.com/caraml-dev/turing/engines/router/missionctl/server/upi/interceptors"
"github.com/caraml-dev/turing/engines/router/missionctl/turingctx"
)

Expand All @@ -49,7 +50,8 @@ func NewUPIServer(mc missionctl.MissionControlUPI, rl *resultlog.UPIResultLogger
}

func (us *Server) Run(listener net.Listener) {
s := grpc.NewServer()

s := grpc.NewServer(grpc.UnaryInterceptor(interceptors.PanicRecoveryInterceptor()))
upiv1.RegisterUniversalPredictionServiceServer(s, us)
reflection.Register(s)

Expand Down

0 comments on commit ca54bfd

Please sign in to comment.