Add debug logging to all HTTP requests

go/cmd/entrypoint/client.go

@@ -1,14 +1,9 @@
 package main
 
 import (
-	"fmt"
 	"isle/bootstrap"
 )
 
 func (ctx subCmdCtx) getHosts() ([]bootstrap.Host, error) {
-	res, err := ctx.getDaemonRPC().GetHosts(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("calling GetHosts: %w", err)
-	}
-	return res, nil
+	return ctx.getDaemonRPC().GetHosts(ctx)
 }

go/cmd/entrypoint/sub_cmd.go

@@ -8,6 +8,7 @@ import (
 	"isle/daemon"
 	"isle/daemon/jsonrpc2"
 	"isle/jsonutil"
+	"isle/toolkit"
 	"os"
 	"strings"
 
@@ -95,10 +96,16 @@ func usagePrefix(subCmdNames []string) string {
 
 func (ctx subCmdCtx) getDaemonRPC() daemon.RPC {
 	if ctx.opts.daemonRPC == nil {
+		// TODO Close is not being called on the HTTPClient
+		httpClient, baseURL := toolkit.NewUnixHTTPClient(
+			ctx.logger.WithNamespace("http-client"),
+			daemon.HTTPSocketPath(),
+		)
+
+		baseURL.Path = daemonHTTPRPCPath
+
 		ctx.opts.daemonRPC = daemon.RPCFromClient(
-			jsonrpc2.NewUnixHTTPClient(
-				daemon.HTTPSocketPath(), daemonHTTPRPCPath,
-			),
+			jsonrpc2.NewHTTPClient(httpClient, baseURL.String()),
 		)
 	}
 	return ctx.opts.daemonRPC

go/daemon/jsonrpc2/client_http.go

@@ -6,50 +6,28 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"isle/toolkit"
 	"net/http"
-	"net/url"
-
-	"github.com/tv42/httpunix"
 )
 
-type httpClient struct {
-	c   *http.Client
+// HTTPClient makes JSONRPC2 requests over an HTTP endpoint.
+//
+// Close should be called once the HTTPClient is not longer needed.
+type HTTPClient struct {
+	c   toolkit.HTTPClient
 	url string
 }
 
-// NewHTTPClient returns a Client which will use HTTP POST requests against the
-// given URL as a transport for JSONRPC2 method calls.
-func NewHTTPClient(urlStr string) Client {
-	return &httpClient{
-		c: &http.Client{
-			Transport: http.DefaultTransport.(*http.Transport).Clone(),
-		},
+// NewHTTPClient returns an HTTPClient which will use HTTP POST requests against
+// the given URL as a transport for JSONRPC2 method calls.
+func NewHTTPClient(httpClient toolkit.HTTPClient, urlStr string) *HTTPClient {
+	return &HTTPClient{
+		c:   httpClient,
 		url: urlStr,
 	}
 }
 
-// NewUnixHTTPClient returns a Client which will use HTTP POST requests against
-// the given unix socket as a transport for JSONRPC2 method calls. The given
-// path will be used as the path portion of the HTTP request.
-func NewUnixHTTPClient(unixSocketPath, reqPath string) Client {
-	const host = "uds"
-
-	u := &url.URL{
-		Scheme: httpunix.Scheme,
-		Host:   host,
-		Path:   reqPath,
-	}
-
-	transport := new(httpunix.Transport)
-	transport.RegisterLocation(host, unixSocketPath)
-
-	return &httpClient{
-		c:   &http.Client{Transport: transport},
-		url: u.String(),
-	}
-}
-
-func (c *httpClient) Call(
+func (c *HTTPClient) Call(
 	ctx context.Context, rcv any, method string, args ...any,
 ) error {
 	var (
@@ -88,3 +66,7 @@ func (c *httpClient) Call(
 
 	return nil
 }
+
+func (c *HTTPClient) Close() error {
+	return c.c.Close()
+}

go/daemon/jsonrpc2/jsonrpc2_test.go

@@ -12,6 +12,8 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 type DivideParams struct {
@@ -216,13 +218,17 @@ func TestReadWriter(t *testing.T) {
 }
 
 func TestHTTP(t *testing.T) {
+	logger := toolkit.NewTestLogger(t)
 	server := httptest.NewServer(NewHTTPHandler(testHandler(t)))
 	t.Cleanup(server.Close)
-	testClient(t, NewHTTPClient(server.URL))
+	httpClient := toolkit.NewHTTPClient(logger)
+	t.Cleanup(func() { assert.NoError(t, httpClient.Close()) })
+	testClient(t, NewHTTPClient(httpClient, server.URL))
 }
 
 func TestUnixHTTP(t *testing.T) {
 	var (
+		logger         = toolkit.NewTestLogger(t)
 		unixSocketPath = filepath.Join(t.TempDir(), "test.sock")
 		server         = httptest.NewUnstartedServer(NewHTTPHandler(testHandler(t)))
 	)
@@ -235,5 +241,8 @@ func TestUnixHTTP(t *testing.T) {
 	server.Start()
 	t.Cleanup(server.Close)
 
-	testClient(t, NewUnixHTTPClient(unixSocketPath, "/"))
+	httpClient, baseURL := toolkit.NewUnixHTTPClient(logger, unixSocketPath)
+	t.Cleanup(func() { assert.NoError(t, httpClient.Close()) })
+
+	testClient(t, NewHTTPClient(httpClient, baseURL.String()))
 }

go/garage/admin_client.go

@@ -6,8 +6,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"isle/toolkit"
 	"net/http"
-	"net/http/httputil"
 	"net/netip"
 	"time"
 
@@ -51,35 +51,31 @@ func (e AdminClientError) Error() string {
 // interface.
 type AdminClient struct {
 	logger     *mlog.Logger
-	c          *http.Client
+	c          toolkit.HTTPClient
 	addr       string
 	adminToken string
-
-	transport *http.Transport
 }
 
 // NewAdminClient initializes and returns an AdminClient which will use the
 // given address and adminToken for all requests made.
 //
 // If Logger is nil then logs will be suppressed.
-func NewAdminClient(logger *mlog.Logger, addr, adminToken string) *AdminClient {
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-
+func NewAdminClient(
+	logger *mlog.Logger,
+	addr, adminToken string,
+) *AdminClient {
+	httpClient := toolkit.NewHTTPClient(logger.WithNamespace("http"))
 	return &AdminClient{
 		logger:     logger,
-		c: &http.Client{
-			Transport: transport,
-		},
+		c:          httpClient,
 		addr:       addr,
 		adminToken: adminToken,
-		transport:  transport,
 	}
 }
 
 // Close cleans up all resources held by the lient.
 func (c *AdminClient) Close() error {
-	c.transport.CloseIdleConnections()
-	return nil
+	return c.c.Close()
 }
 
 // do performs an HTTP request with the given method (GET, POST) and path, and
@@ -88,7 +84,6 @@ func (c *AdminClient) Close() error {
 func (c *AdminClient) do(
 	ctx context.Context, rcv any, method, path string, body any,
 ) error {
-
 	var bodyR io.Reader
 
 	if body != nil {
@@ -109,31 +104,10 @@ func (c *AdminClient) do(
 
 	req.Header.Set("Authorization", "Bearer "+c.adminToken)
 
-	if c.logger.MaxLevel() >= mlog.LevelDebug.Int() {
-
-		reqB, err := httputil.DumpRequestOut(req, true)
-		if err != nil {
-			c.logger.Error(ctx, "failed to dump http request", err)
-		} else {
-			c.logger.Debug(ctx, "------ request ------\n"+string(reqB)+"\n")
-		}
-	}
-
 	res, err := c.c.Do(req)
 	if err != nil {
 		return fmt.Errorf("performing http request: %w", err)
 	}
-
-	if c.logger.MaxLevel() >= mlog.LevelDebug.Int() {
-
-		resB, err := httputil.DumpResponse(res, true)
-		if err != nil {
-			c.logger.Error(ctx, "failed to dump http response", err)
-		} else {
-			c.logger.Debug(ctx, "------ response ------\n"+string(resB)+"\n")
-		}
-	}
-
 	defer res.Body.Close()
 
 	if res.StatusCode >= 300 {

go/toolkit/http.go

@@ -0,0 +1,99 @@
+package toolkit
+
+import (
+	"net/http"
+	"net/http/httputil"
+	"net/url"
+
+	"dev.mediocregopher.com/mediocre-go-lib.git/mlog"
+	"github.com/tv42/httpunix"
+)
+
+// HTTPClient is an interface around the default http.Client type.
+type HTTPClient interface {
+	Do(*http.Request) (*http.Response, error)
+
+	// Close cleans up any resources being held by the HTTPClient.
+	Close() error
+}
+
+type httpClient struct {
+	logger *mlog.Logger
+	*http.Client
+}
+
+// NewHTTPClient returns a new HTTPClient using a clone of
+// [http.DefaultTransport].
+func NewHTTPClient(logger *mlog.Logger) HTTPClient {
+	return &httpClient{
+		logger,
+		&http.Client{
+			Transport: http.DefaultTransport.(*http.Transport).Clone(),
+		},
+	}
+}
+
+// NewUnixHTTPClient returns an HTTPClient which will use the given
+// unixSocketPath to serve requests. The base URL (scheme and host) which should
+// be used for requests is returned as well.
+func NewUnixHTTPClient(
+	logger *mlog.Logger, unixSocketPath string,
+) (
+	HTTPClient, *url.URL,
+) {
+	const host = "uds"
+
+	u := &url.URL{
+		Scheme: httpunix.Scheme,
+		Host:   host,
+	}
+
+	transport := new(httpunix.Transport)
+	transport.RegisterLocation(host, unixSocketPath)
+
+	return &httpClient{logger, &http.Client{Transport: transport}}, u
+}
+
+func (c *httpClient) Do(req *http.Request) (*http.Response, error) {
+	if c.logger.MaxLevel() < mlog.LevelDebug.Int() {
+		return c.Client.Do(req)
+	}
+
+	var (
+		ctx        = req.Context()
+		origScheme = req.URL.Scheme
+	)
+
+	// httputil.DumpRequestOut doesn't like the httpunix.Scheme, so we
+	// temporarily switch it.
+	if req.URL.Scheme == httpunix.Scheme {
+		req.URL.Scheme = "http"
+	}
+
+	reqB, err := httputil.DumpRequestOut(req, true)
+	if err != nil {
+		c.logger.Error(ctx, "failed to dump http request", err)
+	} else {
+		c.logger.Debug(ctx, "------ request ------\n"+string(reqB)+"\n")
+	}
+
+	req.URL.Scheme = origScheme
+
+	res, err := c.Client.Do(req)
+
+	if res != nil {
+		resB, err := httputil.DumpResponse(res, true)
+		if err != nil {
+			c.logger.Error(ctx, "failed to dump http response", err)
+		} else {
+			c.logger.Debug(ctx, "------ response ------\n"+string(resB)+"\n")
+		}
+	}
+
+	return res, err
+}
+
+func (c *httpClient) Close() error {
+	c.CloseIdleConnections()
+	return nil
+}