From 54ba26eff84a33640570a1d360c9538cdff763a8 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Fri, 27 Dec 2024 20:49:47 -0600 Subject: [PATCH 01/12] Implement debugging functionality when reading a response's body --- pkg/uhttp/body_print.go | 41 +++++++++++++++++++++++++++++++++++++++++ pkg/uhttp/wrapper.go | 13 +++++++++---- 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 pkg/uhttp/body_print.go diff --git a/pkg/uhttp/body_print.go b/pkg/uhttp/body_print.go new file mode 100644 index 00000000..bf89c6c3 --- /dev/null +++ b/pkg/uhttp/body_print.go @@ -0,0 +1,41 @@ +package uhttp + +// Implements a debugging facility for request responses. This changes +// the behavior of `BaseHttpClient` with an unexported flag. +// +// If you always wanted to see the actual body of your response, now +// you can 👁️👄👁️undress it🫦 to uncover all its... data! + +import ( + "io" + "log" +) + +type printReader struct { + reader io.Reader +} + +func (pr *printReader) Read(p []byte) (int, error) { + n, err := pr.reader.Read(p) + if n > 0 { + log.Print(string(p[:n])) + } + + return n, err +} + +func wrapBodyToUndress(body io.Reader) io.Reader { + return &printReader{reader: body} +} + +type undressOption struct { + undress bool +} + +func (o undressOption) Apply(c *BaseHttpClient) { + c.debugPrintBody = o.undress +} + +func WithUndressBody(undress bool) WrapperOption { + return undressOption{undress: undress} +} diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index 47f4c074..a3722693 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -91,9 +91,10 @@ type ( NewRequest(ctx context.Context, method string, url *url.URL, options ...RequestOption) (*http.Request, error) } BaseHttpClient struct { - HttpClient *http.Client - rateLimiter uRateLimit.Limiter - baseHttpCache icache + HttpClient *http.Client + rateLimiter uRateLimit.Limiter + baseHttpCache icache + debugPrintBody bool } DoOption func(resp *WrapperResponse) error @@ -341,7 +342,11 @@ func (c *BaseHttpClient) Do(req *http.Request, options ...DoOption) (*http.Respo } // Replace resp.Body with a no-op closer so nobody has to worry about closing the reader. - resp.Body = io.NopCloser(bytes.NewBuffer(body)) + if c.debugPrintBody { + resp.Body = io.NopCloser(wrapBodyToUndress(bytes.NewBuffer(body))) + } else { + resp.Body = io.NopCloser(bytes.NewBuffer(body)) + } wresp := WrapperResponse{ Header: resp.Header, From ba3287f1ca73e94ea875397e26e4395edf9b2e59 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:20:46 -0600 Subject: [PATCH 02/12] Remove the silliness :) December 28 has passed already --- pkg/uhttp/body_print.go | 17 +++++++---------- pkg/uhttp/wrapper.go | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/uhttp/body_print.go b/pkg/uhttp/body_print.go index bf89c6c3..3491e17e 100644 --- a/pkg/uhttp/body_print.go +++ b/pkg/uhttp/body_print.go @@ -2,9 +2,6 @@ package uhttp // Implements a debugging facility for request responses. This changes // the behavior of `BaseHttpClient` with an unexported flag. -// -// If you always wanted to see the actual body of your response, now -// you can 👁️👄👁️undress it🫦 to uncover all its... data! import ( "io" @@ -24,18 +21,18 @@ func (pr *printReader) Read(p []byte) (int, error) { return n, err } -func wrapBodyToUndress(body io.Reader) io.Reader { +func wrapPrintBody(body io.Reader) io.Reader { return &printReader{reader: body} } -type undressOption struct { - undress bool +type printBodyOption struct { + debugPrintBody bool } -func (o undressOption) Apply(c *BaseHttpClient) { - c.debugPrintBody = o.undress +func (o printBodyOption) Apply(c *BaseHttpClient) { + c.debugPrintBody = o.debugPrintBody } -func WithUndressBody(undress bool) WrapperOption { - return undressOption{undress: undress} +func WithPrintBody(shouldPrint bool) WrapperOption { + return printBodyOption{debugPrintBody: shouldPrint} } diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index a3722693..26672801 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -343,7 +343,7 @@ func (c *BaseHttpClient) Do(req *http.Request, options ...DoOption) (*http.Respo // Replace resp.Body with a no-op closer so nobody has to worry about closing the reader. if c.debugPrintBody { - resp.Body = io.NopCloser(wrapBodyToUndress(bytes.NewBuffer(body))) + resp.Body = io.NopCloser(wrapPrintBody(bytes.NewBuffer(body))) } else { resp.Body = io.NopCloser(bytes.NewBuffer(body)) } From 48f2d9c9f3a0da0898e56874ffd5205e4c920397 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:30:30 -0600 Subject: [PATCH 03/12] Don't use log.Print for writing to Stdout --- pkg/uhttp/body_print.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/uhttp/body_print.go b/pkg/uhttp/body_print.go index 3491e17e..6f8fa7a4 100644 --- a/pkg/uhttp/body_print.go +++ b/pkg/uhttp/body_print.go @@ -4,8 +4,10 @@ package uhttp // the behavior of `BaseHttpClient` with an unexported flag. import ( + "errors" + "fmt" "io" - "log" + "os" ) type printReader struct { @@ -15,7 +17,10 @@ type printReader struct { func (pr *printReader) Read(p []byte) (int, error) { n, err := pr.reader.Read(p) if n > 0 { - log.Print(string(p[:n])) + _, merr := fmt.Fprint(os.Stdout, string(p[:n])) + if merr != nil { + return -1, errors.Join(err, merr) + } } return n, err From c523fdbe680e97b47bc28ec19d2fa1c8a96693b6 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:40:37 -0600 Subject: [PATCH 04/12] Add important note --- pkg/uhttp/body_print.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/uhttp/body_print.go b/pkg/uhttp/body_print.go index 6f8fa7a4..5c1f1618 100644 --- a/pkg/uhttp/body_print.go +++ b/pkg/uhttp/body_print.go @@ -2,6 +2,15 @@ package uhttp // Implements a debugging facility for request responses. This changes // the behavior of `BaseHttpClient` with an unexported flag. +// +// IMPORTANT: This feature is intended for development and debugging purposes only. +// Do not enable in production as it may expose sensitive information in logs. +// +// Usage: +// client := uhttp.NewBaseHttpClient( +// httpClient, +// uhttp.WithPrintBody(true), // Enable response body printing +// ) import ( "errors" From dc4aa385d0f505f7dd25ff70b66b312c96418f1c Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:42:30 -0600 Subject: [PATCH 05/12] Update flake.lock --- flake.lock | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/flake.lock b/flake.lock index 9dfbe6f5..2c6b402b 100644 --- a/flake.lock +++ b/flake.lock @@ -18,12 +18,12 @@ }, "flake-schemas": { "locked": { - "lastModified": 1697467827, - "narHash": "sha256-j8SR19V1SRysyJwpOBF4TLuAvAjF5t+gMiboN4gYQDU=", - "rev": "764932025c817d4e500a8d2a4d8c565563923d29", - "revCount": 29, + "lastModified": 1721999734, + "narHash": "sha256-G5CxYeJVm4lcEtaO87LKzOsVnWeTcHGKbKxNamNWgOw=", + "rev": "0a5c42297d870156d9c57d8f99e476b738dcd982", + "revCount": 75, "type": "tarball", - "url": "https://api.flakehub.com/f/pinned/DeterminateSystems/flake-schemas/0.1.2/018b3da8-4cc3-7fbb-8ff7-1588413c53e2/source.tar.gz" + "url": "https://api.flakehub.com/f/pinned/DeterminateSystems/flake-schemas/0.1.5/0190ef2f-61e0-794b-ba14-e82f225e55e6/source.tar.gz?rev=0a5c42297d870156d9c57d8f99e476b738dcd982&revCount=75" }, "original": { "type": "tarball", @@ -53,12 +53,12 @@ }, "nixpkgs": { "locked": { - "lastModified": 1717952948, - "narHash": "sha256-mJi4/gjiwQlSaxjA6AusXBN/6rQRaPCycR7bd8fydnQ=", - "rev": "2819fffa7fa42156680f0d282c60d81e8fb185b7", - "revCount": 631440, + "lastModified": 1735669367, + "narHash": "sha256-tfYRbFhMOnYaM4ippqqid3BaLOXoFNdImrfBfCp4zn0=", + "rev": "edf04b75c13c2ac0e54df5ec5c543e300f76f1c9", + "revCount": 712148, "type": "tarball", - "url": "https://api.flakehub.com/f/pinned/NixOS/nixpkgs/0.2405.631440%2Brev-2819fffa7fa42156680f0d282c60d81e8fb185b7/0190034c-678d-7039-b45c-fa38168f2500/source.tar.gz" + "url": "https://api.flakehub.com/f/pinned/NixOS/nixpkgs/0.2411.712148%2Brev-edf04b75c13c2ac0e54df5ec5c543e300f76f1c9/0194235a-7100-7180-896b-1aa5e516625b/source.tar.gz?rev=edf04b75c13c2ac0e54df5ec5c543e300f76f1c9&revCount=712148" }, "original": { "type": "tarball", @@ -67,27 +67,27 @@ }, "nixpkgs-stable": { "locked": { - "lastModified": 1718811006, - "narHash": "sha256-0Y8IrGhRmBmT7HHXlxxepg2t8j1X90++qRN3lukGaIk=", + "lastModified": 1730741070, + "narHash": "sha256-edm8WG19kWozJ/GqyYx2VjW99EdhjKwbY3ZwdlPAAlo=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "03d771e513ce90147b65fe922d87d3a0356fc125", + "rev": "d063c1dd113c91ab27959ba540c0d9753409edf3", "type": "github" }, "original": { "owner": "NixOS", - "ref": "nixos-23.11", + "ref": "nixos-24.05", "repo": "nixpkgs", "type": "github" } }, "nixpkgs_2": { "locked": { - "lastModified": 1719082008, - "narHash": "sha256-jHJSUH619zBQ6WdC21fFAlDxHErKVDJ5fpN0Hgx4sjs=", + "lastModified": 1730768919, + "narHash": "sha256-8AKquNnnSaJRXZxc5YmF/WfmxiHX6MMZZasRP6RRQkE=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "9693852a2070b398ee123a329e68f0dab5526681", + "rev": "a04d33c0c3f1a59a2c1cb0c6e34cd24500e5a1dc", "type": "github" }, "original": { @@ -105,11 +105,11 @@ "nixpkgs-stable": "nixpkgs-stable" }, "locked": { - "lastModified": 1719259945, - "narHash": "sha256-F1h+XIsGKT9TkGO3omxDLEb/9jOOsI6NnzsXFsZhry4=", + "lastModified": 1734797603, + "narHash": "sha256-ulZN7ps8nBV31SE+dwkDvKIzvN6hroRY8sYOT0w+E28=", "owner": "cachix", "repo": "pre-commit-hooks.nix", - "rev": "0ff4381bbb8f7a52ca4a851660fc7a437a4c6e07", + "rev": "f0f0dc4920a903c3e08f5bdb9246bb572fcae498", "type": "github" }, "original": { From 48989290ae7179e2fc691c281c857352032fdcb4 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:44:09 -0600 Subject: [PATCH 06/12] Add fmt.Fprint to the list of unhandled errors --- .golangci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yml b/.golangci.yml index feb6fc62..d541999f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,6 +36,7 @@ linters-settings: arguments: - fmt.Printf - fmt.Println + - fmt.Fprint - fmt.Fprintf - fmt.Fprintln - os.Stderr.Sync From d3a1b7f2c308001288f555e956c00ff006478496 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 08:48:38 -0600 Subject: [PATCH 07/12] End comment with a period --- pkg/sync/syncer.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index d8c0ea67..6900fc35 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -33,9 +33,7 @@ const maxDepth = 8 var dontFixCycles, _ = strconv.ParseBool(os.Getenv("BATON_DONT_FIX_CYCLES")) -var ( - ErrSyncNotComplete = fmt.Errorf("sync exited without finishing") -) +var ErrSyncNotComplete = fmt.Errorf("sync exited without finishing") type Syncer interface { Sync(context.Context) error @@ -972,7 +970,7 @@ func (s *syncer) SyncAssets(ctx context.Context) error { } // SyncGrantExpansion -// TODO(morgabra) Docs. +// TODO(morgabra) Docs func (s *syncer) SyncGrantExpansion(ctx context.Context) error { l := ctxzap.Extract(ctx) entitlementGraph := s.state.EntitlementGraph(ctx) From a5112c641e3e85a9ff85b3074ef32a27c83d2fd9 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 11:46:33 -0600 Subject: [PATCH 08/12] Remove the comment completely --- pkg/sync/syncer.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index 6900fc35..0cb8ef54 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -969,8 +969,6 @@ func (s *syncer) SyncAssets(ctx context.Context) error { return nil } -// SyncGrantExpansion -// TODO(morgabra) Docs func (s *syncer) SyncGrantExpansion(ctx context.Context) error { l := ctxzap.Extract(ctx) entitlementGraph := s.state.EntitlementGraph(ctx) From c9fc0bc6d62d8cde186e0e00be7f46671f96b63d Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 12:00:36 -0600 Subject: [PATCH 09/12] Document a method as having its documentation pending --- pkg/sync/syncer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index 0cb8ef54..6ecfa529 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -969,6 +969,8 @@ func (s *syncer) SyncAssets(ctx context.Context) error { return nil } +// SyncGrantExpansion documentation pending. +// TODO(morgabra) Docs func (s *syncer) SyncGrantExpansion(ctx context.Context) error { l := ctxzap.Extract(ctx) entitlementGraph := s.state.EntitlementGraph(ctx) From 267b1f16ecb4e0c72142f6baeac11fe02c9b1d9a Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Thu, 2 Jan 2025 13:49:35 -0600 Subject: [PATCH 10/12] Remove TODO line --- pkg/sync/syncer.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index 6ecfa529..cdcbfd2e 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -970,7 +970,6 @@ func (s *syncer) SyncAssets(ctx context.Context) error { } // SyncGrantExpansion documentation pending. -// TODO(morgabra) Docs func (s *syncer) SyncGrantExpansion(ctx context.Context) error { l := ctxzap.Extract(ctx) entitlementGraph := s.state.EntitlementGraph(ctx) From 0017a6450d6b27d0990e2d6b67c1fc083e2a1721 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Fri, 10 Jan 2025 17:58:03 -0600 Subject: [PATCH 11/12] Activate printing the response body from envvars - Adds `BATON_DEBUG_PRINT_RESPONSE_BODY` --- pkg/uhttp/body_print.go | 12 ------------ pkg/uhttp/wrapper.go | 11 ++++++----- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/pkg/uhttp/body_print.go b/pkg/uhttp/body_print.go index 5c1f1618..861e084b 100644 --- a/pkg/uhttp/body_print.go +++ b/pkg/uhttp/body_print.go @@ -38,15 +38,3 @@ func (pr *printReader) Read(p []byte) (int, error) { func wrapPrintBody(body io.Reader) io.Reader { return &printReader{reader: body} } - -type printBodyOption struct { - debugPrintBody bool -} - -func (o printBodyOption) Apply(c *BaseHttpClient) { - c.debugPrintBody = o.debugPrintBody -} - -func WithPrintBody(shouldPrint bool) WrapperOption { - return printBodyOption{debugPrintBody: shouldPrint} -} diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index 26672801..1426bc17 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -10,6 +10,7 @@ import ( "io" "net/http" "net/url" + "os" "syscall" "time" @@ -91,10 +92,9 @@ type ( NewRequest(ctx context.Context, method string, url *url.URL, options ...RequestOption) (*http.Request, error) } BaseHttpClient struct { - HttpClient *http.Client - rateLimiter uRateLimit.Limiter - baseHttpCache icache - debugPrintBody bool + HttpClient *http.Client + rateLimiter uRateLimit.Limiter + baseHttpCache icache } DoOption func(resp *WrapperResponse) error @@ -342,7 +342,8 @@ func (c *BaseHttpClient) Do(req *http.Request, options ...DoOption) (*http.Respo } // Replace resp.Body with a no-op closer so nobody has to worry about closing the reader. - if c.debugPrintBody { + shouldPrint := os.Getenv("BATON_DEBUG_PRINT_RESPONSE_BODY") + if shouldPrint != "" { resp.Body = io.NopCloser(wrapPrintBody(bytes.NewBuffer(body))) } else { resp.Body = io.NopCloser(bytes.NewBuffer(body)) From e9a3deb98ff8bc6e357bf583dbaa1985ffebdfb4 Mon Sep 17 00:00:00 2001 From: Jorge Javier Araya Navarro Date: Fri, 10 Jan 2025 18:02:08 -0600 Subject: [PATCH 12/12] Remove `logBody` unexported function --- pkg/uhttp/wrapper.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index 1426bc17..c1a61de0 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -34,8 +34,6 @@ const ( authorizationHeader = "Authorization" ) -const maxBodySize = 4096 - type WrapperResponse struct { Header http.Header Body []byte @@ -141,8 +139,8 @@ func WithJSONResponse(response interface{}) DoOption { if !IsJSONContentType(contentHeader) { if len(resp.Body) != 0 { - // we want to see the body regardless - return fmt.Errorf("unexpected content type for JSON response: %s. status code: %d. body: «%s»", contentHeader, resp.StatusCode, logBody(resp.Body, maxBodySize)) + // to print the response, set the envvar BATON_DEBUG_PRINT_RESPONSE_BODY as non-empty, instead + return fmt.Errorf("unexpected content type for JSON response: %s. status code: %d", contentHeader, resp.StatusCode) } return fmt.Errorf("unexpected content type for JSON response: %s. status code: %d", contentHeader, resp.StatusCode) } @@ -151,7 +149,8 @@ func WithJSONResponse(response interface{}) DoOption { } err := json.Unmarshal(resp.Body, response) if err != nil { - return fmt.Errorf("failed to unmarshal json response: %w. status code: %d. body %v", err, resp.StatusCode, logBody(resp.Body, maxBodySize)) + // to print the response, set the envvar BATON_DEBUG_PRINT_RESPONSE_BODY as non-empty, instead + return fmt.Errorf("failed to unmarshal json response: %w. status code: %d", err, resp.StatusCode) } return nil } @@ -165,19 +164,13 @@ func WithAlwaysJSONResponse(response interface{}) DoOption { } err := json.Unmarshal(resp.Body, response) if err != nil { - return fmt.Errorf("failed to unmarshal json response: %w. status code: %d. body %v", err, resp.StatusCode, logBody(resp.Body, maxBodySize)) + // to print the response, set the envvar BATON_DEBUG_PRINT_RESPONSE_BODY as non-empty, instead + return fmt.Errorf("failed to unmarshal json response: %w. status code: %d", err, resp.StatusCode) } return nil } } -func logBody(body []byte, size int) string { - if len(body) > size { - return string(body[:size]) + " ..." - } - return string(body) -} - type ErrorResponse interface { Message() string } @@ -191,12 +184,14 @@ func WithErrorResponse(resource ErrorResponse) DoOption { contentHeader := resp.Header.Get(ContentType) if !IsJSONContentType(contentHeader) { - return fmt.Errorf("unexpected content type for JSON error response: %s. status code: %d. body: «%s»", contentHeader, resp.StatusCode, logBody(resp.Body, maxBodySize)) + // to print the response, set the envvar BATON_DEBUG_PRINT_RESPONSE_BODY as non-empty, instead + return fmt.Errorf("unexpected content type for JSON error response: %s. status code: %d", contentHeader, resp.StatusCode) } // Decode the JSON response body into the ErrorResponse if err := json.Unmarshal(resp.Body, &resource); err != nil { - return fmt.Errorf("failed to unmarshal JSON error response: %w. status code: %d. body %v", err, resp.StatusCode, logBody(resp.Body, maxBodySize)) + // to print the response, set the envvar BATON_DEBUG_PRINT_RESPONSE_BODY as non-empty, instead + return fmt.Errorf("failed to unmarshal JSON error response: %w. status code: %d", err, resp.StatusCode) } // Construct a more detailed error message