Create packages client/internal/ retry and lhttp.

client/internal/lhttp/client.go

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package lhttp
+
+import (
+        "bytes"
+        "encoding/json"
+        "errors"
+        "fmt"
+        "io"
+        "io/ioutil"
+        "net/http"
+        "net/url"
+        "os"
+        "strings"
+
+        "github.com/luci/luci-go/client/internal/retry"
+)
+
+// Handler is called once or multiple times for each HTTP request that is tried.
+type Handler func(*http.Response) error
+
+// NewRequest returns a retriable request.
+//
+// To enable automatic retry support, the Request.Body, if present, must
+// implement io.Seeker.
+//
+// handler should return retry.Error in case of retriable error, for example if
+// a TCP connection is teared off while receiving the content.
+func NewRequest(c *http.Client, req *http.Request, handler Handler) (retry.Retriable, error) {
+        // Handle req.Body if specified. It has to implement io.Seeker.
+        newReq := *req
+        out := &httpRetriable{
+                handler:   handler,
+                c:         c,
+                req:       &newReq,
+                closeBody: req.Body,
+        }
+        if req.Body != nil {
+                ok := false
+                if out.seekBody, ok = req.Body.(io.Seeker); !ok {
+                        return nil, errors.New("req.Body must implement io.Seeker")
+                }
+                // Make sure the body is not closed when calling http.Client.Do().
+                out.req.Body = ioutil.NopCloser(req.Body)
+        }
+        return out, nil
+}
+
+// NewRequestJSON returns a retriable request calling a JSON endpoint.
+func NewRequestJSON(c *http.Client, url, method string, in, out interface{}) (retry.Retriable, error) {
+        var body io.Reader
+        if in != nil {
+                encoded, err := json.Marshal(in)
+                if err != nil {
+                        return nil, err
+                }
+                body = NewReader(encoded)
+        }
+        req, err := http.NewRequest(method, url, body)
+        if err != nil {
+                return nil, err
+        }
+        if in != nil {
+                req.Header.Set("Content-Type", jsonContentType)
+        }
+        return NewRequest(c, req, func(resp *http.Response) error {
+                defer resp.Body.Close()
+                if ct := strings.ToLower(resp.Header.Get("Content-Type")); ct != jsonContentType {
+                        // Non-retriable.
+                        return fmt.Errorf("unexpected Content-Type, expected \"%s\", got \"%s\"", jsonContentType, ct)
+                }
+                if out == nil {
+                        // The client doesn't care about the response. Still ensure the response
+                        // is valid json.
+                        out = &map[string]interface{}{}
+                }
+                if err := json.NewDecoder(resp.Body).Decode(out); err != nil {
+                        // Retriable.

[Vadim Sh., 2015/05/13 21:54:47]

ideally JSON formatting errors and network related errors should be
distinguished somehow :( Stupid Go makes it difficult.

[M-A Ruel, 2015/05/14 01:00:54]

Actually, you just create typed errors. That's exactly what I do with
retry.Error. The problem is the URL parsing that can fail but I added code to
check it earlier instead.

+                        return retry.Error{fmt.Errorf("bad response %s: %s", url, err)}
+                }
+                return nil
+        })
+}
+
+// GetJSON is a shorthand.
+func GetJSON(config *retry.Config, c *http.Client, url string, out interface{}) error {
+        req, err := NewRequestJSON(c, url, "GET", nil, out)
+        if err != nil {
+                return err
+        }
+        return config.Do(req)
+}
+
+// PostJSON is a shorthand.
+func PostJSON(config *retry.Config, c *http.Client, url string, in, out interface{}) error {
+        req, err := NewRequestJSON(c, url, "POST", in, out)
+        if err != nil {
+                return err
+        }
+        return config.Do(req)
+}
+
+// NewReader returns a io.ReadCloser compatible read-only buffer that also

[Vadim Sh., 2015/05/13 21:54:47]

I think it's better to keep it private. lhttp/client is not an obvious place to
find this sort of functions, so no need to expose it here.

[M-A Ruel, 2015/05/14 01:00:54]

Ok but it's possible we may need to expose it for non JSON posts.

+// exposes io.Seeker. This should be used instead of bytes.NewReader(), which
+// doesn't implement Close().
+func NewReader(p []byte) io.ReadCloser {
+        return &reader{bytes.NewReader(p)}
+}
+
+// Private details.
+
+const jsonContentType = "application/json; charset=utf-8"
+
+type reader struct {
+        *bytes.Reader
+}
+
+func (r *reader) Close() error {
+        return nil
+}
+
+type httpRetriable struct {
+        handler   Handler
+        c         *http.Client
+        req       *http.Request
+        closeBody io.Closer
+        seekBody  io.Seeker
+}
+
+func (h *httpRetriable) Close() error {
+        if h.closeBody != nil {
+                return h.closeBody.Close()
+        }
+        return nil
+}
+
+// Warning: it returns an error on HTTP >=400. This is different than
+// http.Client.Do() but hell it makes coding simpler.
+func (h *httpRetriable) Do() error {
+        if h.seekBody != nil {
+                if _, err := h.seekBody.Seek(0, os.SEEK_SET); err != nil {
+                        // Can't be retried.
+                        return err
+                }
+        }
+        resp, err := h.c.Do(h.req)
+        if err != nil {
+                // Any TCP level failure can be retried but malformed URL should nt.
+                if err2, ok := err.(*url.Error); ok {
+                        return err2
+                }
+                // http.badStringError{} is not exported. Hack around.
+                if strings.HasPrefix(err.Error(), "unsupported protocol scheme") {

[Vadim Sh., 2015/05/13 21:54:47]

that's what I meant when I was saying "stupid go" :)

[M-A Ruel, 2015/05/14 01:00:54]

Removed since I added a check earlier. It's only an issue because the error type
is wrapped.

+                        return err
+                }
+                return retry.Error{err}
+        }
+        // If the HTTP status code means the request should be retried.
+        if resp.StatusCode == 408 || resp.StatusCode == 429 || resp.StatusCode >= 500 {
+                return retry.Error{fmt.Errorf("http request failed: %s (HTTP %d)", http.StatusText(resp.StatusCode), resp.StatusCode)}
+        }
+        // Any other failure code is a hard failure.
+        if resp.StatusCode >= 400 {

[Vadim Sh., 2015/05/13 21:54:47]

I may make sense to return ErrNotFound on 404 and ErrNotAuthorized on 401 and
403.

[M-A Ruel, 2015/05/14 01:00:54]

Changed to add an interface instead if needed. The interface exposes Status(). I
had started using an interface initially, removed it and now added it back. It
permits adding more stateful information.

For the JSON functions, I just added status as a return code.

+                return fmt.Errorf("http request failed: %s (HTTP %d)", http.StatusText(resp.StatusCode), resp.StatusCode)
+        }
+        // The handler may still return a retry.Error to indicate that the request
+        // should be retried even on successful status code.
+        return h.handler(resp)
+}