From e7c88d0fb13cef1e3336c2b395949dd91f0d3490 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Fri, 21 Feb 2020 02:46:18 +0100 Subject: [PATCH 1/3] impr(verbose): using package httpretty to log requests on DEBUG. --- api/client.go | 35 +++++++++++++---------------------- go.mod | 1 + go.sum | 2 ++ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/api/client.go b/api/client.go index c056680c9..837fa81ca 100644 --- a/api/client.go +++ b/api/client.go @@ -9,6 +9,8 @@ import ( "net/http" "regexp" "strings" + + "github.com/henvic/httpretty" ) // ClientOption represents an argument to NewClient @@ -37,29 +39,18 @@ func AddHeader(name, value string) ClientOption { // VerboseLog enables request/response logging within a RoundTripper func VerboseLog(out io.Writer, logBodies bool) ClientOption { - return func(tr http.RoundTripper) http.RoundTripper { - return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - fmt.Fprintf(out, "> %s %s\n", req.Method, req.URL.RequestURI()) - if logBodies && req.Body != nil && inspectableMIMEType(req.Header.Get("Content-type")) { - newBody := &bytes.Buffer{} - io.Copy(out, io.TeeReader(req.Body, newBody)) - fmt.Fprintln(out) - req.Body = ioutil.NopCloser(newBody) - } - res, err := tr.RoundTrip(req) - if err == nil { - fmt.Fprintf(out, "< HTTP %s\n", res.Status) - if logBodies && res.Body != nil && inspectableMIMEType(res.Header.Get("Content-type")) { - newBody := &bytes.Buffer{} - // TODO: pretty-print response JSON - io.Copy(out, io.TeeReader(res.Body, newBody)) - fmt.Fprintln(out) - res.Body = ioutil.NopCloser(newBody) - } - } - return res, err - }} + logger := &httpretty.Logger{ + RequestHeader: true, + RequestBody: logBodies, + ResponseHeader: true, + ResponseBody: logBodies, + Formatters: []httpretty.Formatter{&httpretty.JSONFormatter{}}, } + logger.SetOutput(out) + logger.SetBodyFilter(func(h http.Header) (skip bool, err error) { + return !inspectableMIMEType(h.Get("Content-Type")), nil + }) + return logger.RoundTripper } // ReplaceTripper substitutes the underlying RoundTripper with a custom one diff --git a/go.mod b/go.mod index 40d22c24c..458a7268e 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/dlclark/regexp2 v1.2.0 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/hashicorp/go-version v1.2.0 + github.com/henvic/httpretty v0.0.3 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.4 github.com/mattn/go-isatty v0.0.12 diff --git a/go.sum b/go.sum index 9ae8a5ec4..518f745aa 100644 --- a/go.sum +++ b/go.sum @@ -80,6 +80,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t github.com/hashicorp/go-version v1.2.0 h1:3vNe/fWF5CBgRIguda1meWhsZHy3m8gCJ5wx+dIzX/E= github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= +github.com/henvic/httpretty v0.0.3 h1:oHTreVv2lcdRYUNm4h3cgbrGN0dTieO9H8UnxEZNlvw= +github.com/henvic/httpretty v0.0.3/go.mod h1:X38wLjWXHkXT7r2+uK8LjCMne9rsuNaBLJ+5cU2/Pmo= github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174 h1:WlZsjVhE8Af9IcZDGgJGQpNflI3+MJSBhsgT5PCtzBQ= github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= From 9c2efd6c1c8c72333f638e149a7611c9ba5ef983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 21 Feb 2020 12:46:21 +0100 Subject: [PATCH 2/3] Extract reusable `IsTerminal()` --- cmd/gh/main.go | 4 +--- utils/color.go | 8 ++++++-- utils/table_printer.go | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 00ee1e00c..a9e0837f3 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -13,7 +13,6 @@ import ( "github.com/cli/cli/context" "github.com/cli/cli/update" "github.com/cli/cli/utils" - "github.com/mattn/go-isatty" "github.com/mgutz/ansi" "github.com/spf13/cobra" ) @@ -71,8 +70,7 @@ func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) { } func shouldCheckForUpdate() bool { - errFd := os.Stderr.Fd() - return updaterEnabled != "" && (isatty.IsTerminal(errFd) || isatty.IsCygwinTerminal(errFd)) + return updaterEnabled != "" && utils.IsTerminal(os.Stderr) } func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { diff --git a/utils/color.go b/utils/color.go index ed22bc3e9..0a002588f 100644 --- a/utils/color.go +++ b/utils/color.go @@ -14,13 +14,17 @@ var checkedTerminal = false func isStdoutTerminal() bool { if !checkedTerminal { - fd := os.Stdout.Fd() - _isStdoutTerminal = isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) + _isStdoutTerminal = IsTerminal(os.Stdout) checkedTerminal = true } return _isStdoutTerminal } +// IsTerminal reports whether the file descriptor is connected to a terminal +func IsTerminal(f *os.File) bool { + return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) +} + // NewColorable returns an output stream that handles ANSI color sequences on Windows func NewColorable(f *os.File) io.Writer { return colorable.NewColorable(f) diff --git a/utils/table_printer.go b/utils/table_printer.go index e61833fb3..b6f651196 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -22,6 +22,7 @@ type TablePrinter interface { func NewTablePrinter(w io.Writer) TablePrinter { if outFile, isFile := w.(*os.File); isFile { + // TODO: use utils.IsTerminal() isCygwin := isatty.IsCygwinTerminal(outFile.Fd()) if isatty.IsTerminal(outFile.Fd()) || isCygwin { ttyWidth := 80 From 9c00ac0224b51166ced8b80211cfcee7888c83bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 21 Feb 2020 12:57:00 +0100 Subject: [PATCH 3/3] Tweak verbose HTTP logging - log headers only in DEBUG=api mode - enable color output on stderr - hide little-useful TLS debbuging info - ensure all request headers are logged --- api/client.go | 13 ++++++++----- command/root.go | 27 +++++++++++++++++---------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/api/client.go b/api/client.go index 837fa81ca..a0ed6a859 100644 --- a/api/client.go +++ b/api/client.go @@ -38,12 +38,15 @@ func AddHeader(name, value string) ClientOption { } // VerboseLog enables request/response logging within a RoundTripper -func VerboseLog(out io.Writer, logBodies bool) ClientOption { +func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { logger := &httpretty.Logger{ - RequestHeader: true, - RequestBody: logBodies, - ResponseHeader: true, - ResponseBody: logBodies, + Time: true, + TLS: false, + Colors: colorize, + RequestHeader: logTraffic, + RequestBody: logTraffic, + ResponseHeader: logTraffic, + ResponseBody: logTraffic, Formatters: []httpretty.Formatter{&httpretty.JSONFormatter{}}, } logger.SetOutput(out) diff --git a/command/root.go b/command/root.go index 4f9d5ba09..0257b6a48 100644 --- a/command/root.go +++ b/command/root.go @@ -97,15 +97,14 @@ var initContext = func() context.Context { // BasicClient returns an API client that borrows from but does not depend on // user configuration func BasicClient() (*api.Client, error) { - opts := []api.ClientOption{ - api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), + opts := []api.ClientOption{} + if verbose := os.Getenv("DEBUG"); verbose != "" { + opts = append(opts, apiVerboseLog()) } + opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version))) if c, err := context.ParseDefaultConfig(); err == nil { opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", c.Token))) } - if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, api.VerboseLog(os.Stderr, false)) - } return api.NewClient(opts...), nil } @@ -123,20 +122,28 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { if err != nil { return nil, err } - opts := []api.ClientOption{ + opts := []api.ClientOption{} + if verbose := os.Getenv("DEBUG"); verbose != "" { + opts = append(opts, apiVerboseLog()) + } + opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks // shadow-cat-preview: Draft pull requests api.AddHeader("Accept", "application/vnd.github.antiope-preview+json, application/vnd.github.shadow-cat-preview"), api.AddHeader("GraphQL-Features", "pe_mobile"), - } - if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, api.VerboseLog(os.Stderr, strings.Contains(verbose, "api"))) - } + ) + return api.NewClient(opts...), nil } +func apiVerboseLog() api.ClientOption { + logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") + colorize := utils.IsTerminal(os.Stderr) + return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) +} + func colorableOut(cmd *cobra.Command) io.Writer { out := cmd.OutOrStdout() if outFile, isFile := out.(*os.File); isFile {