Create packages client/internal/ retry and lhttp.

client/internal/lhttp/client.go

Index: client/internal/lhttp/client.go
diff --git a/client/internal/lhttp/client.go b/client/internal/lhttp/client.go
new file mode 100644
index 0000000000000000000000000000000000000000..c80690f768f8c2610b076ae5061de28807caf150
--- /dev/null
+++ b/client/internal/lhttp/client.go
@@ -0,0 +1,172 @@
+// 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)
+}