From c3b10ec0ce2cc48607e8d66a59e3e93fbdaa9edd Mon Sep 17 00:00:00 2001 From: Lorenzo Canese Date: Sat, 7 Mar 2020 14:19:04 +0100 Subject: [PATCH 1/2] Add RequestID in error body --- transport/http/jsonrpc/server.go | 12 +++++++++- transport/http/jsonrpc/server_test.go | 34 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/transport/http/jsonrpc/server.go b/transport/http/jsonrpc/server.go index 0310f3edd..d942d3fe1 100644 --- a/transport/http/jsonrpc/server.go +++ b/transport/http/jsonrpc/server.go @@ -11,6 +11,8 @@ import ( httptransport "github.com/go-kit/kit/transport/http" ) +const requestIDKey = "request-id" + // Server wraps an endpoint and implements http.Handler. type Server struct { ecm EndpointCodecMap @@ -105,6 +107,8 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + ctx = context.WithValue(ctx, requestIDKey, req.ID) + // Get the endpoint and codecs from the map using the method // defined in the JSON object ecm, ok := s.ecm[req.Method] @@ -160,7 +164,7 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // If the error implements ErrorCoder, the provided code will be set on the // response error. // If the error implements Headerer, the given headers will be set. -func DefaultErrorEncoder(_ context.Context, err error, w http.ResponseWriter) { +func DefaultErrorEncoder(ctx context.Context, err error, w http.ResponseWriter) { w.Header().Set("Content-Type", ContentType) if headerer, ok := err.(httptransport.Headerer); ok { for k := range headerer.Headers() { @@ -177,7 +181,13 @@ func DefaultErrorEncoder(_ context.Context, err error, w http.ResponseWriter) { } w.WriteHeader(http.StatusOK) + + var requestID *RequestID + if ctx.Value(requestIDKey) != nil { + requestID = ctx.Value(requestIDKey).(*RequestID) + } _ = json.NewEncoder(w).Encode(Response{ + ID: requestID, JSONRPC: Version, Error: &e, }) diff --git a/transport/http/jsonrpc/server_test.go b/transport/http/jsonrpc/server_test.go index d7960fe05..2426f5653 100644 --- a/transport/http/jsonrpc/server_test.go +++ b/transport/http/jsonrpc/server_test.go @@ -24,9 +24,14 @@ func body(in string) io.Reader { return strings.NewReader(in) } -func expectErrorCode(t *testing.T, want int, body []byte) { +func unmarshalResponse(body []byte) (jsonrpc.Response, error) { var r jsonrpc.Response err := json.Unmarshal(body, &r) + return r, err +} + +func expectErrorCode(t *testing.T, want int, body []byte) { + r, err := unmarshalResponse(body) if err != nil { t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) } @@ -38,6 +43,30 @@ func expectErrorCode(t *testing.T, want int, body []byte) { } } +func expectValidRequestID(t *testing.T, want int, body []byte) { + r, err := unmarshalResponse(body) + if err != nil { + t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) + } + have, err := r.ID.Int() + if err != nil { + t.Fatalf("Cant' get requestID in response. err=%s, body=%s", err, body) + } + if want != have { + t.Fatalf("Unexpected request ID. Want %d, have %d: %s", want, have, body) + } +} + +func expectNilRequestID(t *testing.T, body []byte) { + r, err := unmarshalResponse(body) + if err != nil { + t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) + } + if nil != r.ID { + t.Fatalf("Unexpected request ID. Want nil, have %v", r.ID) + } +} + func nopDecoder(context.Context, json.RawMessage) (interface{}, error) { return struct{}{}, nil } func nopEncoder(context.Context, interface{}) (json.RawMessage, error) { return []byte("[]"), nil } @@ -92,6 +121,7 @@ func TestServerBadEndpoint(t *testing.T) { } buf, _ := ioutil.ReadAll(resp.Body) expectErrorCode(t, jsonrpc.InternalError, buf) + expectValidRequestID(t, 1, buf) } func TestServerBadEncode(t *testing.T) { @@ -111,6 +141,7 @@ func TestServerBadEncode(t *testing.T) { } buf, _ := ioutil.ReadAll(resp.Body) expectErrorCode(t, jsonrpc.InternalError, buf) + expectValidRequestID(t, 1, buf) } func TestServerErrorEncoder(t *testing.T) { @@ -162,6 +193,7 @@ func TestCanRejectInvalidJSON(t *testing.T) { } buf, _ := ioutil.ReadAll(resp.Body) expectErrorCode(t, jsonrpc.ParseError, buf) + expectNilRequestID(t, buf) } func TestServerUnregisteredMethod(t *testing.T) { From 5968c8276c8879d2afaeb52bb99bec1905471fb8 Mon Sep 17 00:00:00 2001 From: Lorenzo Canese Date: Fri, 20 Mar 2020 14:09:32 +0100 Subject: [PATCH 2/2] Implementing review suggestions --- transport/http/jsonrpc/server.go | 8 ++++--- transport/http/jsonrpc/server_test.go | 32 +++++++++++++++------------ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/transport/http/jsonrpc/server.go b/transport/http/jsonrpc/server.go index d942d3fe1..75666e4bf 100644 --- a/transport/http/jsonrpc/server.go +++ b/transport/http/jsonrpc/server.go @@ -11,7 +11,9 @@ import ( httptransport "github.com/go-kit/kit/transport/http" ) -const requestIDKey = "request-id" +type requestIDKeyType struct{} + +var requestIDKey requestIDKeyType // Server wraps an endpoint and implements http.Handler. type Server struct { @@ -183,8 +185,8 @@ func DefaultErrorEncoder(ctx context.Context, err error, w http.ResponseWriter) w.WriteHeader(http.StatusOK) var requestID *RequestID - if ctx.Value(requestIDKey) != nil { - requestID = ctx.Value(requestIDKey).(*RequestID) + if v := ctx.Value(requestIDKey); v != nil { + requestID = v.(*RequestID) } _ = json.NewEncoder(w).Encode(Response{ ID: requestID, diff --git a/transport/http/jsonrpc/server_test.go b/transport/http/jsonrpc/server_test.go index 2426f5653..c279d3c92 100644 --- a/transport/http/jsonrpc/server_test.go +++ b/transport/http/jsonrpc/server_test.go @@ -24,16 +24,17 @@ func body(in string) io.Reader { return strings.NewReader(in) } -func unmarshalResponse(body []byte) (jsonrpc.Response, error) { - var r jsonrpc.Response - err := json.Unmarshal(body, &r) - return r, err +func unmarshalResponse(body []byte) (resp jsonrpc.Response, err error) { + err = json.Unmarshal(body, &resp) + return } func expectErrorCode(t *testing.T, want int, body []byte) { + t.Helper() + r, err := unmarshalResponse(body) if err != nil { - t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) + t.Fatalf("Can't decode response: %v (%s)", err, body) } if r.Error == nil { t.Fatalf("Expected error on response. Got none: %s", body) @@ -44,26 +45,30 @@ func expectErrorCode(t *testing.T, want int, body []byte) { } func expectValidRequestID(t *testing.T, want int, body []byte) { + t.Helper() + r, err := unmarshalResponse(body) if err != nil { - t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) + t.Fatalf("Can't decode response: %v (%s)", err, body) } have, err := r.ID.Int() if err != nil { - t.Fatalf("Cant' get requestID in response. err=%s, body=%s", err, body) + t.Fatalf("Can't get requestID in response. err=%s, body=%s", err, body) } if want != have { - t.Fatalf("Unexpected request ID. Want %d, have %d: %s", want, have, body) + t.Fatalf("Request ID: want %d, have %d (%s)", want, have, body) } } func expectNilRequestID(t *testing.T, body []byte) { + t.Helper() + r, err := unmarshalResponse(body) if err != nil { - t.Fatalf("Cant' decode response. err=%s, body=%s", err, body) + t.Fatalf("Can't decode response: %v (%s)", err, body) } - if nil != r.ID { - t.Fatalf("Unexpected request ID. Want nil, have %v", r.ID) + if r.ID != nil { + t.Fatalf("Request ID: want nil, have %v", r.ID) } } @@ -218,10 +223,9 @@ func TestServerHappyPath(t *testing.T) { if want, have := http.StatusOK, resp.StatusCode; want != have { t.Errorf("want %d, have %d (%s)", want, have, buf) } - var r jsonrpc.Response - err := json.Unmarshal(buf, &r) + r, err := unmarshalResponse(buf) if err != nil { - t.Fatalf("Cant' decode response. err=%s, body=%s", err, buf) + t.Fatalf("Can't decode response. err=%s, body=%s", err, buf) } if r.JSONRPC != jsonrpc.Version { t.Fatalf("JSONRPC Version: want=%s, got=%s", jsonrpc.Version, r.JSONRPC)