From 7accf1942cca5acde9586e5998f647f94bfeb60e Mon Sep 17 00:00:00 2001 From: ysugimoto Date: Fri, 7 Aug 2020 03:03:46 +0900 Subject: [PATCH 1/4] Define custom error on middleware and use it as GraphQL error --- runtime/middlewares.go | 16 ++++++++++++ runtime/mux.go | 58 +++++++++++++++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/runtime/middlewares.go b/runtime/middlewares.go index 3096083..9572d33 100644 --- a/runtime/middlewares.go +++ b/runtime/middlewares.go @@ -5,6 +5,22 @@ import ( "net/http" ) +type MiddlewareError struct { + Code string + Message string +} + +func (m *MiddlewareError) Error() string { + return m.Message +} + +func NewMiddlewareError(code, message string) *MiddlewareError { + return &MiddlewareError{ + Code: code, + Message: message, + } +} + // Cors is middelware function to provide CORS headers to response headers func Cors() MiddlewareFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error) { diff --git a/runtime/mux.go b/runtime/mux.go index ead9c15..7bce99a 100644 --- a/runtime/mux.go +++ b/runtime/mux.go @@ -96,7 +96,21 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error ctx, err = m(ctx, w, r) if err != nil { - http.Error(w, "middleware error occurred: "+err.Error(), http.StatusInternalServerError) + ge := gqlerrors.FormattedError{} + if me, ok := err.(*MiddlewareError); ok { + ge.Message = me.Message + ge.Extensions = map[string]interface{}{ + "code": me.Code, + } + } else { + ge.Message = err.Error() + ge.Extensions = map[string]interface{}{ + "code": "MIDDLEWARE_ERROR", + } + } + respondResult(w, &graphql.Result{ + Errors: []gqlerrors.FormattedError{ge}, + }) return } } @@ -106,7 +120,16 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { for _, h := range s.handlers { c, closer, err := h.CreateConnection(ctx) if err != nil { - http.Error(w, "failed to create grpc connection: "+err.Error(), http.StatusInternalServerError) + respondResult(w, &graphql.Result{ + Errors: []gqlerrors.FormattedError{ + { + Message: "Failed to create grpc connection: " + err.Error(), + Extensions: map[string]interface{}{ + "code": "GRPC_CONNECT_ERROR", + }, + }, + }, + }) return } defer closer() @@ -135,13 +158,31 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { schema, err := graphql.NewSchema(schemaConfig) if err != nil { - http.Error(w, "failed to build schema: "+err.Error(), http.StatusInternalServerError) + respondResult(w, &graphql.Result{ + Errors: []gqlerrors.FormattedError{ + { + Message: "Failed to build schema: " + err.Error(), + Extensions: map[string]interface{}{ + "code": "SCHEMA_GENERATION_ERROR", + }, + }, + }, + }) return } req, err := parseRequest(r) if err != nil { - http.Error(w, "failed to parse request: "+err.Error(), http.StatusInternalServerError) + respondResult(w, &graphql.Result{ + Errors: []gqlerrors.FormattedError{ + { + Message: "Failed to parse request: " + err.Error(), + Extensions: map[string]interface{}{ + "code": "REQUEST_PARSE_ERROR", + }, + }, + }, + }) return } @@ -157,12 +198,11 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.ErrorHandler(result.Errors) } } + respondResult(w, result) +} - out, err := json.Marshal(result) - if err != nil { - http.Error(w, "failed to marshal response JSON: "+err.Error(), http.StatusInternalServerError) - return - } +func respondResult(w http.ResponseWriter, result *graphql.Result) { + out, _ := json.Marshal(result) w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Length", fmt.Sprint(len(out))) From 1af55e45ebf7ce01c06b159b10871588bd911a20 Mon Sep 17 00:00:00 2001 From: ysugimoto Date: Fri, 7 Aug 2020 03:34:11 +0900 Subject: [PATCH 2/4] add test and divide changelog file --- CHANGELOG.md | 36 ++++++++++++++++++++ README.md | 9 ----- go.mod | 1 + go.sum | 10 ++++++ runtime/middlewares_test.go | 68 +++++++++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 runtime/middlewares_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d4392dd --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,36 @@ +# CHANGELOG + +## v0.12.0 + +### Define MiddlewareError and respond with error code + +We defined `MiddleWareError` which can return in Middleware function. If middleware function returns that pointer instead of common error, +The runtime responds error with that field data. + +The definition is: + +```go +type MiddlewareError struct { + Code string + Message string +} + +// generate error +return runtime.NewMiddlewareError("CODE", "MESSAGE") +``` + +```json +// If middleware returns common error, the runtime error will be: +{"data": null, "errors": [{"message": "error message of err.Error()", "extensions": {"code": "MIDDLEWARE_ERROR"}]} + +// If middleware returns `MiddlewareError`, the runtime error will be: +{"data": null, "errors": [{"message": "Message field value of MiddlewareError", "extensions": {"code": "Code field value of MiddlewareError"}]} +``` + + +## v0.9.1 + +### Changed middleware fucntion type + +On MiddlewareFunc, you need to return `context.Context` as first return value. this is because we need to make custom metadata to gRPC on middleware process. +If you are already using your onw middleware, plase change its interface. see https://github.com/ysugimoto/grpc-graphql-gateway/pull/10 diff --git a/README.md b/README.md index a9d7c70..6b72c98 100644 --- a/README.md +++ b/README.md @@ -6,15 +6,6 @@ ![image](https://raw.githubusercontent.com/ysugimoto/grpc-graphql-gateway/master/misc/grpc-graphql-gateway.png) -## Change Log - -### v0.9.1 - -#### Changed middleware fucntion type - -On MiddlewareFunc, you need to return `context.Context` as first return value. this is because we need to make custom metadata to gRPC on middleware process. -If you are already using your onw middleware, plase change its interface. see https://github.com/ysugimoto/grpc-graphql-gateway/pull/10 - ## Motivation On API development, frequently we choose some IDL, in order to manage API definitions from a file. diff --git a/go.mod b/go.mod index 8da50bf..f3967c2 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/k0kubun/pp v3.0.1+incompatible // indirect github.com/mattn/go-colorable v0.1.4 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/stretchr/testify v1.6.1 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect google.golang.org/grpc v1.27.0 diff --git a/go.sum b/go.sum index 8c16c4f..370410c 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= @@ -29,7 +31,12 @@ github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -70,5 +77,8 @@ google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQ google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE= google.golang.org/protobuf v1.21.0 h1:qdOKuR/EIArgaWNjetjgTzgVTAZ+S/WXVrq9HW9zimw= google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/runtime/middlewares_test.go b/runtime/middlewares_test.go new file mode 100644 index 0000000..9c8cbad --- /dev/null +++ b/runtime/middlewares_test.go @@ -0,0 +1,68 @@ +package runtime + +import ( + "context" + "errors" + "testing" + + "encoding/json" + "net/http" + "net/http/httptest" + + "github.com/graphql-go/graphql" + "github.com/stretchr/testify/assert" +) + +func TestMiddlewareError(t *testing.T) { + t.Run("Set message field on common error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mux := NewServeMux() + mux.Use(func(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error) { + return ctx, errors.New("error") + }) + mux.ServeHTTP(w, r) + })) + defer srv.Close() + resp, err := http.Get(srv.URL) + assert.NoError(t, err) + defer resp.Body.Close() + + var r graphql.Result + err = json.NewDecoder(resp.Body).Decode(&r) + assert.NoError(t, err) + assert.Nil(t, r.Data) + if assert.Len(t, r.Errors, 1) { + e := r.Errors[0] + assert.Equal(t, "error", e.Message) + if assert.Len(t, e.Extensions, 1) { + assert.Equal(t, "MIDDLEWARE_ERROR", e.Extensions["code"]) + } + } + }) + + t.Run("Set message field on MiddewareError", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mux := NewServeMux() + mux.Use(func(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error) { + return ctx, NewMiddlewareError("CUSTOM_CODE", "CUSTOM_MESSAGE") + }) + mux.ServeHTTP(w, r) + })) + defer srv.Close() + resp, err := http.Get(srv.URL) + assert.NoError(t, err) + defer resp.Body.Close() + + var r graphql.Result + err = json.NewDecoder(resp.Body).Decode(&r) + assert.NoError(t, err) + assert.Nil(t, r.Data) + if assert.Len(t, r.Errors, 1) { + e := r.Errors[0] + assert.Equal(t, "CUSTOM_MESSAGE", e.Message) + if assert.Len(t, e.Extensions, 1) { + assert.Equal(t, "CUSTOM_CODE", e.Extensions["code"]) + } + } + }) +} From 0c622939f9302ff5478632f7a35c26fdcebb7418 Mon Sep 17 00:00:00 2001 From: ysugimoto Date: Fri, 7 Aug 2020 03:35:28 +0900 Subject: [PATCH 3/4] Modify CHANGELOG --- CHANGELOG.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4392dd..298a626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,15 @@ type MiddlewareError struct { return runtime.NewMiddlewareError("CODE", "MESSAGE") ``` -```json -// If middleware returns common error, the runtime error will be: +If middleware returns common error, the runtime error will be: + +``` {"data": null, "errors": [{"message": "error message of err.Error()", "extensions": {"code": "MIDDLEWARE_ERROR"}]} +``` -// If middleware returns `MiddlewareError`, the runtime error will be: +If middleware returns `MiddlewareError`, the runtime error will be: + +``` {"data": null, "errors": [{"message": "Message field value of MiddlewareError", "extensions": {"code": "Code field value of MiddlewareError"}]} ``` From 8d81ea72fda43a0bb1b7272ff581471bf6e22aac Mon Sep 17 00:00:00 2001 From: ysugimoto Date: Fri, 7 Aug 2020 03:38:50 +0900 Subject: [PATCH 4/4] add test task in make --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b7c72e4..ad9aa98 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,10 @@ plugin: mv graphql/github.com/ysugimoto/grpc-graphql-gateway/graphql/graphql.pb.go graphql/ rm -rf graphql/github.com -build: +test: + go list ./... | xargs go test + +build: test protoc -I google \ -I include/graphql \ --go_out=./graphql \