From efd10d87065b959c8cb406abcca22a44a1ad1a63 Mon Sep 17 00:00:00 2001 From: TRAVIS ALLEN SALAS COX Date: Wed, 17 May 2017 21:17:02 -0500 Subject: [PATCH 1/2] added client finalizer based on the server finazlizer --- transport/http/client.go | 33 +++++++++++++++++++-- transport/http/client_test.go | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/transport/http/client.go b/transport/http/client.go index 08f1b886e..88581a386 100644 --- a/transport/http/client.go +++ b/transport/http/client.go @@ -23,6 +23,7 @@ type Client struct { dec DecodeResponseFunc before []RequestFunc after []ClientResponseFunc + finalizer ClientFinalizerFunc bufferedStream bool } @@ -72,6 +73,12 @@ func ClientAfter(after ...ClientResponseFunc) ClientOption { return func(c *Client) { c.after = append(c.after, after...) } } +// ClientFinalizer is executed at the end of every HTTP request. +// By default, no finalizer is registered. +func ClientFinalizer(f ClientFinalizerFunc) ClientOption { + return func(s *Client) { s.finalizer = f } +} + // BufferedStream sets whether the Response.Body is left open, allowing it // to be read from later. Useful for transporting a file as a buffered stream. func BufferedStream(buffered bool) ClientOption { @@ -84,7 +91,21 @@ func (c Client) Endpoint() endpoint.Endpoint { ctx, cancel := context.WithCancel(ctx) defer cancel() - req, err := http.NewRequest(c.method, c.tgt.String(), nil) + // Vars used for client finalizer to ensure there are no nil values + var ( + req *http.Request = &http.Request{} + resp *http.Response = &http.Response{} + err error + ) + if c.finalizer != nil { + defer func() { + ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header) + ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength) + c.finalizer(ctx, resp.StatusCode, req) + }() + } + + req, err = http.NewRequest(c.method, c.tgt.String(), nil) if err != nil { return nil, err } @@ -97,10 +118,11 @@ func (c Client) Endpoint() endpoint.Endpoint { ctx = f(ctx, req) } - resp, err := ctxhttp.Do(ctx, c.client, req) + resp, err = ctxhttp.Do(ctx, c.client, req) if err != nil { return nil, err } + if !c.bufferedStream { defer resp.Body.Close() } @@ -118,6 +140,13 @@ func (c Client) Endpoint() endpoint.Endpoint { } } +// ClientFinalizerFunc can be used to perform work at the end of a client HTTP +// request, after the response is returned. The principal +// intended use is for request logging. In addition to the response code +// provided in the function signature, additional response parameters are +// provided in the context under keys with the ContextKeyResponse prefix. +type ClientFinalizerFunc func(ctx context.Context, code int, r *http.Request) + // EncodeJSONRequest is an EncodeRequestFunc that serializes the request as a // JSON object to the Request body. Many JSON-over-HTTP services can use it as // a sensible default. If the request implements Headerer, the provided headers diff --git a/transport/http/client_test.go b/transport/http/client_test.go index ad366d1ac..9a50cc8f4 100644 --- a/transport/http/client_test.go +++ b/transport/http/client_test.go @@ -140,6 +140,62 @@ func TestHTTPClientBufferedStream(t *testing.T) { } } +func TestClientFinalizer(t *testing.T) { + var ( + headerKey = "X-Henlo-Lizer" + headerVal = "Helllo you stinky lizard" + statusCode = http.StatusTeapot + responseBody = "go eat a fly ugly\n" + done = make(chan struct{}) + encode = func(context.Context, *http.Request, interface{}) error { return nil } + decode = func(_ context.Context, r *http.Response) (interface{}, error) { + return TestResponse{r.Body, ""}, nil + } + ) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerKey, headerVal) + w.WriteHeader(statusCode) + w.Write([]byte(responseBody)) + })) + defer server.Close() + + client := httptransport.NewClient( + "GET", + mustParse(server.URL), + encode, + decode, + httptransport.ClientFinalizer(func(ctx context.Context, code int, _ *http.Request) { + if want, have := statusCode, code; want != have { + t.Errorf("StatusCode: want %d, have %d", want, have) + } + + responseHeader := ctx.Value(httptransport.ContextKeyResponseHeaders).(http.Header) + if want, have := headerVal, responseHeader.Get(headerKey); want != have { + t.Errorf("%s: want %q, have %q", headerKey, want, have) + } + + responseSize := ctx.Value(httptransport.ContextKeyResponseSize).(int64) + if want, have := int64(len(responseBody)), responseSize; want != have { + t.Errorf("response size: want %d, have %d", want, have) + } + + close(done) + }), + ) + + _, err := client.Endpoint()(context.Background(), struct{}{}) + if err != nil { + t.Fatal(err) + } + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("timeout waiting for finalizer") + } +} + func TestEncodeJSONRequest(t *testing.T) { var header http.Header var body string From fd2bb1e8062fc20d0394c06975b2a6834ed30b47 Mon Sep 17 00:00:00 2001 From: TRAVIS ALLEN SALAS COX Date: Mon, 22 May 2017 18:09:31 -0500 Subject: [PATCH 2/2] update based on comments update based on comments to previous commit. --- transport/http/client.go | 21 +++++++++++---------- transport/http/client_test.go | 8 +------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/transport/http/client.go b/transport/http/client.go index 88581a386..c5dace97b 100644 --- a/transport/http/client.go +++ b/transport/http/client.go @@ -91,21 +91,21 @@ func (c Client) Endpoint() endpoint.Endpoint { ctx, cancel := context.WithCancel(ctx) defer cancel() - // Vars used for client finalizer to ensure there are no nil values var ( - req *http.Request = &http.Request{} - resp *http.Response = &http.Response{} + resp *http.Response err error ) if c.finalizer != nil { defer func() { - ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header) - ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength) - c.finalizer(ctx, resp.StatusCode, req) + if resp != nil { + ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header) + ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength) + } + c.finalizer(ctx, err) }() } - req, err = http.NewRequest(c.method, c.tgt.String(), nil) + req, err := http.NewRequest(c.method, c.tgt.String(), nil) if err != nil { return nil, err } @@ -142,10 +142,11 @@ func (c Client) Endpoint() endpoint.Endpoint { // ClientFinalizerFunc can be used to perform work at the end of a client HTTP // request, after the response is returned. The principal -// intended use is for request logging. In addition to the response code -// provided in the function signature, additional response parameters are +// intended use is for error logging. Additional response parameters are // provided in the context under keys with the ContextKeyResponse prefix. -type ClientFinalizerFunc func(ctx context.Context, code int, r *http.Request) +// Note: err may be nil. There maybe also no additional response parameters depending on +// when an error occurs. +type ClientFinalizerFunc func(ctx context.Context, err error) // EncodeJSONRequest is an EncodeRequestFunc that serializes the request as a // JSON object to the Request body. Many JSON-over-HTTP services can use it as diff --git a/transport/http/client_test.go b/transport/http/client_test.go index 9a50cc8f4..d66381000 100644 --- a/transport/http/client_test.go +++ b/transport/http/client_test.go @@ -144,7 +144,6 @@ func TestClientFinalizer(t *testing.T) { var ( headerKey = "X-Henlo-Lizer" headerVal = "Helllo you stinky lizard" - statusCode = http.StatusTeapot responseBody = "go eat a fly ugly\n" done = make(chan struct{}) encode = func(context.Context, *http.Request, interface{}) error { return nil } @@ -155,7 +154,6 @@ func TestClientFinalizer(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set(headerKey, headerVal) - w.WriteHeader(statusCode) w.Write([]byte(responseBody)) })) defer server.Close() @@ -165,11 +163,7 @@ func TestClientFinalizer(t *testing.T) { mustParse(server.URL), encode, decode, - httptransport.ClientFinalizer(func(ctx context.Context, code int, _ *http.Request) { - if want, have := statusCode, code; want != have { - t.Errorf("StatusCode: want %d, have %d", want, have) - } - + httptransport.ClientFinalizer(func(ctx context.Context, err error) { responseHeader := ctx.Value(httptransport.ContextKeyResponseHeaders).(http.Header) if want, have := headerVal, responseHeader.Get(headerKey); want != have { t.Errorf("%s: want %q, have %q", headerKey, want, have)