feat: add debug-mode around all network requests (#1239)

* feat: add debug-mode around all network requests

* fix: tests

---------

Co-authored-by: Kevin P. Fleming <kpfleming@users.noreply.github.com>

pkg/api/undocumented/undocumented.go

@@ -3,16 +3,15 @@
 package undocumented
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"net/http"
-	"net/http/httputil"
 	"net/url"
 	"strings"
 	"time"
 
 	"github.com/fastly/cli/pkg/api"
+	"github.com/fastly/cli/pkg/debug"
 	fsterr "github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/useragent"
 )
@@ -81,17 +80,11 @@ func Call(opts CallOptions) (data []byte, err error) {
 	}
 
 	if opts.Debug {
-		rc := req.Clone(context.Background())
-		rc.Header.Set("Fastly-Key", "REDACTED")
-		dump, _ := httputil.DumpRequest(rc, true)
-		fmt.Printf("undocumented.Call request dump:\n\n%#v\n\n", string(dump))
+		debug.DumpHTTPRequest(req)
 	}
-
 	res, err := opts.HTTPClient.Do(req)
-
-	if opts.Debug && res != nil {
-		dump, _ := httputil.DumpResponse(res, true)
-		fmt.Printf("undocumented.Call response dump:\n\n%#v\n\n", string(dump))
+	if opts.Debug {
+		debug.DumpHTTPResponse(res)
 	}
 
 	if err != nil {

pkg/app/run.go

@@ -115,21 +115,33 @@ var Init = func(args []string, stdin io.Reader) (*global.Data, error) {
 		return client, err
 	}
 
+	// Identify debug-mode flag early (before Kingpin parser has executed) so we
+	// can inform the github versioners that we're in debug mode.
+	var debugMode bool
+	for _, seg := range args {
+		if seg == "--debug-mode" {
+			debugMode = true
+		}
+	}
+
 	versioners := global.Versioners{
 		CLI: github.New(github.Opts{
+			DebugMode:  debugMode,
 			HTTPClient: httpClient,
 			Org:        "fastly",
 			Repo:       "cli",
 			Binary:     "fastly",
 		}),
 		Viceroy: github.New(github.Opts{
+			DebugMode:  debugMode,
 			HTTPClient: httpClient,
 			Org:        "fastly",
 			Repo:       "viceroy",
 			Binary:     "viceroy",
 			Version:    md.File.LocalServer.ViceroyVersion,
 		}),
 		WasmTools: github.New(github.Opts{
+			DebugMode:  debugMode,
 			HTTPClient: httpClient,
 			Org:        "bytecodealliance",
 			Repo:       "wasm-tools",

pkg/auth/auth.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/http/httputil"
 	"net/url"
 	"strconv"
 	"strings"
@@ -19,6 +18,7 @@ import (
 	"github.com/fastly/cli/pkg/api"
 	"github.com/fastly/cli/pkg/api/undocumented"
 	"github.com/fastly/cli/pkg/config"
+	"github.com/fastly/cli/pkg/debug"
 	fsterr "github.com/fastly/cli/pkg/errors"
 )
 
@@ -144,19 +144,14 @@ func (s Server) GetJWT(authorizationCode string) (JWT, error) {
 	}
 	req.Header.Add("content-type", "application/x-www-form-urlencoded")
 
-	debug, _ := strconv.ParseBool(s.DebugMode)
-	if debug {
-		rc := req.Clone(context.Background())
-		rc.Header.Set("Fastly-Key", "REDACTED")
-		dump, _ := httputil.DumpRequest(rc, true)
-		fmt.Printf("GetJWT request dump:\n\n%#v\n\n", string(dump))
-	}
+	debugMode, _ := strconv.ParseBool(s.DebugMode)
 
+	if debugMode {
+		debug.DumpHTTPRequest(req)
+	}
 	res, err := http.DefaultClient.Do(req)
-
-	if debug && res != nil {
-		dump, _ := httputil.DumpResponse(res, true)
-		fmt.Printf("GetJWT response dump:\n\n%#v\n\n", string(dump))
+	if debugMode {
+		debug.DumpHTTPResponse(res)
 	}
 
 	if err != nil {
@@ -363,19 +358,13 @@ func (s *Server) RefreshAccessToken(refreshToken string) (JWT, error) {
 	}
 	req.Header.Add("content-type", "application/x-www-form-urlencoded")
 
-	debug, _ := strconv.ParseBool(s.DebugMode)
-	if debug {
-		rc := req.Clone(context.Background())
-		rc.Header.Set("Fastly-Key", "REDACTED")
-		dump, _ := httputil.DumpRequest(rc, true)
-		fmt.Printf("RefreshAccessToken request dump:\n\n%#v\n\n", string(dump))
+	debugMode, _ := strconv.ParseBool(s.DebugMode)
+	if debugMode {
+		debug.DumpHTTPRequest(req)
 	}
-
 	res, err := http.DefaultClient.Do(req)
-
-	if debug && res != nil {
-		dump, _ := httputil.DumpResponse(res, true)
-		fmt.Printf("RefreshAccessToken response dump:\n\n%#v\n\n", string(dump))
+	if debugMode {
+		debug.DumpHTTPResponse(res)
 	}
 
 	if err != nil {

pkg/commands/compute/deploy.go

@@ -22,6 +22,7 @@ import (
 	"github.com/fastly/cli/pkg/api/undocumented"
 	"github.com/fastly/cli/pkg/argparser"
 	"github.com/fastly/cli/pkg/commands/compute/setup"
+	"github.com/fastly/cli/pkg/debug"
 	fsterr "github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/internal/beacon"
@@ -1109,7 +1110,7 @@ func checkingServiceAvailability(
 			// We overwrite the `status` variable in the parent scope (defined in the
 			// return arguments list) so it can be used as part of both the timeout
 			// and success scenarios.
-			ok, status, err = pingServiceURL(serviceURL, c.Globals.HTTPClient, c.StatusCheckCode)
+			ok, status, err = pingServiceURL(serviceURL, c.Globals.HTTPClient, c.StatusCheckCode, c.Globals.Flags.Debug)
 			if err != nil {
 				err := fmt.Errorf("failed to ping service URL: %w", err)
 				returnedStatus := fmt.Sprintf(" (status: %d)", status)
@@ -1144,7 +1145,7 @@ func generateTimeout(d time.Duration) string {
 // pingServiceURL indicates if the service returned a non-5xx response (or
 // whatever the user defined with --status-check-code), which should help
 // signify if the service is generally available.
-func pingServiceURL(serviceURL string, httpClient api.HTTPClient, expectedStatusCode int) (ok bool, status int, err error) {
+func pingServiceURL(serviceURL string, httpClient api.HTTPClient, expectedStatusCode int, debugMode bool) (ok bool, status int, err error) {
 	req, err := http.NewRequest("GET", serviceURL, nil)
 	if err != nil {
 		return false, 0, err
@@ -1154,7 +1155,13 @@ func pingServiceURL(serviceURL string, httpClient api.HTTPClient, expectedStatus
 	// G107 (CWE-88): Potential HTTP request made with variable url
 	// Disabling as we trust the source of the variable.
 	// #nosec
+	if debugMode {
+		debug.DumpHTTPRequest(req)
+	}
 	resp, err := httpClient.Do(req)
+	if debugMode {
+		debug.DumpHTTPResponse(resp)
+	}
 	if err != nil {
 		return false, 0, err
 	}

pkg/commands/compute/init.go

@@ -16,10 +16,12 @@ import (
 	"strings"
 	"time"
 
+	"github.com/fastly/go-fastly/v9/fastly"
 	cp "github.com/otiai10/copy"
 
 	"github.com/fastly/cli/pkg/argparser"
 	"github.com/fastly/cli/pkg/config"
+	"github.com/fastly/cli/pkg/debug"
 	fsterr "github.com/fastly/cli/pkg/errors"
 	fstexec "github.com/fastly/cli/pkg/exec"
 	"github.com/fastly/cli/pkg/file"
@@ -29,7 +31,6 @@ import (
 	"github.com/fastly/cli/pkg/manifest"
 	"github.com/fastly/cli/pkg/profile"
 	"github.com/fastly/cli/pkg/text"
-	"github.com/fastly/go-fastly/v9/fastly"
 )
 
 var (
@@ -838,6 +839,7 @@ func (c *InitCommand) FetchPackageTemplate(branch, tag string, archives []file.A
 	if err != nil {
 		return err
 	}
+	text.Break(out)
 	msg := "Fetching package template"
 	spinner.Message(msg + "...")
 
@@ -911,9 +913,16 @@ func (c *InitCommand) FetchPackageTemplate(branch, tag string, archives []file.A
 		}
 	}
 
+	if c.Globals.Flags.Debug {
+		debug.DumpHTTPRequest(req)
+	}
 	res, err := c.Globals.HTTPClient.Do(req)
+	if c.Globals.Flags.Debug {
+		debug.DumpHTTPResponse(res)
+	}
+
 	if err != nil {
-		err = fmt.Errorf("failed to get package: %w", err)
+		err = fmt.Errorf("failed to get package '%s': %w", req.URL.String(), err)
 		c.Globals.ErrLog.Add(err)
 		spinner.StopFailMessage(msg)
 		spinErr := spinner.StopFail()
@@ -925,7 +934,7 @@ func (c *InitCommand) FetchPackageTemplate(branch, tag string, archives []file.A
 	defer res.Body.Close() // #nosec G307
 
 	if res.StatusCode != http.StatusOK {
-		err := fmt.Errorf("failed to get package: %s", res.Status)
+		err := fmt.Errorf("failed to get package '%s': %s", req.URL.String(), res.Status)
 		c.Globals.ErrLog.Add(err)
 		spinner.StopFailMessage(msg)
 		spinErr := spinner.StopFail()

pkg/commands/compute/init_test.go

@@ -10,14 +10,15 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/fastly/go-fastly/v9/fastly"
+
 	"github.com/fastly/cli/pkg/app"
 	"github.com/fastly/cli/pkg/config"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/manifest"
 	"github.com/fastly/cli/pkg/mock"
 	"github.com/fastly/cli/pkg/testutil"
 	"github.com/fastly/cli/pkg/threadsafe"
-	"github.com/fastly/go-fastly/v9/fastly"
 )
 
 func TestInit(t *testing.T) {
@@ -60,7 +61,7 @@ func TestInit(t *testing.T) {
 		{
 			name:      "broken endpoint",
 			args:      args("compute init --from https://example.com/i-dont-exist"),
-			wantError: "failed to get package: Not Found",
+			wantError: "failed to get package 'https://example.com/i-dont-exist': Not Found",
 			httpClientRes: []*http.Response{
 				{
 					Body:       io.NopCloser(strings.NewReader("")),

pkg/commands/logtail/root.go

@@ -21,6 +21,7 @@ import (
 	"github.com/tomnomnom/linkheader"
 
 	"github.com/fastly/cli/pkg/argparser"
+	"github.com/fastly/cli/pkg/debug"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
 )
@@ -462,7 +463,13 @@ func (c *RootCommand) doReq(req *http.Request) (*http.Response, error) {
 		}
 	}()
 
+	if c.Globals.Flags.Debug {
+		debug.DumpHTTPRequest(req)
+	}
 	resp, err := c.hClient.Do(req)
+	if c.Globals.Flags.Debug {
+		debug.DumpHTTPResponse(resp)
+	}
 	return resp, err
 }
 

pkg/debug/debug.go

@@ -1,8 +1,11 @@
 package debug
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
+	"net/http/httputil"
 )
 
 // PrintStruct pretty prints the given struct.
@@ -13,3 +16,21 @@ func PrintStruct(v any) error {
 	}
 	return err
 }
+
+// DumpHTTPRequest dumps the HTTP network request if --debug-mode is set.
+func DumpHTTPRequest(r *http.Request) {
+	req := r.Clone(context.Background())
+	if req.Header.Get("Fastly-Key") != "" {
+		req.Header.Set("Fastly-Key", "REDACTED")
+	}
+	dump, _ := httputil.DumpRequest(r, true)
+	fmt.Printf("\n\nhttp.Request (dump): %q\n\n", dump)
+}
+
+// DumpHTTPResponse dumps the HTTP network response if --debug-mode is set.
+func DumpHTTPResponse(r *http.Response) {
+	if r != nil {
+		dump, _ := httputil.DumpResponse(r, true)
+		fmt.Printf("\n\nhttp.Response (dump): %q\n\n", dump)
+	}
+}

pkg/github/github.go

@@ -15,6 +15,7 @@ import (
 	"github.com/mholt/archiver"
 
 	"github.com/fastly/cli/pkg/api"
+	"github.com/fastly/cli/pkg/debug"
 	fstruntime "github.com/fastly/cli/pkg/runtime"
 )
 
@@ -47,6 +48,7 @@ func New(opts Opts) *Asset {
 
 	return &Asset{
 		binary:           binary,
+		debug:            opts.DebugMode,
 		external:         opts.External,
 		httpClient:       opts.HTTPClient,
 		nested:           opts.Nested,
@@ -60,6 +62,8 @@ func New(opts Opts) *Asset {
 type Opts struct {
 	// Binary is the name of the executable binary.
 	Binary string
+	// DebugMode indicates the user has set debug-mode.
+	DebugMode bool
 	// External indicates the repository is a non-Fastly repo.
 	// This means we need a custom metadata fetcher (i.e. dont use metadataURL).
 	External bool
@@ -83,6 +87,8 @@ type Opts struct {
 type Asset struct {
 	// binary is the name of the executable binary.
 	binary string
+	// debug indicates if the user is running in debug-mode.
+	debug bool
 	// external indicates the repository is a non-Fastly repo.
 	external bool
 	// httpClient is able to make HTTP requests.
@@ -145,7 +151,13 @@ func (g *Asset) Download(endpoint string) (bin string, err error) {
 	if g.httpClient == nil {
 		g.httpClient = http.DefaultClient
 	}
+	if g.debug {
+		debug.DumpHTTPRequest(req)
+	}
 	res, err := g.httpClient.Do(req)
+	if g.debug {
+		debug.DumpHTTPResponse(res)
+	}
 	if err != nil {
 		return "", fmt.Errorf("failed to request GitHub release asset: %w", err)
 	}
@@ -235,7 +247,13 @@ func (g *Asset) metadata() (m DevHubMetadata, err error) {
 	if g.httpClient == nil {
 		g.httpClient = http.DefaultClient
 	}
+	if g.debug {
+		debug.DumpHTTPRequest(req)
+	}
 	res, err := g.httpClient.Do(req)
+	if g.debug {
+		debug.DumpHTTPResponse(res)
+	}
 	if err != nil {
 		return m, fmt.Errorf("failed to request GitHub metadata: %w", err)
 	}