fix: return better error messages for grpc-gateway errors (#1397)

* fix: return better error messages for grpc-gateway errors * feat: slip logger in to the special marhsaller, and also add an integration test for the behavior.
flipt-io · Mar 14, 2023 · 1628eca · 1628eca
1 parent 1273bed
commit 1628eca
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 20 deletions.
diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go
@@ -137,6 +137,7 @@ func registerFunc(ctx context.Context, conn *grpc.ClientConn, fn func(context.Co
 
 func authenticationHTTPMount(
 	ctx context.Context,
+	logger *zap.Logger,
 	cfg config.AuthenticationConfig,
 	r chi.Router,
 	conn *grpc.ClientConn,
@@ -172,6 +173,6 @@ func authenticationHTTPMount(
 	r.Group(func(r chi.Router) {
 		r.Use(middleware...)
 
-		r.Mount("/auth/v1", gateway.NewGatewayServeMux(muxOpts...))
+		r.Mount("/auth/v1", gateway.NewGatewayServeMux(logger, muxOpts...))
 	})
 }
diff --git a/internal/cmd/http.go b/internal/cmd/http.go
@@ -55,7 +55,7 @@ func NewHTTPServer(
 		isConsole = cfg.Log.Encoding == config.LogEncodingConsole
 
 		r        = chi.NewRouter()
-		api      = gateway.NewGatewayServeMux()
+		api      = gateway.NewGatewayServeMux(logger)
 		httpPort = cfg.Server.HTTPPort
 	)
 
@@ -130,7 +130,7 @@ func NewHTTPServer(
 
 		// mount all authentication related HTTP components
 		// to the chi router.
-		authenticationHTTPMount(ctx, cfg.Authentication, r, conn)
+		authenticationHTTPMount(ctx, logger, cfg.Authentication, r, conn)
 
 		r.Group(func(r chi.Router) {
 			r.Use(func(handler http.Handler) http.Handler {

diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go
@@ -1,8 +1,11 @@
 package gateway
 
 import (
+	"sync"
+
 	"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
 	"go.flipt.io/flipt/rpc/flipt"
+	"go.uber.org/zap"
 	"google.golang.org/protobuf/encoding/protojson"
 )
 
@@ -13,20 +16,26 @@ import (
 // See: rpc/flipt/marshal.go
 //
 // See: https://github.com/flipt-io/flipt/issues/664
-var commonMuxOptions = []runtime.ServeMuxOption{
-	runtime.WithMarshalerOption(runtime.MIMEWildcard, flipt.NewV1toV2MarshallerAdapter()),
-	runtime.WithMarshalerOption("application/json+pretty", &runtime.JSONPb{
-		MarshalOptions: protojson.MarshalOptions{
-			Indent:    "  ",
-			Multiline: true, // Optional, implied by presence of "Indent".
-		},
-		UnmarshalOptions: protojson.UnmarshalOptions{
-			DiscardUnknown: true,
-		},
-	}),
-}
+var commonMuxOptions []runtime.ServeMuxOption
+var once sync.Once
 
 // NewGatewayServeMux builds a new gateway serve mux with common options.
-func NewGatewayServeMux(opts ...runtime.ServeMuxOption) *runtime.ServeMux {
+func NewGatewayServeMux(logger *zap.Logger, opts ...runtime.ServeMuxOption) *runtime.ServeMux {
+	once.Do(func() {
+		commonMuxOptions = []runtime.ServeMuxOption{
+			runtime.WithMarshalerOption(runtime.MIMEWildcard, flipt.NewV1toV2MarshallerAdapter(logger)),
+			runtime.WithMarshalerOption("application/json+pretty", &runtime.JSONPb{
+				MarshalOptions: protojson.MarshalOptions{
+					Indent:    "  ",
+					Multiline: true, // Optional, implied by presence of "Indent".
+				},
+				UnmarshalOptions: protojson.UnmarshalOptions{
+					DiscardUnknown: true,
+				},
+			}),
+		}
+
+	})
+
 	return runtime.NewServeMux(append(commonMuxOptions, opts...)...)
 }
diff --git a/internal/server/auth/method/kubernetes/testing/http.go b/internal/server/auth/method/kubernetes/testing/http.go
@@ -26,7 +26,7 @@ func StartHTTPServer(
 	t.Helper()
 
 	var (
-		mux        = gateway.NewGatewayServeMux()
+		mux        = gateway.NewGatewayServeMux(logger)
 		httpServer = &HTTPServer{
 			GRPCServer: StartGRPCServer(t, ctx, logger, conf),
 		}

diff --git a/internal/server/auth/method/oidc/testing/http.go b/internal/server/auth/method/oidc/testing/http.go
@@ -34,6 +34,7 @@ func StartHTTPServer(
 
 		oidcmiddleware = oidc.NewHTTPMiddleware(conf.Session)
 		mux            = gateway.NewGatewayServeMux(
+			logger,
 			runtime.WithMetadata(oidc.ForwardCookies),
 			runtime.WithForwardResponseOption(oidcmiddleware.ForwardResponseOption),
 		)

diff --git a/rpc/flipt/marshaller.go b/rpc/flipt/marshaller.go
@@ -1,10 +1,13 @@
 package flipt
 
 import (
+	"encoding/json"
+	"errors"
 	"io"
 
 	grpc_gateway_v1 "github.com/grpc-ecosystem/grpc-gateway/runtime"
 	grpc_gateway_v2 "github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
+	"go.uber.org/zap"
 )
 
 var _ grpc_gateway_v2.Marshaler = &V1toV2MarshallerAdapter{}
@@ -20,10 +23,11 @@ var _ grpc_gateway_v2.Marshaler = &V1toV2MarshallerAdapter{}
 // TODO: remove this custom marshaller for Flipt API v2 as we want to use the default v2 marshaller directly.
 type V1toV2MarshallerAdapter struct {
 	*grpc_gateway_v1.JSONPb
+	logger *zap.Logger
 }
 
-func NewV1toV2MarshallerAdapter() *V1toV2MarshallerAdapter {
-	return &V1toV2MarshallerAdapter{&grpc_gateway_v1.JSONPb{OrigName: false, EmitDefaults: true}}
+func NewV1toV2MarshallerAdapter(logger *zap.Logger) *V1toV2MarshallerAdapter {
+	return &V1toV2MarshallerAdapter{&grpc_gateway_v1.JSONPb{OrigName: false, EmitDefaults: true}, logger}
 }
 
 func (m *V1toV2MarshallerAdapter) ContentType(_ interface{}) string {
@@ -34,8 +38,30 @@ func (m *V1toV2MarshallerAdapter) Marshal(v interface{}) ([]byte, error) {
 	return m.JSONPb.Marshal(v)
 }
 
+// decoderInterceptor intercepts and modifies the outbound error return value for
+// inputs that fail to unmarshal against the protobuf.
+type decoderInterceptor struct {
+	grpc_gateway_v1.Decoder
+	logger *zap.Logger
+}
+
+func (c *decoderInterceptor) Decode(v interface{}) error {
+	err := c.Decoder.Decode(v)
+	if err != nil {
+		c.logger.Debug("JSON decoding failed for inputs", zap.Error(err))
+
+		if _, ok := err.(*json.UnmarshalTypeError); ok {
+			return errors.New("invalid values for key(s) in json body")
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 func (m *V1toV2MarshallerAdapter) NewDecoder(r io.Reader) grpc_gateway_v2.Decoder {
-	return m.JSONPb.NewDecoder(r)
+	return &decoderInterceptor{Decoder: m.JSONPb.NewDecoder(r), logger: m.logger}
 }
 
 func (m *V1toV2MarshallerAdapter) NewEncoder(w io.Writer) grpc_gateway_v2.Encoder {

diff --git a/test/api.sh b/test/api.sh
@@ -263,6 +263,12 @@ step_5_test_evaluation()
         status 200
         matches "\"flagKey\":\"$flag_key\""
         matches "\"match\":false"
+
+    # evaluate returns 400 plus user friendly error message
+    authedShakedown POST "/api/v1/evaluate" -H 'Content-Type:application/json' -d "{\"flag_key\":\"$flag_key\",\"entity_id\":\"$(uuid_str)\",\"context\":\"hello\"}"
+        status 400
+        matches "\"code\":3"
+        contains "\"message\":\"invalid values for key(s) in json body\""
 }
 
 step_6_test_batch_evaluation()