From 4e86b02bda5b92c2fde6485b20358beb91587431 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 7 Feb 2024 16:46:50 +0000 Subject: [PATCH 01/16] Refactor logging implementation and add GetLogLevel method --- internal/httpclient/http_headers.go | 28 ++++ internal/httpclient/http_logger.go | 6 + internal/httpclient/http_logging.go.refactor | 83 ----------- internal/httpclient/http_request.go | 144 +++++++++++++++---- 4 files changed, 149 insertions(+), 112 deletions(-) create mode 100644 internal/httpclient/http_headers.go delete mode 100644 internal/httpclient/http_logging.go.refactor diff --git a/internal/httpclient/http_headers.go b/internal/httpclient/http_headers.go new file mode 100644 index 0000000..8cc8ec9 --- /dev/null +++ b/internal/httpclient/http_headers.go @@ -0,0 +1,28 @@ +package httpclient + +import ( + "net/http" +) + +// LogHTTPHeaders logs the HTTP headers of an HTTP request or response, with an option to hide sensitive information like the token in secure mode. +func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) { + var keysAndValues []interface{} + if secureMode { + for key, values := range headers { + if key != "Authorization" { // Exclude the token header + // Assuming each header has a single value for simplicity + keysAndValues = append(keysAndValues, key, values[0]) + } + } + } else { + for key, values := range headers { + // Assuming each header has a single value for simplicity + keysAndValues = append(keysAndValues, key, values[0]) + } + } + + // Log the headers using the logger from the httpclient package + if len(keysAndValues) > 0 { + logger.Debug("HTTP Headers", keysAndValues...) + } +} diff --git a/internal/httpclient/http_logger.go b/internal/httpclient/http_logger.go index 02a0703..3984b15 100644 --- a/internal/httpclient/http_logger.go +++ b/internal/httpclient/http_logger.go @@ -25,6 +25,7 @@ type Logger interface { Error(msg string, keysAndValues ...interface{}) Panic(msg string, keysAndValues ...interface{}) Fatal(msg string, keysAndValues ...interface{}) + GetLogLevel() LogLevel } // defaultLogger is an implementation of the Logger interface using Uber's zap logging library. @@ -108,3 +109,8 @@ func (d *defaultLogger) Fatal(msg string, keysAndValues ...interface{}) { d.logger.Fatal(msg, toZapFields(keysAndValues...)...) } } + +// GetLevel returns the current logging level +func (d *defaultLogger) GetLogLevel() LogLevel { + return d.logLevel +} diff --git a/internal/httpclient/http_logging.go.refactor b/internal/httpclient/http_logging.go.refactor deleted file mode 100644 index dbdbdfe..0000000 --- a/internal/httpclient/http_logging.go.refactor +++ /dev/null @@ -1,83 +0,0 @@ -// http_logging.go -package httpclient - -import "log" - -type LogLevel int - -const ( - LogLevelNone LogLevel = iota - LogLevelWarning - LogLevelInfo - LogLevelDebug -) - -// Logger is an interface for logging within the SDK. -type Logger interface { - SetLevel(level LogLevel) - Trace(msg string, keysAndValues ...interface{}) // For very detailed logs - Debug(msg string, keysAndValues ...interface{}) // For development and troubleshooting - Info(msg string, keysAndValues ...interface{}) // Informational messages - Warn(msg string, keysAndValues ...interface{}) // For potentially problematic situations - Error(msg string, keysAndValues ...interface{}) // For errors that might still allow the app to continue running - Fatal(msg string, keysAndValues ...interface{}) // For errors that might prevent the app from continuing -} - -// defaultLogger is the default logger based on Go's standard log package and includes a logLevel field to keep track of the current logging level. -type defaultLogger struct { - logLevel LogLevel -} - -// SetLevel sets the current logging level for the defaultLogger. -func (d *defaultLogger) SetLevel(level LogLevel) { - d.logLevel = level -} - -// NewDefaultLogger now initializes a defaultLogger with a default log level. -func NewDefaultLogger() Logger { - return &defaultLogger{ - logLevel: LogLevelWarning, // default log level. - } -} - -// Trace checks if the current log level permits debug messages before logging. -func (d *defaultLogger) Trace(msg string, keysAndValues ...interface{}) { - if d.logLevel >= LogLevelDebug { // Trace is a part of LogLevelDebug - log.Println("[TRACE]", msg, keysAndValues) - } -} - -// Debug checks if the current log level permits debug messages before logging. -func (d *defaultLogger) Debug(msg string, keysAndValues ...interface{}) { - if d.logLevel >= LogLevelDebug { - log.Println("[DEBUG]", msg, keysAndValues) - } -} - -// Info checks if the current log level permits debug messages before logging. -func (d *defaultLogger) Info(msg string, keysAndValues ...interface{}) { - if d.logLevel >= LogLevelInfo { - log.Println("[INFO]", msg, keysAndValues) - } -} - -// Warn checks if the current log level permits Warning messages before logging. -func (d *defaultLogger) Warn(msg string, keysAndValues ...interface{}) { - if d.logLevel >= LogLevelWarning { - log.Println("[WARN]", msg, keysAndValues) - } -} - -// Error checks if the current log level is greater than LogLevelNone, before logging. -func (d *defaultLogger) Error(msg string, keysAndValues ...interface{}) { - if d.logLevel > LogLevelNone { - log.Println("[ERROR]", msg, keysAndValues) - } -} - -// Fatal checks if the current log level is greater than LogLevelNone, before logging. -func (d *defaultLogger) Fatal(msg string, keysAndValues ...interface{}) { - if d.logLevel > LogLevelNone { - log.Fatalln("[FATAL]", msg, keysAndValues) - } -} diff --git a/internal/httpclient/http_request.go b/internal/httpclient/http_request.go index 2b82572..db93719 100644 --- a/internal/httpclient/http_request.go +++ b/internal/httpclient/http_request.go @@ -129,13 +129,16 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt // Start response time measurement responseTimeStart := time.Now() - // Execute Request with context + // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to send request", "method", method, "endpoint", endpoint, "error", err) + c.logger.Error("Failed to send multipart request", + "method", method, + "endpoint", endpoint, + "error", err.Error(), + ) return nil, err } - // After each request, compute and update response time responseDuration := time.Since(responseTimeStart) c.PerfMetrics.lock.Lock() @@ -147,45 +150,86 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt CheckDeprecationHeader(resp, c.logger) } - // Handle (unmarshall) response with API Handler + // Handle (unmarshal) response with API Handler if err := handler.UnmarshalResponse(resp, out); err != nil { switch e := err.(type) { - case *APIError: - c.logger.Error("Received an API error", "status_code", e.StatusCode, "message", e.Message) + case *APIError: // Assuming APIError is a type that includes StatusCode and Message + // Log the API error with structured logging + c.logger.Error("Received an API error", + "method", method, + "endpoint", endpoint, + "status_code", e.StatusCode, + "message", e.Message, + ) return resp, e default: - // Existing error handling logic - c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err) + // Log the default error with structured logging + c.logger.Error("Failed to unmarshal HTTP response", + "method", method, + "endpoint", endpoint, + "error", err.Error(), // Convert error to string to log as a value + ) return resp, err } } if resp.StatusCode >= 200 && resp.StatusCode < 300 { - c.logger.Info("HTTP request succeeded", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode) + // Log successful HTTP request + c.logger.Info("HTTP request succeeded", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + ) return resp, nil } else if resp.StatusCode == http.StatusNotFound { - c.logger.Warn("Resource not found", "method", method, "endpoint", endpoint) + // Log when resource is not found + c.logger.Warn("Resource not found", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + ) return resp, fmt.Errorf("resource not found: %s", endpoint) } // Retry Logic if IsNonRetryableError(resp) { - c.logger.Warn("Encountered a non-retryable error", "status", resp.StatusCode, "description", TranslateStatusCode(resp.StatusCode)) + c.logger.Warn("Encountered a non-retryable error", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "description", TranslateStatusCode(resp.StatusCode), + ) return resp, c.HandleAPIError(resp) } else if IsRateLimitError(resp) { - waitDuration := parseRateLimitHeaders(resp) // Checks for the Retry-After, X-RateLimit-Remaining and X-RateLimit-Reset headers - c.logger.Warn("Encountered a rate limit error. Retrying after wait duration.", "wait_duration", waitDuration) + waitDuration := parseRateLimitHeaders(resp) // Parses headers to determine wait duration + c.logger.Warn("Encountered a rate limit error. Retrying after wait duration.", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "wait_duration", waitDuration, + ) time.Sleep(waitDuration) i++ continue // This will restart the loop, effectively "retrying" the request } else if IsTransientError(resp) { - waitDuration := calculateBackoff(i) //uses exponential backoff (with jitter) - c.logger.Warn("Encountered a transient error. Retrying after backoff.", "wait_duration", waitDuration) + waitDuration := calculateBackoff(i) // Calculates backoff duration + c.logger.Warn("Encountered a transient error. Retrying after backoff.", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "wait_duration", waitDuration, + "attempt", i, + ) time.Sleep(waitDuration) i++ continue // This will restart the loop, effectively "retrying" the request } else { - c.logger.Error("Received unexpected error status from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, "description", TranslateStatusCode(resp.StatusCode)) + c.logger.Error("Received unexpected error status from HTTP request", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "description", TranslateStatusCode(resp.StatusCode), + ) return resp, c.HandleAPIError(resp) } } @@ -194,10 +238,14 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt responseTimeStart := time.Now() // For non-retryable HTTP Methods (POST - Create) req = req.WithContext(ctx) + // Execute the request resp, err := c.httpClient.Do(req) - if err != nil { - c.logger.Error("Failed to send request", "method", method, "endpoint", endpoint, "error", err) + c.logger.Error("Failed to send multipart request", + "method", method, + "endpoint", endpoint, + "error", err.Error(), + ) return nil, err } @@ -209,25 +257,46 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt CheckDeprecationHeader(resp, c.logger) - // Unmarshal the response with the determined API Handler + // Handle (unmarshal) response with API Handler if err := handler.UnmarshalResponse(resp, out); err != nil { switch e := err.(type) { - case *APIError: - c.logger.Error("Received an API error", "status_code", e.StatusCode, "message", e.Message) + case *APIError: // Assuming APIError is a type that includes StatusCode and Message + // Log the API error with structured logging + c.logger.Error("Received an API error", + "method", method, + "endpoint", endpoint, + "status_code", e.StatusCode, + "message", e.Message, + ) return resp, e default: - // Existing error handling logic - c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err) + // Log the default error with structured logging + c.logger.Error("Failed to unmarshal HTTP response", + "method", method, + "endpoint", endpoint, + "error", err.Error(), // Convert error to string to log as a value + ) return resp, err } } // Check if the response status code is within the success range if resp.StatusCode >= 200 && resp.StatusCode < 300 { + // Success, no need for logging return resp, nil } else { + // Translate the status code to a human-readable description statusDescription := TranslateStatusCode(resp.StatusCode) - c.logger.Error("Received non-success status code from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, "description", statusDescription) + + // Log the error with structured context + c.logger.Error("Received non-success status code from HTTP request", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "description", statusDescription, + ) + + // Return an error with the status code and its description return resp, fmt.Errorf("Error status code: %d - %s", resp.StatusCode, statusDescription) } } @@ -294,24 +363,41 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s // Debug: Print request headers if in debug mode - c.logger.Debug("HTTP Multipart Request Headers:", req.Header) + // Check if logging level is DEBUG or higher before logging headers + if c.logger.GetLogLevel() <= LogLevelDebug { + // Debug: Print request headers without hiding sensitive information + LogHTTPHeaders(c.logger, req.Header, false) // Use false to display all headers + } // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to send multipart request", "method", method, "endpoint", endpoint, "error", err) + c.logger.Error("Failed to send multipart request", + "method", method, + "endpoint", endpoint, + "error", err.Error(), + ) return nil, err } // Check for successful status code if resp.StatusCode < 200 || resp.StatusCode >= 300 { - c.logger.Error("Received non-success status code from multipart request", "status_code", resp.StatusCode) - return resp, fmt.Errorf("received non-success status code: %d", resp.StatusCode) + c.logger.Error("Received non-success status code from multipart request", + "method", method, + "endpoint", endpoint, + "status_code", resp.StatusCode, + "status_text", http.StatusText(resp.StatusCode), + ) + return resp, fmt.Errorf("received non-success status code: %d - %s", resp.StatusCode, http.StatusText(resp.StatusCode)) } // Unmarshal the response if err := handler.UnmarshalResponse(resp, out); err != nil { - c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err) + c.logger.Error("Failed to unmarshal HTTP response", + "method", method, + "endpoint", endpoint, + "error", err.Error(), + ) return resp, err } From 5ae2210ec02c5dab69800591d5e92f1651544800 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 7 Feb 2024 17:28:54 +0000 Subject: [PATCH 02/16] Refactor APIHandler and Client struct*** ***Update APIHandler interface and its implementations in APIHandler.go.*** ***Update Client struct and its methods in http_client.go.*** ***Update DoRequest and DoMultipartRequest methods in http_request.go.*** ***Add logger package and use logger.Logger interface in http_client.go. --- .../jamfpro/jamfpro_api_handler.go | 58 ++++++++++--------- internal/httpclient/api_handler.go | 12 ++-- internal/httpclient/http_client.go | 12 ++-- internal/httpclient/http_request.go | 24 ++++---- .../http_logger.go => logger/logger.go} | 2 +- 5 files changed, 55 insertions(+), 53 deletions(-) rename internal/{httpclient/http_logger.go => logger/logger.go} (99%) diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 135b160..22364d6 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -48,8 +48,6 @@ import ( "strings" _ "embed" - - "github.com/deploymenttheory/go-api-http-client/internal/httpclient" ) // Endpoint constants represent the URL suffixes used for Jamf API token interactions. @@ -104,9 +102,15 @@ type EndpointConfig struct { // JamfAPIHandler implements the APIHandler interface for the Jamf Pro API. type JamfAPIHandler struct { - logger httpclient.Logger // logger is used to output logs for the API handling processes. - OverrideBaseDomain string // OverrideBaseDomain is used to override the base domain for URL construction. - InstanceName string // InstanceName is the name of the Jamf instance. + OverrideBaseDomain string // OverrideBaseDomain is used to override the base domain for URL construction. + InstanceName string // InstanceName is the name of the Jamf instance. +} + +type Logger interface { + Debug(msg string, keysAndValues ...interface{}) + Info(msg string, keysAndValues ...interface{}) + Warn(msg string, keysAndValues ...interface{}) + Error(msg string, keysAndValues ...interface{}) } // Functions @@ -121,18 +125,18 @@ func (j *JamfAPIHandler) GetBaseDomain() string { } // ConstructAPIResourceEndpoint returns the full URL for a Jamf API resource endpoint path. -func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string) string { +func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string, logger Logger) string { baseDomain := j.GetBaseDomain() url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath) - j.logger.Info("Request will be made to API URL:", "URL", url) + logger.Info("Request will be made to API URL:", "URL", url) return url } // ConstructAPIAuthEndpoint returns the full URL for a Jamf API auth endpoint path. -func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string) string { +func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string, logger Logger) string { baseDomain := j.GetBaseDomain() url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath) - j.logger.Info("Request will be made to API authentication URL:", "URL", url) + logger.Info("Request will be made to API authentication URL:", "URL", url) return url } @@ -144,15 +148,15 @@ func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string) string { // - For url endpoints starting with "/api", it defaults to "application/json" for the JamfPro API. // If the endpoint does not match any of the predefined patterns, "application/json" is used as a fallback. // This method logs the decision process at various stages for debugging purposes. -func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string) string { +func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string, logger Logger) string { // Dynamic lookup from configuration should be the first priority for key, config := range configMap { if strings.HasPrefix(endpoint, key) { if config.ContentType != nil { - u.logger.Debug("Content-Type for endpoint found in configMap", "endpoint", endpoint, "content_type", *config.ContentType) + logger.Debug("Content-Type for endpoint found in configMap", "endpoint", endpoint, "content_type", *config.ContentType) return *config.ContentType } - u.logger.Debug("Content-Type for endpoint is nil in configMap, handling as special case", "endpoint", endpoint) + logger.Debug("Content-Type for endpoint is nil in configMap, handling as special case", "endpoint", endpoint) // If a nil ContentType is an expected case, do not set Content-Type header. return "" // Return empty to indicate no Content-Type should be set. } @@ -160,20 +164,20 @@ func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string) string { // If no specific configuration is found, then check for standard URL patterns. if strings.Contains(endpoint, "/JSSResource") { - u.logger.Debug("Content-Type for endpoint defaulting to XML for Classic API", "endpoint", endpoint) + logger.Debug("Content-Type for endpoint defaulting to XML for Classic API", "endpoint", endpoint) return "application/xml" // Classic API uses XML } else if strings.Contains(endpoint, "/api") { - u.logger.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", "endpoint", endpoint) + logger.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", "endpoint", endpoint) return "application/json" // JamfPro API uses JSON } // Fallback to JSON if no other match is found. - u.logger.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", "endpoint", endpoint) + logger.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", "endpoint", endpoint) return "application/json" } // MarshalRequest encodes the request body according to the endpoint for the API. -func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string) ([]byte, error) { +func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string, logger Logger) ([]byte, error) { var ( data []byte err error @@ -195,18 +199,18 @@ func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin } if method == "POST" || method == "PUT" { - u.logger.Trace("XML Request Body:", "Body", string(data)) + logger.Debug("XML Request Body:", "Body", string(data)) } case "json": data, err = json.Marshal(body) if err != nil { - u.logger.Error("Failed marshaling JSON request", "error", err) + logger.Error("Failed marshaling JSON request", "error", err) return nil, err } if method == "POST" || method == "PUT" || method == "PATCH" { - u.logger.Debug("JSON Request Body:", string(data)) + logger.Debug("JSON Request Body:", string(data)) } } @@ -214,7 +218,7 @@ func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin } // UnmarshalResponse decodes the response body from XML or JSON format depending on the Content-Type header. -func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}) error { +func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, logger Logger) error { // Handle DELETE method if resp.Request.Method == "DELETE" { if resp.StatusCode >= 200 && resp.StatusCode < 300 { @@ -226,16 +230,16 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}) bodyBytes, err := io.ReadAll(resp.Body) if err != nil { - u.logger.Error("Failed reading response body", "error", err) + logger.Error("Failed reading response body", "error", err) return err } // Log the raw response body and headers - u.logger.Trace("Raw HTTP Response:", string(bodyBytes)) - u.logger.Debug("Unmarshaling response", "status", resp.Status) + logger.Trace("Raw HTTP Response:", string(bodyBytes)) + logger.Debug("Unmarshaling response", "status", resp.Status) // Log headers when in debug mode - u.logger.Debug("HTTP Response Headers:", resp.Header) + logger.Debug("HTTP Response Headers:", resp.Header) // Check the Content-Type and Content-Disposition headers contentType := resp.Header.Get("Content-Type") @@ -249,7 +253,7 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}) // If content type is HTML, extract the error message if strings.Contains(contentType, "text/html") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - u.logger.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode) + logger.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode) return &APIError{ StatusCode: resp.StatusCode, Message: errMsg, @@ -289,12 +293,12 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}) // If unmarshalling fails, check if the content might be HTML if strings.Contains(string(bodyBytes), "") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - u.logger.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode) + logger.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode) return fmt.Errorf(errMsg) } // Log the error and return it - u.logger.Error("Failed to unmarshal response", "error", err) + logger.Error("Failed to unmarshal response", "error", err) return fmt.Errorf("failed to unmarshal response: %v", err) } diff --git a/internal/httpclient/api_handler.go b/internal/httpclient/api_handler.go index 7af3e02..1cba14b 100644 --- a/internal/httpclient/api_handler.go +++ b/internal/httpclient/api_handler.go @@ -13,12 +13,12 @@ import ( type APIHandler interface { GetBaseDomain() string ConstructAPIResourceEndpoint(endpointPath string) string - ConstructAPIAuthEndpoint(endpointPath string) string - MarshalRequest(body interface{}, method string, endpoint string) ([]byte, error) - MarshalMultipartRequest(fields map[string]string, files map[string]string) ([]byte, string, error) // New method for multipart - UnmarshalResponse(resp *http.Response, out interface{}) error - GetContentTypeHeader(method string) string - GetAcceptHeader() string + ConstructAPIAuthEndpoint(endpointPath string, logger Logger) string + MarshalRequest(body interface{}, method string, endpoint string, logger Logger) ([]byte, error) + MarshalMultipartRequest(fields map[string]string, files map[string]string, logger Logger) ([]byte, string, error) + UnmarshalResponse(resp *http.Response, out interface{}, logger Logger) error + GetContentTypeHeader(method string, logger Logger) string + GetAcceptHeader(logger Logger) string } // LoadAPIHandler returns an APIHandler based on the provided API type. diff --git a/internal/httpclient/http_client.go b/internal/httpclient/http_client.go index 7e743da..584293e 100644 --- a/internal/httpclient/http_client.go +++ b/internal/httpclient/http_client.go @@ -13,6 +13,7 @@ import ( "sync" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -23,11 +24,10 @@ type Config struct { Auth AuthConfig // User can either supply these values manually or pass from LoadAuthConfig/Env vars APIType string `json:"apiType"` // Optional - LogLevel LogLevel // Field for defining tiered logging level. - MaxRetryAttempts int // Config item defines the max number of retry request attempts for retryable HTTP methods. + LogLevel logger.LogLevel // Field for defining tiered logging level. + MaxRetryAttempts int // Config item defines the max number of retry request attempts for retryable HTTP methods. EnableDynamicRateLimiting bool - Logger Logger // Field for the packages initailzed logger - MaxConcurrentRequests int // Field for defining the maximum number of concurrent requests allowed in the semaphore + MaxConcurrentRequests int // Field for defining the maximum number of concurrent requests allowed in the semaphore TokenRefreshBufferPeriod time.Duration TotalRetryDuration time.Duration CustomTimeout time.Duration @@ -67,7 +67,7 @@ type Client struct { httpClient *http.Client tokenLock sync.Mutex config Config - logger Logger + logger logger.Logger ConcurrencyMgr *ConcurrencyManager PerfMetrics PerformanceMetrics } @@ -75,7 +75,7 @@ type Client struct { // BuildClient creates a new HTTP client with the provided configuration. func BuildClient(config Config) (*Client, error) { // Use the Logger interface type for the logger variable - var logger Logger + var logger logger.Logger if config.Logger == nil { logger = NewDefaultLogger() } else { diff --git a/internal/httpclient/http_request.go b/internal/httpclient/http_request.go index db93719..c066aae 100644 --- a/internal/httpclient/http_request.go +++ b/internal/httpclient/http_request.go @@ -67,17 +67,16 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt ctx = context.WithValue(ctx, requestIDKey{}, requestID) // Determine which set of encoding and content-type request rules to use - //handler := GetAPIHandler(endpoint, c.config.LogLevel) - handler := GetAPIHandler(c.config) + apiHandler := c.APIHandler // Marshal Request with correct encoding - requestData, err := handler.MarshalRequest(body, method, endpoint) + requestData, err := apiHandler.MarshalRequest(body, method, endpoint, c.logger) if err != nil { return nil, err } // Construct URL using the ConstructAPIResourceEndpoint function - url := c.ConstructAPIResourceEndpoint(endpoint) + url := apiHandler.ConstructAPIResourceEndpoint(endpoint) // Initialize total request counter c.PerfMetrics.lock.Lock() @@ -91,9 +90,9 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt } // Define header content type based on url and http method - contentType := handler.GetContentTypeHeader(endpoint) + contentType := apiHandler.GetContentTypeHeader(endpoint, c.logger) // Define Request Headers dynamically based on handler logic - acceptHeader := handler.GetAcceptHeader() + acceptHeader := apiHandler.GetAcceptHeader(c.logger) // Set Headers req.Header.Add("Authorization", "Bearer "+c.Token) @@ -151,7 +150,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt } // Handle (unmarshal) response with API Handler - if err := handler.UnmarshalResponse(resp, out); err != nil { + if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { switch e := err.(type) { case *APIError: // Assuming APIError is a type that includes StatusCode and Message // Log the API error with structured logging @@ -258,7 +257,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt CheckDeprecationHeader(resp, c.logger) // Handle (unmarshal) response with API Handler - if err := handler.UnmarshalResponse(resp, out); err != nil { + if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { switch e := err.(type) { case *APIError: // Assuming APIError is a type that includes StatusCode and Message // Log the API error with structured logging @@ -338,17 +337,16 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s } // Determine which set of encoding and content-type request rules to use - //handler := GetAPIHandler(endpoint, c.config.LogLevel) - handler := GetAPIHandler(c.config) + apiHandler := c.APIHandler // Marshal the multipart form data - requestData, contentType, err := handler.MarshalMultipartRequest(fields, files) + requestData, contentType, err := apiHandler.MarshalMultipartRequest(fields, files, c.logger) if err != nil { return nil, err } // Construct URL using the ConstructAPIResourceEndpoint function - url := c.ConstructAPIResourceEndpoint(endpoint) + url := apiHandler.ConstructAPIResourceEndpoint(endpoint) // Create the request req, err := http.NewRequest(method, url, bytes.NewBuffer(requestData)) @@ -392,7 +390,7 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s } // Unmarshal the response - if err := handler.UnmarshalResponse(resp, out); err != nil { + if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, diff --git a/internal/httpclient/http_logger.go b/internal/logger/logger.go similarity index 99% rename from internal/httpclient/http_logger.go rename to internal/logger/logger.go index 3984b15..de22c1a 100644 --- a/internal/httpclient/http_logger.go +++ b/internal/logger/logger.go @@ -1,4 +1,4 @@ -package httpclient +package logger import ( "go.uber.org/zap" From 0d8e4d0326c864f42cbf46b8224f6548e8df49ff Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 06:25:22 +0000 Subject: [PATCH 03/16] Refactor APIHandler interface and LoadAPIHandler function --- .../jamfpro/jamfpro_api_handler.go | 59 ++++--- internal/httpclient/api_handler.go | 35 ++-- internal/httpclient/http_client.go | 62 ++++--- internal/logger/logger.go | 162 +++++++++++++----- internal/logger/sensitive_values.go.fixme | 59 +++++++ 5 files changed, 254 insertions(+), 123 deletions(-) create mode 100644 internal/logger/sensitive_values.go.fixme diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 22364d6..1b30c70 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -48,6 +48,9 @@ import ( "strings" _ "embed" + + "github.com/deploymenttheory/go-api-http-client/internal/logger" + "go.uber.org/zap" ) // Endpoint constants represent the URL suffixes used for Jamf API token interactions. @@ -124,19 +127,19 @@ func (j *JamfAPIHandler) GetBaseDomain() string { return DefaultBaseDomain } -// ConstructAPIResourceEndpoint returns the full URL for a Jamf API resource endpoint path. -func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string, logger Logger) string { +// ConstructAPIResourceEndpoint constructs the full URL for a Jamf API resource endpoint path and logs the URL. +func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string, log logger.Logger) string { baseDomain := j.GetBaseDomain() url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath) - logger.Info("Request will be made to API URL:", "URL", url) + log.Info("Constructed API resource endpoint URL", zap.String("URL", url)) return url } -// ConstructAPIAuthEndpoint returns the full URL for a Jamf API auth endpoint path. -func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string, logger Logger) string { +// ConstructAPIAuthEndpoint constructs the full URL for a Jamf API auth endpoint path and logs the URL. +func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string, log logger.Logger) string { baseDomain := j.GetBaseDomain() url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath) - logger.Info("Request will be made to API authentication URL:", "URL", url) + log.Info("Constructed API authentication URL", zap.String("URL", url)) return url } @@ -148,15 +151,15 @@ func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string, logger Lo // - For url endpoints starting with "/api", it defaults to "application/json" for the JamfPro API. // If the endpoint does not match any of the predefined patterns, "application/json" is used as a fallback. // This method logs the decision process at various stages for debugging purposes. -func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string, logger Logger) string { +func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string, log logger.Logger) string { // Dynamic lookup from configuration should be the first priority for key, config := range configMap { if strings.HasPrefix(endpoint, key) { if config.ContentType != nil { - logger.Debug("Content-Type for endpoint found in configMap", "endpoint", endpoint, "content_type", *config.ContentType) + log.Debug("Content-Type for endpoint found in configMap", zap.String("endpoint", endpoint), zap.String("content_type", *config.ContentType)) return *config.ContentType } - logger.Debug("Content-Type for endpoint is nil in configMap, handling as special case", "endpoint", endpoint) + log.Debug("Content-Type for endpoint is nil in configMap, handling as special case", zap.String("endpoint", endpoint)) // If a nil ContentType is an expected case, do not set Content-Type header. return "" // Return empty to indicate no Content-Type should be set. } @@ -164,20 +167,20 @@ func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string, logger Logger) st // If no specific configuration is found, then check for standard URL patterns. if strings.Contains(endpoint, "/JSSResource") { - logger.Debug("Content-Type for endpoint defaulting to XML for Classic API", "endpoint", endpoint) + log.Debug("Content-Type for endpoint defaulting to XML for Classic API", zap.String("endpoint", endpoint)) return "application/xml" // Classic API uses XML } else if strings.Contains(endpoint, "/api") { - logger.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", "endpoint", endpoint) + log.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", zap.String("endpoint", endpoint)) return "application/json" // JamfPro API uses JSON } // Fallback to JSON if no other match is found. - logger.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", "endpoint", endpoint) + log.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", zap.String("endpoint", endpoint)) return "application/json" } // MarshalRequest encodes the request body according to the endpoint for the API. -func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string, logger Logger) ([]byte, error) { +func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string, log logger.Logger) ([]byte, error) { var ( data []byte err error @@ -199,18 +202,18 @@ func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin } if method == "POST" || method == "PUT" { - logger.Debug("XML Request Body:", "Body", string(data)) + log.Debug("XML Request Body", zap.String("Body", string(data))) } case "json": data, err = json.Marshal(body) if err != nil { - logger.Error("Failed marshaling JSON request", "error", err) + log.Error("Failed marshaling JSON request", zap.Error(err)) return nil, err } if method == "POST" || method == "PUT" || method == "PATCH" { - logger.Debug("JSON Request Body:", string(data)) + log.Debug("JSON Request Body", zap.String("Body", string(data))) } } @@ -218,28 +221,28 @@ func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin } // UnmarshalResponse decodes the response body from XML or JSON format depending on the Content-Type header. -func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, logger Logger) error { +func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, log logger.Logger) error { // Handle DELETE method if resp.Request.Method == "DELETE" { if resp.StatusCode >= 200 && resp.StatusCode < 300 { return nil } else { - return fmt.Errorf("DELETE request failed with status code: %d", resp.StatusCode) + return log.Error("DELETE request failed", zap.Int("Status Code", resp.StatusCode)) } } bodyBytes, err := io.ReadAll(resp.Body) if err != nil { - logger.Error("Failed reading response body", "error", err) + log.Error("Failed reading response body", zap.Error(err)) return err } // Log the raw response body and headers - logger.Trace("Raw HTTP Response:", string(bodyBytes)) - logger.Debug("Unmarshaling response", "status", resp.Status) + log.Debug("Raw HTTP Response", zap.String("Body", string(bodyBytes))) + log.Debug("Unmarshaling response", zap.String("status", resp.Status)) // Log headers when in debug mode - logger.Debug("HTTP Response Headers:", resp.Header) + log.Debug("HTTP Response Headers", zap.Any("Headers", resp.Header)) // Check the Content-Type and Content-Disposition headers contentType := resp.Header.Get("Content-Type") @@ -253,7 +256,7 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, // If content type is HTML, extract the error message if strings.Contains(contentType, "text/html") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - logger.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode) + log.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode) return &APIError{ StatusCode: resp.StatusCode, Message: errMsg, @@ -266,14 +269,14 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, if strings.Contains(contentType, "application/json") { description, err := ParseJSONErrorResponse(bodyBytes) if err != nil { - u.logger.Error("Failed to parse JSON error response", "error", err) + log.Error("Failed to parse JSON error response", "error", err) return fmt.Errorf("received non-success status code: %d and failed to parse error response", resp.StatusCode) } return fmt.Errorf("received non-success status code: %d, error: %s", resp.StatusCode, description) } // If the response is not JSON or another error occurs, return a generic error message - u.logger.Error("Received non-success status code", "status_code", resp.StatusCode) + log.Error("Received non-success status code", "status_code", resp.StatusCode) return fmt.Errorf("received non-success status code: %d", resp.StatusCode) } @@ -293,12 +296,12 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, // If unmarshalling fails, check if the content might be HTML if strings.Contains(string(bodyBytes), "") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - logger.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode) + log.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode) return fmt.Errorf(errMsg) } // Log the error and return it - logger.Error("Failed to unmarshal response", "error", err) + log.Error("Failed to unmarshal response", "error", err) return fmt.Errorf("failed to unmarshal response: %v", err) } @@ -331,7 +334,7 @@ func (u *JamfAPIHandler) GetAcceptHeader() string { } // MarshalMultipartFormData takes a map with form fields and file paths and returns the encoded body and content type. -func (u *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string) ([]byte, string, error) { +func (u *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) diff --git a/internal/httpclient/api_handler.go b/internal/httpclient/api_handler.go index 1cba14b..82fb278 100644 --- a/internal/httpclient/api_handler.go +++ b/internal/httpclient/api_handler.go @@ -2,47 +2,46 @@ package httpclient import ( - "fmt" "net/http" "github.com/deploymenttheory/go-api-http-client/internal/apihandlers/jamfpro" + "github.com/deploymenttheory/go-api-http-client/internal/logger" + "go.uber.org/zap" ) // APIHandler is an interface for encoding, decoding, and determining content types for different API implementations. // It encapsulates behavior for encoding and decoding requests and responses. type APIHandler interface { GetBaseDomain() string - ConstructAPIResourceEndpoint(endpointPath string) string - ConstructAPIAuthEndpoint(endpointPath string, logger Logger) string - MarshalRequest(body interface{}, method string, endpoint string, logger Logger) ([]byte, error) - MarshalMultipartRequest(fields map[string]string, files map[string]string, logger Logger) ([]byte, string, error) - UnmarshalResponse(resp *http.Response, out interface{}, logger Logger) error - GetContentTypeHeader(method string, logger Logger) string - GetAcceptHeader(logger Logger) string + ConstructAPIResourceEndpoint(endpointPath string, log logger.Logger) string + ConstructAPIAuthEndpoint(endpointPath string, log logger.Logger) string + MarshalRequest(body interface{}, method string, endpoint string, log logger.Logger) ([]byte, error) + MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error) + UnmarshalResponse(resp *http.Response, out interface{}, log logger.Logger) error + GetContentTypeHeader(method string, log logger.Logger) string + GetAcceptHeader() string } // LoadAPIHandler returns an APIHandler based on the provided API type. // 'apiType' parameter could be "jamf" or "graph" to specify which API handler to load. -func LoadAPIHandler(config Config, apiType string) (APIHandler, error) { +func LoadAPIHandler(apiType string, log logger.Logger) (APIHandler, error) { var apiHandler APIHandler switch apiType { case "jamfpro": - // Assuming GetAPIHandler returns a JamfAPIHandler apiHandler = &jamfpro.JamfAPIHandler{ // Initialize with necessary parameters } + log.Info("API handler loaded successfully", zap.String("APIType", apiType)) + /*case "graph": - // Assuming GetAPIHandler returns a GraphAPIHandler apiHandler = &graph.GraphAPIHandler{ - // Initialize with necessary parameters - }*/ + // Initialize with necessary parameters + } + log.Info("API handler loaded successfully", zap.String("APIType", apiType)) + */ default: - return nil, fmt.Errorf("unsupported API type: %s", apiType) + return nil, log.Error("Unsupported API type", zap.String("APIType", apiType)) } - // Set the logger level for the handler if needed - logger := NewDefaultLogger() // Or use config.Logger if it's not nil - logger.SetLevel(config.LogLevel) - return apiHandler, nil } diff --git a/internal/httpclient/http_client.go b/internal/httpclient/http_client.go index 584293e..b498ba3 100644 --- a/internal/httpclient/http_client.go +++ b/internal/httpclient/http_client.go @@ -8,7 +8,6 @@ like the baseURL, authentication details, and an embedded standard HTTP client. package httpclient import ( - "fmt" "net/http" "sync" "time" @@ -67,75 +66,73 @@ type Client struct { httpClient *http.Client tokenLock sync.Mutex config Config - logger logger.Logger + Logger logger.Logger ConcurrencyMgr *ConcurrencyManager PerfMetrics PerformanceMetrics } // BuildClient creates a new HTTP client with the provided configuration. func BuildClient(config Config) (*Client, error) { - // Use the Logger interface type for the logger variable - var logger logger.Logger - if config.Logger == nil { - logger = NewDefaultLogger() - } else { - logger = config.Logger - } + // Initialize the logger using the NewDefaultLogger function from the logger package. + log := logger.NewDefaultLogger() - // Set the logger's level based on the provided configuration if present - logger.SetLevel(config.LogLevel) + // Set the logger's level based on the provided configuration. + log.SetLevel(config.LogLevel) - // Validate LogLevel - if config.LogLevel < LogLevelNone || config.LogLevel > LogLevelDebug { - return nil, fmt.Errorf("invalid LogLevel setting: %d", config.LogLevel) + if config.LogLevel < logger.LogLevelDebug || config.LogLevel > logger.LogLevelFatal { + return nil, log.Error("Invalid LogLevel setting", zap.Int("Provided LogLevel", int(config.LogLevel))) } // Use the APIType from the config to determine which API handler to load - apiHandler, err := LoadAPIHandler(config, config.APIType) + apiHandler, err := LoadAPIHandler(config.APIType, log) if err != nil { - logger.Error("Failed to load API handler", zap.String("APIType", config.APIType), zap.Error(err)) - return nil, err // Return the original error without wrapping it in fmt.Errorf + return nil, log.Error("Failed to load API handler", zap.String("APIType", config.APIType), zap.Error(err)) } - logger.Info("Initializing new HTTP client", zap.String("InstanceName", config.InstanceName), zap.String("APIType", config.APIType), zap.Int("LogLevel", int(config.LogLevel))) + log.Info("Initializing new HTTP client", + zap.String("InstanceName", config.InstanceName), // Using zap.String for structured logging + zap.String("APIType", config.APIType), // Using zap.String for structured logging + zap.Int("LogLevel", int(config.LogLevel)), // Using zap.Int to log LogLevel as an integer + ) + // Validate and set default values for the configuration if config.InstanceName == "" { - return nil, fmt.Errorf("instanceName cannot be empty") + return nil, log.Error("InstanceName cannot be empty") } if config.MaxRetryAttempts < 0 { config.MaxRetryAttempts = DefaultMaxRetryAttempts - logger.Info("MaxRetryAttempts was negative, set to default value", zap.Int("MaxRetryAttempts", DefaultMaxRetryAttempts)) + log.Info("MaxRetryAttempts was negative, set to default value", zap.Int("MaxRetryAttempts", DefaultMaxRetryAttempts)) } if config.MaxConcurrentRequests <= 0 { config.MaxConcurrentRequests = DefaultMaxConcurrentRequests - logger.Info("MaxConcurrentRequests was negative or zero, set to default value", zap.Int("MaxConcurrentRequests", DefaultMaxConcurrentRequests)) + log.Info("MaxConcurrentRequests was negative or zero, set to default value", zap.Int("MaxConcurrentRequests", DefaultMaxConcurrentRequests)) } if config.TokenRefreshBufferPeriod < 0 { config.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod - logger.Info("TokenRefreshBufferPeriod was negative, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) + log.Info("TokenRefreshBufferPeriod was negative, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) } if config.TotalRetryDuration <= 0 { config.TotalRetryDuration = DefaultTotalRetryDuration - logger.Info("TotalRetryDuration was negative or zero, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) + log.Info("TotalRetryDuration was negative or zero, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) } if config.TokenRefreshBufferPeriod == 0 { config.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod - logger.Info("TokenRefreshBufferPeriod not set, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) + log.Info("TokenRefreshBufferPeriod not set, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) } if config.TotalRetryDuration == 0 { config.TotalRetryDuration = DefaultTotalRetryDuration - logger.Info("TotalRetryDuration not set, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) + log.Info("TotalRetryDuration not set, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) } if config.CustomTimeout == 0 { config.CustomTimeout = DefaultTimeout - logger.Info("CustomTimeout not set, set to default value", zap.Duration("CustomTimeout", DefaultTimeout)) + log.Info("CustomTimeout not set, set to default value", zap.Duration("CustomTimeout", DefaultTimeout)) } // Determine the authentication method @@ -145,7 +142,7 @@ func BuildClient(config Config) (*Client, error) { } else if config.Auth.ClientID != "" && config.Auth.ClientSecret != "" { AuthMethod = "oauth" } else { - return nil, fmt.Errorf("invalid AuthConfig") + return nil, log.Error("Invalid AuthConfig", zap.String("Username", config.Auth.Username), zap.String("ClientID", config.Auth.ClientID)) } client := &Client{ @@ -154,21 +151,20 @@ func BuildClient(config Config) (*Client, error) { AuthMethod: AuthMethod, httpClient: &http.Client{Timeout: config.CustomTimeout}, config: config, - logger: logger, - ConcurrencyMgr: NewConcurrencyManager(config.MaxConcurrentRequests, logger, true), + Logger: log, + ConcurrencyMgr: NewConcurrencyManager(config.MaxConcurrentRequests, log, true), PerfMetrics: PerformanceMetrics{}, } - // Get auth token + // Authenticate and check token validity. _, err = client.ValidAuthTokenCheck() if err != nil { - logger.Error("Failed to validate or obtain auth token", zap.Error(err)) - return nil, fmt.Errorf("failed to validate auth: %w", err) + return nil, log.Error("Failed to validate or obtain auth token", zap.Error(err)) } go client.StartMetricEvaluation() - logger.Info("New client initialized", zap.String("InstanceName", client.InstanceName), zap.String("AuthMethod", AuthMethod), zap.Int("MaxRetryAttempts", config.MaxRetryAttempts), zap.Int("MaxConcurrentRequests", config.MaxConcurrentRequests), zap.Bool("EnableDynamicRateLimiting", config.EnableDynamicRateLimiting)) + log.Info("New client initialized", zap.String("InstanceName", client.InstanceName), zap.String("AuthMethod", AuthMethod), zap.Int("MaxRetryAttempts", config.MaxRetryAttempts), zap.Int("MaxConcurrentRequests", config.MaxConcurrentRequests), zap.Bool("EnableDynamicRateLimiting", config.EnableDynamicRateLimiting)) return client, nil diff --git a/internal/logger/logger.go b/internal/logger/logger.go index de22c1a..2bbe906 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -1,30 +1,47 @@ +// logger.go package logger +// Ref: https://betterstack.com/community/guides/logging/go/zap/#logging-errors-with-zap import ( + "fmt" + "os" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) +// LogLevel represents the level of logging. Higher values denote more severe log messages. type LogLevel int const ( - LogLevelNone LogLevel = iota - LogLevelDebug + // LogLevelDebug is for messages that are useful during software debugging. + LogLevelDebug LogLevel = iota - 1 // Adjusted value to -1 to follow Zap's convention for DEBUG. + // LogLevelInfo is for informational messages, indicating normal operation. LogLevelInfo - LogLevelWarning + // LogLevelWarn is for messages that highlight potential issues in the system. + LogLevelWarn + // LogLevelError is for messages that highlight errors in the application's execution. LogLevelError + // LogLevelDPanic is for severe error conditions that are actionable in development. It panics in development and logs as an error in production. + LogLevelDPanic + // LogLevelPanic is for severe error conditions that should cause the program to panic. LogLevelPanic + // LogLevelFatal is for errors that require immediate program termination. LogLevelFatal + // LogLevelNone is used to disable logging. + LogLevelNone ) -// Logger interface as defined earlier +// Logger interface with structured logging capabilities at various levels. type Logger interface { SetLevel(level LogLevel) - Debug(msg string, keysAndValues ...interface{}) - Info(msg string, keysAndValues ...interface{}) - Warn(msg string, keysAndValues ...interface{}) - Error(msg string, keysAndValues ...interface{}) - Panic(msg string, keysAndValues ...interface{}) - Fatal(msg string, keysAndValues ...interface{}) + Debug(msg string, fields ...zapcore.Field) + Info(msg string, fields ...zapcore.Field) + Warn(msg string, fields ...zapcore.Field) + Error(msg string, fields ...zapcore.Field) error + Panic(msg string, fields ...zapcore.Field) + Fatal(msg string, fields ...zapcore.Field) + With(fields ...zapcore.Field) Logger GetLogLevel() LogLevel } @@ -36,30 +53,51 @@ type defaultLogger struct { logLevel LogLevel // logLevel determines the current logging level (e.g., DEBUG, INFO, WARN). } -// NewDefaultLogger initializes and returns a new instance of defaultLogger with a production -// configuration from the zap logging library. This function sets the default logging level to -// LogLevelWarning, which means that by default, DEBUG and INFO logs will be suppressed. -// In case of an error while initializing the zap.Logger, this function will panic, as the -// inability to log is considered a fatal error in production environments. +// NewDefaultLogger creates and returns a new logger instance with a default production configuration. +// It uses JSON formatting for log messages and sets the initial log level to Info. If the logger cannot +// be initialized, the function panics to indicate a critical setup failure. func NewDefaultLogger() Logger { - logger, err := zap.NewProduction() // Initialize a zap logger with production settings. + // Set up custom encoder configuration + encoderCfg := zap.NewProductionEncoderConfig() + encoderCfg.TimeKey = "timestamp" + encoderCfg.EncodeTime = zapcore.ISO8601TimeEncoder // Use ISO8601 format for timestamps + + // Define the logger configuration + config := zap.Config{ + Level: zap.NewAtomicLevelAt(zap.InfoLevel), // Default log level is Info + Development: false, // Set to true if the logger is used in a development environment + Encoding: "json", // Use JSON format for structured logging + EncoderConfig: encoderCfg, + OutputPaths: []string{"stdout"}, // Log to standard output + ErrorOutputPaths: []string{"stderr"}, // Log internal Zap errors to standard error + InitialFields: map[string]interface{}{ + "application": "your-application-name", // Customize this field to suit your needs + }, + } + + // Build the logger from the configuration + logger, err := config.Build() if err != nil { - panic(err) // Panic if there is an error initializing the logger, as logging is critical. + panic(err) // Panic if there is an error initializing the logger } + // Wrap the Zap logger in your defaultLogger struct, which implements your Logger interface return &defaultLogger{ - logger: logger, // Set the initialized zap.Logger. - logLevel: LogLevelWarning, // Set the default log level to warning. + logger: logger, + logLevel: LogLevelInfo, // Assuming LogLevelInfo maps to zap.InfoLevel } } -// Implement the SetLevel method for defaultLogger +// SetLevel updates the logging level of the logger. It controls the verbosity of the logs, +// allowing the option to filter out less severe messages based on the specified level. func (d *defaultLogger) SetLevel(level LogLevel) { d.logLevel = level } -// Convert keysAndValues to zap.Fields -func toZapFields(keysAndValues ...interface{}) []zap.Field { +// ToZapFields converts a variadic list of key-value pairs into a slice of Zap fields. +// This allows for structured logging with strongly-typed values. The function assumes +// that keys are strings and values can be of any type, leveraging zap.Any for type detection. +func ToZapFields(keysAndValues ...interface{}) []zap.Field { var fields []zap.Field for i := 0; i < len(keysAndValues)-1; i += 2 { key, val := keysAndValues[i], keysAndValues[i+1] @@ -68,49 +106,85 @@ func toZapFields(keysAndValues ...interface{}) []zap.Field { return fields } -// Debug method implementation -func (d *defaultLogger) Debug(msg string, keysAndValues ...interface{}) { +// Debug logs a message at the Debug level. This level is typically used for detailed troubleshooting +// information that is only relevant during active development or debugging. +func (d *defaultLogger) Debug(msg string, fields ...zapcore.Field) { if d.logLevel >= LogLevelDebug { - d.logger.Debug(msg, toZapFields(keysAndValues...)...) + d.logger.Debug(msg, fields...) } } -// Info method implementation -func (d *defaultLogger) Info(msg string, keysAndValues ...interface{}) { +// Info logs a message at the Info level. This level is used for informational messages that highlight +// the normal operation of the application. +func (d *defaultLogger) Info(msg string, fields ...zapcore.Field) { if d.logLevel >= LogLevelInfo { - d.logger.Info(msg, toZapFields(keysAndValues...)...) + d.logger.Info(msg, fields...) } } -// Warn method implementation -func (d *defaultLogger) Warn(msg string, keysAndValues ...interface{}) { - if d.logLevel >= LogLevelWarning { - d.logger.Warn(msg, toZapFields(keysAndValues...)...) +// Warn logs a message at the Warn level. This level is used for potentially harmful situations or to +// indicate that some issues may require attention. +func (d *defaultLogger) Warn(msg string, fields ...zapcore.Field) { + if d.logLevel >= LogLevelWarn { + d.logger.Warn(msg, fields...) } } -// Error method implementation -func (d *defaultLogger) Error(msg string, keysAndValues ...interface{}) { - if d.logLevel > LogLevelNone { - d.logger.Error(msg, toZapFields(keysAndValues...)...) - } +// Error logs a message at the Error level. This level is used to log error events that might still allow +// the application to continue running. +// Error logs a message at the Error level and returns a formatted error. +func (d *defaultLogger) Error(msg string, fields ...zapcore.Field) error { + d.logger.Error(msg, fields...) + return fmt.Errorf(msg) } -// Panic method implementation -func (d *defaultLogger) Panic(msg string, keysAndValues ...interface{}) { +// Panic logs a message at the Panic level and then panics. This level is used to log severe error events +// that will likely lead the application to abort. +func (d *defaultLogger) Panic(msg string, fields ...zapcore.Field) { if d.logLevel >= LogLevelPanic { - d.logger.Panic(msg, toZapFields(keysAndValues...)...) + d.logger.Panic(msg, fields...) } } -// Fatal method implementation -func (d *defaultLogger) Fatal(msg string, keysAndValues ...interface{}) { +// Fatal logs a message at the Fatal level and then calls os.Exit(1). This level is used to log severe +// error events that will result in the termination of the application. +func (d *defaultLogger) Fatal(msg string, fields ...zapcore.Field) { if d.logLevel >= LogLevelFatal { - d.logger.Fatal(msg, toZapFields(keysAndValues...)...) + d.logger.Fatal(msg, fields...) + } +} + +// With adds contextual key-value pairs to the logger, returning a new logger instance with the context. +// This is useful for creating a logger with common fields that should be included in all subsequent log entries. +func (d *defaultLogger) With(fields ...zapcore.Field) Logger { + return &defaultLogger{ + logger: d.logger.With(fields...), + logLevel: d.logLevel, } } -// GetLevel returns the current logging level +// GetLogLevel returns the current logging level of the logger. This allows for checking the logger's +// verbosity level programmatically, which can be useful in conditional logging scenarios. func (d *defaultLogger) GetLogLevel() LogLevel { return d.logLevel } + +// GetLoggerBasedOnEnv returns a zap.Logger instance configured for either +// production or development based on the APP_ENV environment variable. +// If APP_ENV is set to "development", it returns a development logger. +// Otherwise, it defaults to a production logger. +func GetLoggerBasedOnEnv() *zap.Logger { + if os.Getenv("APP_ENV") == "development" { + logger, err := zap.NewDevelopment() + if err != nil { + panic(err) // Handle error according to your application's error policy + } + return logger + } + + logger, err := zap.NewProduction() + if err != nil { + panic(err) // Handle error according to your application's error policy + } + return logger +} diff --git a/internal/logger/sensitive_values.go.fixme b/internal/logger/sensitive_values.go.fixme new file mode 100644 index 0000000..a8493cd --- /dev/null +++ b/internal/logger/sensitive_values.go.fixme @@ -0,0 +1,59 @@ +package logger + +import ( + "os" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// RedactSensitiveHeadersEncoder wraps a base encoder and redacts specified headers. +type RedactSensitiveHeadersEncoder struct { + zapcore.Encoder +} + +// RedactHeaders specifies the headers to be redacted. +var RedactHeaders = map[string]struct{}{ + "Authorization": {}, + // Specify other headers to redact as needed. +} + +// EncodeEntry redacts sensitive headers from log entries. This method implements the zapcore.Encoder interface. +func (e *RedactSensitiveHeadersEncoder) EncodeEntry(entry zapcore.Entry, fields []zapcore.Field) (*zapcore.Buffer, error) { + for i, field := range fields { + if field.Key == "headers" { + if headers, ok := field.Interface.(map[string][]string); ok { + redactedHeaders := make(map[string][]string, len(headers)) + for k, v := range headers { + if _, found := RedactHeaders[k]; found { + redactedHeaders[k] = []string{"[REDACTED]"} + } else { + redactedHeaders[k] = v + } + } + fields[i] = zap.Any(field.Key, redactedHeaders) + } + } + } + // Delegate the encoding to the base encoder + return e.Encoder.EncodeEntry(entry, fields) +} + +// NewRedactSensitiveHeadersEncoder creates a new RedactSensitiveHeadersEncoder wrapping a base encoder. +func NewRedactSensitiveHeadersEncoder(encoder zapcore.Encoder) *RedactSensitiveHeadersEncoder { + return &RedactSensitiveHeadersEncoder{Encoder: encoder} +} + +// NewLoggerWithSensitiveHeadersRedaction creates a new zap.Logger that redacts sensitive headers. +func NewLoggerWithSensitiveHeadersRedaction() *zap.Logger { + config := zap.NewProductionEncoderConfig() + config.EncodeTime = zapcore.ISO8601TimeEncoder + + baseEncoder := zapcore.NewJSONEncoder(config) + sensitiveEncoder := NewRedactSensitiveHeadersEncoder(baseEncoder) + + core := zapcore.NewCore(sensitiveEncoder, zapcore.AddSync(os.Stdout), zap.DebugLevel) + logger := zap.New(core) + + return logger +} From 7c93a66fb4add92b89d9f5b5ae9e829e3dfdedd7 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 06:27:28 +0000 Subject: [PATCH 04/16] Update logger variable name in http_error_handling.go --- internal/httpclient/http_error_handling.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/httpclient/http_error_handling.go b/internal/httpclient/http_error_handling.go index 5e4adae..cb7bb3a 100644 --- a/internal/httpclient/http_error_handling.go +++ b/internal/httpclient/http_error_handling.go @@ -30,7 +30,7 @@ func (c *Client) HandleAPIError(resp *http.Response) error { err := json.NewDecoder(resp.Body).Decode(&structuredErr) if err == nil && structuredErr.Error.Message != "" { // Using structured logging to log the structured error details - c.logger.Warn("API returned structured error", + c.Logger.Warn("API returned structured error", zap.String("status", resp.Status), zap.String("error_code", structuredErr.Error.Code), zap.String("error_message", structuredErr.Error.Message), @@ -46,13 +46,13 @@ func (c *Client) HandleAPIError(resp *http.Response) error { if err != nil || errMsg == "" { errMsg = fmt.Sprintf("Unexpected error with status code: %d", resp.StatusCode) // Logging with structured fields - c.logger.Warn("Failed to decode API error message, using default error message", + c.Logger.Warn("Failed to decode API error message, using default error message", zap.String("status", resp.Status), zap.String("error_message", errMsg), ) } else { // Logging non-structured error as a warning with structured fields - c.logger.Warn("API returned non-structured error", + c.Logger.Warn("API returned non-structured error", zap.String("status", resp.Status), zap.String("error_message", errMsg), ) From d01e30bc26ff86d95e899b1f7e2170cb83187015 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 08:07:42 +0000 Subject: [PATCH 05/16] Add httpclient package and update logger initialization --- .../jamfpro/jamfpro_api_handler.go | 5 +- internal/httpclient/http_client.go | 4 +- internal/logger/logger.go | 52 ++----------- internal/logger/loggerconfig.go | 73 +++++++++++++++++++ 4 files changed, 86 insertions(+), 48 deletions(-) create mode 100644 internal/logger/loggerconfig.go diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 1b30c70..19a7043 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -49,6 +49,7 @@ import ( _ "embed" + "github.com/deploymenttheory/go-api-http-client/internal/httpclient" "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -256,8 +257,8 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, // If content type is HTML, extract the error message if strings.Contains(contentType, "text/html") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - log.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode) - return &APIError{ + log.Warn("Received HTML content", zap.String("error_message", errMsg), zap.Int("status_code", resp.StatusCode)) + return &httpclient.APIError{ StatusCode: resp.StatusCode, Message: errMsg, } diff --git a/internal/httpclient/http_client.go b/internal/httpclient/http_client.go index b498ba3..fb1a92b 100644 --- a/internal/httpclient/http_client.go +++ b/internal/httpclient/http_client.go @@ -73,8 +73,8 @@ type Client struct { // BuildClient creates a new HTTP client with the provided configuration. func BuildClient(config Config) (*Client, error) { - // Initialize the logger using the NewDefaultLogger function from the logger package. - log := logger.NewDefaultLogger() + // Initialize the zap logger. + log := logger.BuildLogger(config.LogLevel) // Set the logger's level based on the provided configuration. log.SetLevel(config.LogLevel) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 2bbe906..bfdf7d7 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -15,20 +15,19 @@ type LogLevel int const ( // LogLevelDebug is for messages that are useful during software debugging. - LogLevelDebug LogLevel = iota - 1 // Adjusted value to -1 to follow Zap's convention for DEBUG. + LogLevelDebug LogLevel = -1 // Zap's DEBUG level // LogLevelInfo is for informational messages, indicating normal operation. - LogLevelInfo + LogLevelInfo LogLevel = 0 // Zap's INFO level // LogLevelWarn is for messages that highlight potential issues in the system. - LogLevelWarn + LogLevelWarn LogLevel = 1 // Zap's WARN level // LogLevelError is for messages that highlight errors in the application's execution. - LogLevelError - // LogLevelDPanic is for severe error conditions that are actionable in development. It panics in development and logs as an error in production. - LogLevelDPanic + LogLevelError LogLevel = 2 // Zap's ERROR level + // LogLevelDPanic is for severe error conditions that are actionable in development. + LogLevelDPanic LogLevel = 3 // Zap's DPANIC level // LogLevelPanic is for severe error conditions that should cause the program to panic. - LogLevelPanic + LogLevelPanic LogLevel = 4 // Zap's PANIC level // LogLevelFatal is for errors that require immediate program termination. - LogLevelFatal - // LogLevelNone is used to disable logging. + LogLevelFatal LogLevel = 5 // Zap's FATAL level LogLevelNone ) @@ -53,41 +52,6 @@ type defaultLogger struct { logLevel LogLevel // logLevel determines the current logging level (e.g., DEBUG, INFO, WARN). } -// NewDefaultLogger creates and returns a new logger instance with a default production configuration. -// It uses JSON formatting for log messages and sets the initial log level to Info. If the logger cannot -// be initialized, the function panics to indicate a critical setup failure. -func NewDefaultLogger() Logger { - // Set up custom encoder configuration - encoderCfg := zap.NewProductionEncoderConfig() - encoderCfg.TimeKey = "timestamp" - encoderCfg.EncodeTime = zapcore.ISO8601TimeEncoder // Use ISO8601 format for timestamps - - // Define the logger configuration - config := zap.Config{ - Level: zap.NewAtomicLevelAt(zap.InfoLevel), // Default log level is Info - Development: false, // Set to true if the logger is used in a development environment - Encoding: "json", // Use JSON format for structured logging - EncoderConfig: encoderCfg, - OutputPaths: []string{"stdout"}, // Log to standard output - ErrorOutputPaths: []string{"stderr"}, // Log internal Zap errors to standard error - InitialFields: map[string]interface{}{ - "application": "your-application-name", // Customize this field to suit your needs - }, - } - - // Build the logger from the configuration - logger, err := config.Build() - if err != nil { - panic(err) // Panic if there is an error initializing the logger - } - - // Wrap the Zap logger in your defaultLogger struct, which implements your Logger interface - return &defaultLogger{ - logger: logger, - logLevel: LogLevelInfo, // Assuming LogLevelInfo maps to zap.InfoLevel - } -} - // SetLevel updates the logging level of the logger. It controls the verbosity of the logs, // allowing the option to filter out less severe messages based on the specified level. func (d *defaultLogger) SetLevel(level LogLevel) { diff --git a/internal/logger/loggerconfig.go b/internal/logger/loggerconfig.go new file mode 100644 index 0000000..52b3465 --- /dev/null +++ b/internal/logger/loggerconfig.go @@ -0,0 +1,73 @@ +// logger.go +package logger + +// Ref: https://betterstack.com/community/guides/logging/go/zap/#logging-errors-with-zap + +import ( + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// BuildLogger creates and returns a new logger instance with a default production configuration. +// It uses JSON formatting for log messages and sets the initial log level to Info. If the logger cannot +// be initialized, the function panics to indicate a critical setup failure. +func BuildLogger(logLevel LogLevel) Logger { + // Set up custom encoder configuration + encoderCfg := zap.NewProductionEncoderConfig() + encoderCfg.TimeKey = "timestamp" + encoderCfg.EncodeTime = zapcore.ISO8601TimeEncoder // Use ISO8601 format for timestamps + + // Convert the custom LogLevel to zap's logging level + zapLogLevel := convertToZapLevel(logLevel) + + // Define the logger configuration + config := zap.Config{ + Level: zap.NewAtomicLevelAt(zapLogLevel), // Default log level is Info + Development: false, // Set to true if the logger is used in a development environment + Encoding: "json", // Use JSON format for structured logging + DisableCaller: false, + DisableStacktrace: false, + Sampling: nil, + EncoderConfig: encoderCfg, + OutputPaths: []string{ + "stdout", // Log info and above to standard output + }, + ErrorOutputPaths: []string{ + "stderr", // Log internal Zap errors to standard error + }, + InitialFields: map[string]interface{}{ + "application": "your-application-name", // Customize this field to suit your needs + }, + } + + // Build the logger from the configuration + logger := zap.Must(config.Build()) + + // Wrap the Zap logger in your defaultLogger struct, which implements your Logger interface + return &defaultLogger{ + logger: logger, + logLevel: LogLevelInfo, // Assuming LogLevelInfo maps to zap.InfoLevel + } +} + +// convertToZapLevel converts the custom LogLevel to a zapcore.Level +func convertToZapLevel(level LogLevel) zapcore.Level { + switch level { + case LogLevelDebug: + return zap.DebugLevel + case LogLevelInfo: + return zap.InfoLevel + case LogLevelWarn: + return zap.WarnLevel + case LogLevelError: + return zap.ErrorLevel + case LogLevelDPanic: + return zap.DPanicLevel + case LogLevelPanic: + return zap.PanicLevel + case LogLevelFatal: + return zap.FatalLevel + default: + return zap.InfoLevel // Default to InfoLevel + } +} From 1e76540950202d4133852d68da6d0c5860a67832 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 08:22:07 +0000 Subject: [PATCH 06/16] Add os package import and include pid in initial fields --- internal/logger/loggerconfig.go | 14 +++++++++----- internal/version/version.go | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 internal/version/version.go diff --git a/internal/logger/loggerconfig.go b/internal/logger/loggerconfig.go index 52b3465..5b41418 100644 --- a/internal/logger/loggerconfig.go +++ b/internal/logger/loggerconfig.go @@ -4,6 +4,8 @@ package logger // Ref: https://betterstack.com/community/guides/logging/go/zap/#logging-errors-with-zap import ( + "os" + "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -12,6 +14,7 @@ import ( // It uses JSON formatting for log messages and sets the initial log level to Info. If the logger cannot // be initialized, the function panics to indicate a critical setup failure. func BuildLogger(logLevel LogLevel) Logger { + // Set up custom encoder configuration encoderCfg := zap.NewProductionEncoderConfig() encoderCfg.TimeKey = "timestamp" @@ -32,21 +35,22 @@ func BuildLogger(logLevel LogLevel) Logger { OutputPaths: []string{ "stdout", // Log info and above to standard output }, - ErrorOutputPaths: []string{ + ErrorOutputPaths: []string{ // is similar to OutputPaths but is used for Zap's internal errors only, not those generated or logged by your application (such as the error from mismatched loosely-typed key/value pairs). "stderr", // Log internal Zap errors to standard error }, - InitialFields: map[string]interface{}{ - "application": "your-application-name", // Customize this field to suit your needs + InitialFields: map[string]interface{}{ // specifies global contextual fields that should be included in every log entry produced by each logger created from the Config object + "pid": os.Getpid(), + "application": "your-application-name", }, } // Build the logger from the configuration logger := zap.Must(config.Build()) - // Wrap the Zap logger in your defaultLogger struct, which implements your Logger interface + // Wrap the Zap logger in your defaultLogger struct, which implements the Logger interface return &defaultLogger{ logger: logger, - logLevel: LogLevelInfo, // Assuming LogLevelInfo maps to zap.InfoLevel + logLevel: logLevel, } } diff --git a/internal/version/version.go b/internal/version/version.go new file mode 100644 index 0000000..51b26a8 --- /dev/null +++ b/internal/version/version.go @@ -0,0 +1,18 @@ +// version.go +package version + +// AppName holds the name of the application +var AppName = "go-api-http-client" + +// Version holds the current version of the application +var Version = "1.0.0" + +// GetAppName returns the name of the application +func GetAppName() string { + return AppName +} + +// GetVersion returns the current version of the application +func GetVersion() string { + return Version +} From 25ac658bcab3922e5148aa96464412ae51edb60c Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 08:23:23 +0000 Subject: [PATCH 07/16] Add application name from version package to logger config --- internal/logger/loggerconfig.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/logger/loggerconfig.go b/internal/logger/loggerconfig.go index 5b41418..78af744 100644 --- a/internal/logger/loggerconfig.go +++ b/internal/logger/loggerconfig.go @@ -6,6 +6,7 @@ package logger import ( "os" + "github.com/deploymenttheory/go-api-http-client/internal/version" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -40,7 +41,7 @@ func BuildLogger(logLevel LogLevel) Logger { }, InitialFields: map[string]interface{}{ // specifies global contextual fields that should be included in every log entry produced by each logger created from the Config object "pid": os.Getpid(), - "application": "your-application-name", + "application": version.GetAppName(), }, } From f64f9f518eac0220edadc512ff0747e482ea9c50 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 08:56:17 +0000 Subject: [PATCH 08/16] Refactor HTTP client and API handler*** ***Adjust concurrency management based on metrics --- .../jamfpro/jamfpro_api_handler.go | 3 +- .../http_error_handling.go | 4 +- internal/httpclient/http_client.go | 2 +- .../http_client_auth_token_management.go | 78 ++++--------------- .../httpclient/http_concurrency_management.go | 10 +-- internal/httpclient/http_headers.go | 6 +- 6 files changed, 26 insertions(+), 77 deletions(-) rename internal/{httpclient => errors}/http_error_handling.go (98%) diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 19a7043..43af971 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -49,7 +49,6 @@ import ( _ "embed" - "github.com/deploymenttheory/go-api-http-client/internal/httpclient" "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -258,7 +257,7 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, if strings.Contains(contentType, "text/html") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) log.Warn("Received HTML content", zap.String("error_message", errMsg), zap.Int("status_code", resp.StatusCode)) - return &httpclient.APIError{ + return &APIError{ StatusCode: resp.StatusCode, Message: errMsg, } diff --git a/internal/httpclient/http_error_handling.go b/internal/errors/http_error_handling.go similarity index 98% rename from internal/httpclient/http_error_handling.go rename to internal/errors/http_error_handling.go index cb7bb3a..58baf7e 100644 --- a/internal/httpclient/http_error_handling.go +++ b/internal/errors/http_error_handling.go @@ -1,6 +1,6 @@ // http_error_handling.go // This package provides utility functions and structures for handling and categorizing HTTP error responses. -package httpclient +package errors import ( "encoding/json" @@ -25,7 +25,7 @@ type StructuredError struct { } // HandleAPIError handles error responses from the API, converting them into a structured error if possible. -func (c *Client) HandleAPIError(resp *http.Response) error { +func HandleAPIError(logger Logger, resp *http.Response) error { var structuredErr StructuredError err := json.NewDecoder(resp.Body).Decode(&structuredErr) if err == nil && structuredErr.Error.Message != "" { diff --git a/internal/httpclient/http_client.go b/internal/httpclient/http_client.go index fb1a92b..d8237fe 100644 --- a/internal/httpclient/http_client.go +++ b/internal/httpclient/http_client.go @@ -157,7 +157,7 @@ func BuildClient(config Config) (*Client, error) { } // Authenticate and check token validity. - _, err = client.ValidAuthTokenCheck() + _, err = client.ValidAuthTokenCheck(client.Logger) if err != nil { return nil, log.Error("Failed to validate or obtain auth token", zap.Error(err)) } diff --git a/internal/httpclient/http_client_auth_token_management.go b/internal/httpclient/http_client_auth_token_management.go index 87fd0d0..5dfe52d 100644 --- a/internal/httpclient/http_client_auth_token_management.go +++ b/internal/httpclient/http_client_auth_token_management.go @@ -2,9 +2,9 @@ package httpclient import ( - "fmt" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -14,72 +14,23 @@ type TokenResponse struct { Expires time.Time `json:"expires"` } -/* // ValidAuthTokenCheck checks if the current token is valid and not close to expiry. // If the token is invalid, it tries to refresh it. // It returns a boolean indicating the validity of the token and an error if there's a failure. -func (c *Client) ValidAuthTokenCheck() (bool, error) { - +func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { if c.Token == "" { + c.Logger.Debug("No token found, attempting to obtain a new one") if c.AuthMethod == "bearer" { err := c.ObtainToken() if err != nil { - return false, fmt.Errorf("failed to obtain bearer token: %w", err) + return false, c.Logger.Error("Failed to obtain bearer token", zap.Error(err)) } } else if c.AuthMethod == "oauth" { - err := c.ObtainOAuthToken(c.config.Auth) - if err != nil { - return false, fmt.Errorf("failed to obtain OAuth token: %w", err) - } - } else { - return false, fmt.Errorf("no valid credentials provided. Unable to obtain a token") - } - } - - // If token exists and is close to expiry or already expired - if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { - var err error - if c.BearerTokenAuthCredentials.Username != "" && c.BearerTokenAuthCredentials.Password != "" { - err = c.RefreshToken() - } else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" { - err = c.ObtainOAuthToken(c.config.Auth) - } else { - return false, fmt.Errorf("unknown auth method: %s", c.AuthMethod) - } - - if err != nil { - return false, fmt.Errorf("failed to refresh token: %w", err) - } - } - - if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { - return false, fmt.Errorf("token lifetime setting less than buffer. Buffer setting: %v, Time (seconds) until Exp: %v", c.config.TokenRefreshBufferPeriod, time.Until(c.Expiry)) - } - return true, nil -} -*/ - -// ValidAuthTokenCheck checks if the current token is valid and not close to expiry. -// If the token is invalid, it tries to refresh it. -// It returns a boolean indicating the validity of the token and an error if there's a failure. -func (c *Client) ValidAuthTokenCheck() (bool, error) { - if c.Token == "" { - if c.AuthMethod == "bearer" { - err := c.ObtainToken() - if err != nil { - c.logger.Error("Failed to obtain bearer token", zap.Error(err)) - return false, fmt.Errorf("failed to obtain bearer token: %w", err) - } - } else if c.AuthMethod == "oauth" { - err := c.ObtainOAuthToken(c.config.Auth) - if err != nil { - c.logger.Error("Failed to obtain OAuth token", zap.Error(err)) - return false, fmt.Errorf("failed to obtain OAuth token: %w", err) + if err := c.ObtainOAuthToken(c.config.Auth); err != nil { + return false, c.Logger.Error("Failed to obtain OAuth token", zap.Error(err)) } } else { - err := fmt.Errorf("no valid credentials provided. Unable to obtain a token") - c.logger.Error("No valid credentials provided", zap.Error(err)) - return false, err + return false, c.Logger.Error("No valid credentials provided. Unable to obtain a token", zap.String("authMethod", c.AuthMethod)) } } @@ -90,21 +41,20 @@ func (c *Client) ValidAuthTokenCheck() (bool, error) { } else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" { err = c.ObtainOAuthToken(c.config.Auth) } else { - err = fmt.Errorf("unknown auth method: %s", c.AuthMethod) - c.logger.Error("Unknown auth method", zap.String("auth_method", c.AuthMethod), zap.Error(err)) - return false, err + return false, c.Logger.Error("Unknown auth method", zap.String("authMethod", c.AuthMethod)) } if err != nil { - c.logger.Error("Failed to refresh token", zap.Error(err)) - return false, fmt.Errorf("failed to refresh token: %w", err) + return false, c.Logger.Error("Failed to refresh token", zap.Error(err)) } } if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { - err := fmt.Errorf("token lifetime setting less than buffer. Buffer setting: %v, Time (seconds) until Exp: %v", c.config.TokenRefreshBufferPeriod, time.Until(c.Expiry)) - c.logger.Error("Token lifetime less than buffer", zap.Duration("buffer_period", c.config.TokenRefreshBufferPeriod), zap.Duration("time_until_expiry", time.Until(c.Expiry)), zap.Error(err)) - return false, err + return false, c.Logger.Error( + "Token lifetime setting less than buffer", + zap.Duration("buffer_period", c.config.TokenRefreshBufferPeriod), + zap.Duration("time_until_expiry", time.Until(c.Expiry)), + ) } return true, nil diff --git a/internal/httpclient/http_concurrency_management.go b/internal/httpclient/http_concurrency_management.go index 299a2ab..3fa9363 100644 --- a/internal/httpclient/http_concurrency_management.go +++ b/internal/httpclient/http_concurrency_management.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "github.com/google/uuid" "go.uber.org/zap" ) @@ -23,7 +24,7 @@ const ( // ConcurrencyManager controls the number of concurrent HTTP requests. type ConcurrencyManager struct { sem chan struct{} - logger Logger + logger logger.Logger debugMode bool AcquisitionTimes []time.Duration lock sync.Mutex @@ -37,10 +38,7 @@ type requestIDKey struct{} // NewConcurrencyManager initializes a new ConcurrencyManager with the given concurrency limit, logger, and debug mode. // The ConcurrencyManager ensures no more than a certain number of concurrent requests are made. // It uses a semaphore to control concurrency. -func NewConcurrencyManager(limit int, logger Logger, debugMode bool) *ConcurrencyManager { - if logger == nil { - logger = &defaultLogger{} // Assuming this is the default logger implementation - } +func NewConcurrencyManager(limit int, logger logger.Logger, debugMode bool) *ConcurrencyManager { return &ConcurrencyManager{ sem: make(chan struct{}, limit), logger: logger, @@ -209,7 +207,7 @@ func (c *Client) AdjustConcurrencyBasedOnMetrics() { if newLimit != currentLimit { c.ConcurrencyMgr.AdjustConcurrencyLimit(newLimit) - c.logger.Debug("Adjusted concurrency", + c.Logger.Debug("Adjusted concurrency", zap.Int("OldLimit", currentLimit), zap.Int("NewLimit", newLimit), zap.String("Reason", "Based on average acquisition time"), diff --git a/internal/httpclient/http_headers.go b/internal/httpclient/http_headers.go index 8cc8ec9..555b284 100644 --- a/internal/httpclient/http_headers.go +++ b/internal/httpclient/http_headers.go @@ -2,10 +2,12 @@ package httpclient import ( "net/http" + + "github.com/deploymenttheory/go-api-http-client/internal/logger" ) // LogHTTPHeaders logs the HTTP headers of an HTTP request or response, with an option to hide sensitive information like the token in secure mode. -func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) { +func LogHTTPHeaders(log logger.Logger, headers http.Header, secureMode bool) { var keysAndValues []interface{} if secureMode { for key, values := range headers { @@ -23,6 +25,6 @@ func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) { // Log the headers using the logger from the httpclient package if len(keysAndValues) > 0 { - logger.Debug("HTTP Headers", keysAndValues...) + log.Debug("HTTP Headers", keysAndValues...) } } From da2c1dbca5c22232ccaac40a9003b6f169d46717 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:04:17 +0000 Subject: [PATCH 09/16] Remove unused HTTP methods --- .../http_client_bearer_token_auth.go | 83 +++++-------------- internal/httpclient/http_methods.go | 70 ---------------- 2 files changed, 21 insertions(+), 132 deletions(-) delete mode 100644 internal/httpclient/http_methods.go diff --git a/internal/httpclient/http_client_bearer_token_auth.go b/internal/httpclient/http_client_bearer_token_auth.go index 39dd2a0..c8dcb69 100644 --- a/internal/httpclient/http_client_bearer_token_auth.go +++ b/internal/httpclient/http_client_bearer_token_auth.go @@ -9,6 +9,7 @@ import ( "net/http" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -25,36 +26,35 @@ func (c *Client) SetBearerTokenAuthCredentials(credentials BearerTokenAuthCreden c.BearerTokenAuthCredentials = credentials } -/* // ObtainToken fetches and sets an authentication token using the stored basic authentication credentials. -func (c *Client) ObtainToken() error { +func (c *Client) ObtainToken(log logger.Logger) error { authenticationEndpoint := c.ConstructAPIAuthEndpoint(BearerTokenEndpoint) - c.logger.Debug("Attempting to obtain token for user", "Username", c.BearerTokenAuthCredentials.Username) + log.Debug("Attempting to obtain token for user", zap.String("Username", c.BearerTokenAuthCredentials.Username)) req, err := http.NewRequest("POST", authenticationEndpoint, nil) if err != nil { - c.logger.Error("Failed to create new request for token", "Error", err) + log.Error("Failed to create new request for token", zap.Error(err)) return err } req.SetBasicAuth(c.BearerTokenAuthCredentials.Username, c.BearerTokenAuthCredentials.Password) resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to make request for token", "Error", err) + log.Error("Failed to make request for token", zap.Error(err)) return err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - c.logger.Warn("Received non-OK response while obtaining token", "StatusCode", resp.StatusCode) + log.Warn("Received non-OK response while obtaining token", zap.Int("StatusCode", resp.StatusCode)) return c.HandleAPIError(resp) } tokenResp := &TokenResponse{} err = json.NewDecoder(resp.Body).Decode(tokenResp) if err != nil { - c.logger.Error("Failed to decode token response", "Error", err) + log.Error("Failed to decode token response", zap.Error(err)) return err } @@ -62,48 +62,7 @@ func (c *Client) ObtainToken() error { c.Expiry = tokenResp.Expires tokenDuration := time.Until(c.Expiry) - c.logger.Info("Token obtained successfully", "Expiry", c.Expiry, "Duration", tokenDuration) - - return nil -}*/ - -// ObtainToken fetches and sets an authentication token using the stored basic authentication credentials. -func (c *Client) ObtainToken() error { - authenticationEndpoint := c.ConstructAPIAuthEndpoint(BearerTokenEndpoint) - - c.logger.Debug("Attempting to obtain token for user", zap.String("Username", c.BearerTokenAuthCredentials.Username)) - - req, err := http.NewRequest("POST", authenticationEndpoint, nil) - if err != nil { - c.logger.Error("Failed to create new request for token", zap.Error(err)) - return err - } - req.SetBasicAuth(c.BearerTokenAuthCredentials.Username, c.BearerTokenAuthCredentials.Password) - - resp, err := c.httpClient.Do(req) - if err != nil { - c.logger.Error("Failed to make request for token", zap.Error(err)) - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - c.logger.Warn("Received non-OK response while obtaining token", zap.Int("StatusCode", resp.StatusCode)) - return c.HandleAPIError(resp) - } - - tokenResp := &TokenResponse{} - err = json.NewDecoder(resp.Body).Decode(tokenResp) - if err != nil { - c.logger.Error("Failed to decode token response", zap.Error(err)) - return err - } - - c.Token = tokenResp.Token - c.Expiry = tokenResp.Expires - tokenDuration := time.Until(c.Expiry) - - c.logger.Info("Token obtained successfully", zap.Time("Expiry", c.Expiry), zap.Duration("Duration", tokenDuration)) + log.Info("Token obtained successfully", zap.Time("Expiry", c.Expiry), zap.Duration("Duration", tokenDuration)) return nil } @@ -118,33 +77,33 @@ func (c *Client) RefreshToken() error { req, err := http.NewRequest("POST", tokenRefreshEndpoint, nil) if err != nil { - c.logger.Error("Failed to create new request for token refresh", "error", err) + log.Error("Failed to create new request for token refresh", "error", err) return err } req.Header.Add("Authorization", "Bearer "+c.Token) - c.logger.Debug("Attempting to refresh token", "URL", tokenRefreshEndpoint) + log.Debug("Attempting to refresh token", "URL", tokenRefreshEndpoint) resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to make request for token refresh", "error", err) + log.Error("Failed to make request for token refresh", "error", err) return err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - c.logger.Warn("Token refresh response status is not OK", "StatusCode", resp.StatusCode) + log.Warn("Token refresh response status is not OK", "StatusCode", resp.StatusCode) return c.HandleAPIError(resp) } tokenResp := &TokenResponse{} err = json.NewDecoder(resp.Body).Decode(tokenResp) if err != nil { - c.logger.Error("Failed to decode token response", "error", err) + log.Error("Failed to decode token response", "error", err) return err } - c.logger.Info("Token refreshed successfully", "Expiry", tokenResp.Expires) + log.Info("Token refreshed successfully", "Expiry", tokenResp.Expires) c.Token = tokenResp.Token c.Expiry = tokenResp.Expires @@ -152,7 +111,7 @@ func (c *Client) RefreshToken() error { } */ // RefreshToken refreshes the current authentication token. -func (c *Client) RefreshToken() error { +func (c *Client) RefreshToken(log logger.Logger) error { c.tokenLock.Lock() defer c.tokenLock.Unlock() @@ -160,35 +119,35 @@ func (c *Client) RefreshToken() error { req, err := http.NewRequest("POST", tokenRefreshEndpoint, nil) if err != nil { - c.logger.Error("Failed to create new request for token refresh", zap.Error(err)) + log.Error("Failed to create new request for token refresh", zap.Error(err)) return err } req.Header.Add("Authorization", "Bearer "+c.Token) - c.logger.Debug("Attempting to refresh token", zap.String("URL", tokenRefreshEndpoint)) + log.Debug("Attempting to refresh token", zap.String("URL", tokenRefreshEndpoint)) resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to make request for token refresh", zap.Error(err)) + log.Error("Failed to make request for token refresh", zap.Error(err)) return err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - c.logger.Warn("Token refresh response status is not OK", zap.Int("StatusCode", resp.StatusCode)) + log.Warn("Token refresh response status is not OK", zap.Int("StatusCode", resp.StatusCode)) return c.HandleAPIError(resp) } tokenResp := &TokenResponse{} err = json.NewDecoder(resp.Body).Decode(tokenResp) if err != nil { - c.logger.Error("Failed to decode token response", zap.Error(err)) + log.Error("Failed to decode token response", zap.Error(err)) return err } c.Token = tokenResp.Token c.Expiry = tokenResp.Expires - c.logger.Info("Token refreshed successfully", zap.Time("Expiry", tokenResp.Expires)) + log.Info("Token refreshed successfully", zap.Time("Expiry", tokenResp.Expires)) return nil } diff --git a/internal/httpclient/http_methods.go b/internal/httpclient/http_methods.go deleted file mode 100644 index 9c632c4..0000000 --- a/internal/httpclient/http_methods.go +++ /dev/null @@ -1,70 +0,0 @@ -// http_methods.go -package httpclient - -import "net/http" - -// Get sends a GET request to the specified endpoint and unmarshals the response into 'out'. -// The caller is responsible for closing the response body. -func (c *Client) Get(endpoint string, out interface{}) (*http.Response, error) { - c.logger.Info("Sending GET request", "endpoint", endpoint) - - resp, err := c.DoRequest(http.MethodGet, endpoint, nil, out) - if err != nil { - c.logger.Error("GET request failed", "endpoint", endpoint, "error", err) - return nil, err - } - return resp, nil -} - -// Post sends a POST request to the specified endpoint with the provided body and unmarshals the response into 'out'. -// The caller is responsible for closing the response body. -func (c *Client) Post(endpoint string, body, out interface{}) (*http.Response, error) { - c.logger.Info("Sending POST request", "endpoint", endpoint, "body", body) - - resp, err := c.DoRequest(http.MethodPost, endpoint, body, out) - if err != nil { - c.logger.Error("POST request failed", "endpoint", endpoint, "error", err) - return nil, err - } - return resp, nil -} - -// Put sends a PUT request to the specified endpoint with the provided body and unmarshals the response into 'out'. -// The caller is responsible for closing the response body. -func (c *Client) Put(endpoint string, body, out interface{}) (*http.Response, error) { - - c.logger.Debug("Sending PUT request", "endpoint", endpoint, "body", body) - - resp, err := c.DoRequest(http.MethodPut, endpoint, body, out) - if err != nil { - c.logger.Error("PUT request failed", "endpoint", endpoint, "error", err) - return nil, err - } - return resp, nil -} - -// Delete sends a DELETE request to the specified endpoint and unmarshals the response into 'out'. -// The caller is responsible for closing the response body. -func (c *Client) Delete(endpoint string, out interface{}) (*http.Response, error) { - c.logger.Debug("Sending DELETE request", "endpoint", endpoint) - - resp, err := c.DoRequest(http.MethodDelete, endpoint, nil, out) - if err != nil { - c.logger.Error("DELETE request failed", "endpoint", endpoint, "error", err) - return nil, err - } - return resp, nil -} - -// Patch sends a PATCH request to the specified endpoint with the provided body and unmarshals the response into 'out'. -// The caller is responsible for closing the response body. -func (c *Client) Patch(endpoint string, body, out interface{}) (*http.Response, error) { - c.logger.Debug("Sending PATCH request", "endpoint", endpoint, "body", body) - - resp, err := c.DoRequest(http.MethodPatch, endpoint, body, out) - if err != nil { - c.logger.Error("PATCH request failed", "endpoint", endpoint, "error", err) - return nil, err - } - return resp, nil -} From 83aa4caf3794b63188dc89cae0f670ae0366f95a Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:11:59 +0000 Subject: [PATCH 10/16] Refactor authentication token management and logging --- .../http_client_auth_token_management.go | 8 ++--- .../http_client_bearer_token_auth.go | 2 +- .../httpclient/http_client_defaultconfig.go | 4 ++- internal/httpclient/http_client_oauth.go | 31 ++++++++++--------- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/internal/httpclient/http_client_auth_token_management.go b/internal/httpclient/http_client_auth_token_management.go index 5dfe52d..e3b0dfe 100644 --- a/internal/httpclient/http_client_auth_token_management.go +++ b/internal/httpclient/http_client_auth_token_management.go @@ -21,12 +21,12 @@ func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { if c.Token == "" { c.Logger.Debug("No token found, attempting to obtain a new one") if c.AuthMethod == "bearer" { - err := c.ObtainToken() + err := c.ObtainToken(log) if err != nil { return false, c.Logger.Error("Failed to obtain bearer token", zap.Error(err)) } } else if c.AuthMethod == "oauth" { - if err := c.ObtainOAuthToken(c.config.Auth); err != nil { + if err := c.ObtainOAuthToken(c.config.Auth, log); err != nil { return false, c.Logger.Error("Failed to obtain OAuth token", zap.Error(err)) } } else { @@ -37,9 +37,9 @@ func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { var err error if c.BearerTokenAuthCredentials.Username != "" && c.BearerTokenAuthCredentials.Password != "" { - err = c.RefreshToken() + err = c.RefreshToken(log) } else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" { - err = c.ObtainOAuthToken(c.config.Auth) + err = c.ObtainOAuthToken(c.config.Auth, log) } else { return false, c.Logger.Error("Unknown auth method", zap.String("authMethod", c.AuthMethod)) } diff --git a/internal/httpclient/http_client_bearer_token_auth.go b/internal/httpclient/http_client_bearer_token_auth.go index c8dcb69..cd98f7e 100644 --- a/internal/httpclient/http_client_bearer_token_auth.go +++ b/internal/httpclient/http_client_bearer_token_auth.go @@ -28,7 +28,7 @@ func (c *Client) SetBearerTokenAuthCredentials(credentials BearerTokenAuthCreden // ObtainToken fetches and sets an authentication token using the stored basic authentication credentials. func (c *Client) ObtainToken(log logger.Logger) error { - authenticationEndpoint := c.ConstructAPIAuthEndpoint(BearerTokenEndpoint) + authenticationEndpoint := APIHandler.ConstructAPIAuthEndpoint(BearerTokenEndpoint) log.Debug("Attempting to obtain token for user", zap.String("Username", c.BearerTokenAuthCredentials.Username)) diff --git a/internal/httpclient/http_client_defaultconfig.go b/internal/httpclient/http_client_defaultconfig.go index 54e699d..1585279 100644 --- a/internal/httpclient/http_client_defaultconfig.go +++ b/internal/httpclient/http_client_defaultconfig.go @@ -2,10 +2,12 @@ package httpclient import ( "time" + + "github.com/deploymenttheory/go-api-http-client/internal/logger" ) const ( - DefaultLogLevel = LogLevelInfo + DefaultLogLevel = logger.LogLevelInfo DefaultMaxRetryAttempts = 3 DefaultEnableDynamicRateLimiting = true DefaultMaxConcurrentRequests = 5 diff --git a/internal/httpclient/http_client_oauth.go b/internal/httpclient/http_client_oauth.go index 24eb3fd..8301d04 100644 --- a/internal/httpclient/http_client_oauth.go +++ b/internal/httpclient/http_client_oauth.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -41,32 +42,32 @@ func (c *Client) SetOAuthCredentials(credentials OAuthCredentials) { // ObtainOAuthToken fetches an OAuth access token using the provided OAuthCredentials (Client ID and Client Secret). // It updates the client's Token and Expiry fields with the obtained values. -func (c *Client) ObtainOAuthToken(credentials AuthConfig) error { +func (c *Client) ObtainOAuthToken(credentials AuthConfig, log logger.Logger) error { authenticationEndpoint := c.ConstructAPIAuthEndpoint(OAuthTokenEndpoint) data := url.Values{} data.Set("client_id", credentials.ClientID) data.Set("client_secret", credentials.ClientSecret) data.Set("grant_type", "client_credentials") - c.logger.Debug("Attempting to obtain OAuth token", zap.String("ClientID", credentials.ClientID)) + log.Debug("Attempting to obtain OAuth token", zap.String("ClientID", credentials.ClientID)) req, err := http.NewRequest("POST", authenticationEndpoint, strings.NewReader(data.Encode())) if err != nil { - c.logger.Error("Failed to create request for OAuth token", zap.Error(err)) + log.Error("Failed to create request for OAuth token", zap.Error(err)) return err } req.Header.Add("Content-Type", "application/x-www-form-urlencoded") resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to execute request for OAuth token", zap.Error(err)) + log.Error("Failed to execute request for OAuth token", zap.Error(err)) return err } defer resp.Body.Close() bodyBytes, err := io.ReadAll(resp.Body) if err != nil { - c.logger.Error("Failed to read response body", zap.Error(err)) + log.Error("Failed to read response body", zap.Error(err)) return err } @@ -76,23 +77,23 @@ func (c *Client) ObtainOAuthToken(credentials AuthConfig) error { oauthResp := &OAuthResponse{} err = json.Unmarshal(bodyBytes, oauthResp) if err != nil { - c.logger.Error("Failed to decode OAuth response", zap.Error(err)) + log.Error("Failed to decode OAuth response", zap.Error(err)) return err } if oauthResp.Error != "" { - c.logger.Error("Error obtaining OAuth token", zap.String("Error", oauthResp.Error)) + log.Error("Error obtaining OAuth token", zap.String("Error", oauthResp.Error)) return fmt.Errorf("error obtaining OAuth token: %s", oauthResp.Error) } if oauthResp.AccessToken == "" { - c.logger.Error("Empty access token received") + log.Error("Empty access token received") return fmt.Errorf("empty access token received") } expiresIn := time.Duration(oauthResp.ExpiresIn) * time.Second expirationTime := time.Now().Add(expiresIn) - c.logger.Info("OAuth token obtained successfully", zap.String("AccessToken", oauthResp.AccessToken), zap.Duration("ExpiresIn", expiresIn), zap.Time("ExpirationTime", expirationTime)) + log.Info("OAuth token obtained successfully", zap.String("AccessToken", oauthResp.AccessToken), zap.Duration("ExpiresIn", expiresIn), zap.Time("ExpirationTime", expirationTime)) c.Token = oauthResp.AccessToken c.Expiry = expirationTime @@ -102,32 +103,32 @@ func (c *Client) ObtainOAuthToken(credentials AuthConfig) error { // InvalidateOAuthToken invalidates the current OAuth access token. // After invalidation, the token cannot be used for further API requests. -func (c *Client) InvalidateOAuthToken() error { +func (c *Client) InvalidateOAuthToken(log logger.Logger) error { invalidateTokenEndpoint := c.ConstructAPIAuthEndpoint(TokenInvalidateEndpoint) - c.logger.Debug("Attempting to invalidate OAuth token", zap.String("Endpoint", invalidateTokenEndpoint)) + log.Debug("Attempting to invalidate OAuth token", zap.String("Endpoint", invalidateTokenEndpoint)) req, err := http.NewRequest("POST", invalidateTokenEndpoint, nil) if err != nil { - c.logger.Error("Failed to create new request for token invalidation", zap.Error(err)) + log.Error("Failed to create new request for token invalidation", zap.Error(err)) return err } req.Header.Add("Authorization", "Bearer "+c.Token) resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to make request for token invalidation", zap.Error(err)) + log.Error("Failed to make request for token invalidation", zap.Error(err)) return err } defer resp.Body.Close() if resp.StatusCode != http.StatusNoContent { errMsg := fmt.Errorf("failed to invalidate token, status code: %d", resp.StatusCode) - c.logger.Error("Failed to invalidate OAuth token", zap.Int("StatusCode", resp.StatusCode), zap.Error(errMsg)) + log.Error("Failed to invalidate OAuth token", zap.Int("StatusCode", resp.StatusCode), zap.Error(errMsg)) return errMsg } - c.logger.Info("OAuth token invalidated successfully", zap.String("Endpoint", invalidateTokenEndpoint)) + log.Info("OAuth token invalidated successfully", zap.String("Endpoint", invalidateTokenEndpoint)) return nil } From e37bcb7195d244a85ac1d90611985e0f919825c6 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:50:05 +0000 Subject: [PATCH 11/16] Add APIHandler interface methods for default base domain and token endpoints --- .../jamfpro/jamfpro_api_handler.go | 20 ++++++++++++++ internal/errors/http_error_handling.go | 26 +++++++++++-------- internal/httpclient/api_handler.go | 5 ++++ .../http_client_bearer_token_auth.go | 13 +++++++--- internal/httpclient/http_client_oauth.go | 11 ++++++-- 5 files changed, 58 insertions(+), 17 deletions(-) diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 43af971..2eb7ab3 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -118,6 +118,26 @@ type Logger interface { // Functions +func (j *JamfAPIHandler) GetDefaultBaseDomain() string { + return DefaultBaseDomain +} + +func (j *JamfAPIHandler) GetOAuthTokenEndpoint() string { + return OAuthTokenEndpoint +} + +func (j *JamfAPIHandler) GetBearerTokenEndpoint() string { + return BearerTokenEndpoint +} + +func (j *JamfAPIHandler) GetTokenRefreshEndpoint() string { + return TokenRefreshEndpoint +} + +func (j *JamfAPIHandler) GetTokenInvalidateEndpoint() string { + return TokenInvalidateEndpoint +} + // GetBaseDomain returns the appropriate base domain for URL construction. // It uses OverrideBaseDomain if set, otherwise falls back to DefaultBaseDomain. func (j *JamfAPIHandler) GetBaseDomain() string { diff --git a/internal/errors/http_error_handling.go b/internal/errors/http_error_handling.go index 58baf7e..d712907 100644 --- a/internal/errors/http_error_handling.go +++ b/internal/errors/http_error_handling.go @@ -7,13 +7,15 @@ import ( "fmt" "net/http" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) // APIError represents a structured API error response. type APIError struct { - StatusCode int - Message string + StatusCode int // HTTP status code + Type string // A brief identifier for the type of error (e.g., "RateLimit", "BadRequest", etc.) + Message string // Human-readable message } // StructuredError represents a structured error response from the API. @@ -24,19 +26,25 @@ type StructuredError struct { } `json:"error"` } +// Error returns a string representation of the APIError. +func (e *APIError) Error() string { + return fmt.Sprintf("API Error (Type: %s, Code: %d): %s", e.Type, e.StatusCode, e.Message) +} + // HandleAPIError handles error responses from the API, converting them into a structured error if possible. -func HandleAPIError(logger Logger, resp *http.Response) error { +func HandleAPIError(resp *http.Response, log logger.Logger) error { var structuredErr StructuredError err := json.NewDecoder(resp.Body).Decode(&structuredErr) if err == nil && structuredErr.Error.Message != "" { // Using structured logging to log the structured error details - c.Logger.Warn("API returned structured error", + log.Warn("API returned structured error", zap.String("status", resp.Status), zap.String("error_code", structuredErr.Error.Code), zap.String("error_message", structuredErr.Error.Message), ) return &APIError{ StatusCode: resp.StatusCode, + Type: structuredErr.Error.Code, Message: structuredErr.Error.Message, } } @@ -46,13 +54,13 @@ func HandleAPIError(logger Logger, resp *http.Response) error { if err != nil || errMsg == "" { errMsg = fmt.Sprintf("Unexpected error with status code: %d", resp.StatusCode) // Logging with structured fields - c.Logger.Warn("Failed to decode API error message, using default error message", + log.Warn("Failed to decode API error message, using default error message", zap.String("status", resp.Status), zap.String("error_message", errMsg), ) } else { // Logging non-structured error as a warning with structured fields - c.Logger.Warn("API returned non-structured error", + log.Warn("API returned non-structured error", zap.String("status", resp.Status), zap.String("error_message", errMsg), ) @@ -60,15 +68,11 @@ func HandleAPIError(logger Logger, resp *http.Response) error { return &APIError{ StatusCode: resp.StatusCode, + Type: "UnexpectedError", Message: errMsg, } } -// Error returns a string representation of the APIError. -func (e *APIError) Error() string { - return fmt.Sprintf("API Error (Code: %d): %s", e.StatusCode, e.Message) -} - // TranslateStatusCode provides a human-readable message for HTTP status codes. func TranslateStatusCode(statusCode int) string { messages := map[int]string{ diff --git a/internal/httpclient/api_handler.go b/internal/httpclient/api_handler.go index 82fb278..f723a35 100644 --- a/internal/httpclient/api_handler.go +++ b/internal/httpclient/api_handler.go @@ -20,6 +20,11 @@ type APIHandler interface { UnmarshalResponse(resp *http.Response, out interface{}, log logger.Logger) error GetContentTypeHeader(method string, log logger.Logger) string GetAcceptHeader() string + GetDefaultBaseDomain() string + GetOAuthTokenEndpoint() string + GetBearerTokenEndpoint() string + GetTokenRefreshEndpoint() string + GetTokenInvalidateEndpoint() string } // LoadAPIHandler returns an APIHandler based on the provided API type. diff --git a/internal/httpclient/http_client_bearer_token_auth.go b/internal/httpclient/http_client_bearer_token_auth.go index cd98f7e..0b784f3 100644 --- a/internal/httpclient/http_client_bearer_token_auth.go +++ b/internal/httpclient/http_client_bearer_token_auth.go @@ -9,6 +9,7 @@ import ( "net/http" "time" + "github.com/deploymenttheory/go-api-http-client/internal/errors" "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -28,7 +29,9 @@ func (c *Client) SetBearerTokenAuthCredentials(credentials BearerTokenAuthCreden // ObtainToken fetches and sets an authentication token using the stored basic authentication credentials. func (c *Client) ObtainToken(log logger.Logger) error { - authenticationEndpoint := APIHandler.ConstructAPIAuthEndpoint(BearerTokenEndpoint) + + bearerTokenEndpoint := c.APIHandler.GetBearerTokenEndpoint() + authenticationEndpoint := c.APIHandler.ConstructAPIAuthEndpoint(bearerTokenEndpoint, c.Logger) log.Debug("Attempting to obtain token for user", zap.String("Username", c.BearerTokenAuthCredentials.Username)) @@ -48,7 +51,7 @@ func (c *Client) ObtainToken(log logger.Logger) error { if resp.StatusCode != http.StatusOK { log.Warn("Received non-OK response while obtaining token", zap.Int("StatusCode", resp.StatusCode)) - return c.HandleAPIError(resp) + return errors.HandleAPIError(resp, log) } tokenResp := &TokenResponse{} @@ -115,7 +118,9 @@ func (c *Client) RefreshToken(log logger.Logger) error { c.tokenLock.Lock() defer c.tokenLock.Unlock() - tokenRefreshEndpoint := c.ConstructAPIAuthEndpoint(TokenRefreshEndpoint) + apiTokenRefreshEndpoint := c.APIHandler.GetTokenRefreshEndpoint() + + tokenRefreshEndpoint := c.APIHandler.ConstructAPIAuthEndpoint(apiTokenRefreshEndpoint, c.Logger) req, err := http.NewRequest("POST", tokenRefreshEndpoint, nil) if err != nil { @@ -135,7 +140,7 @@ func (c *Client) RefreshToken(log logger.Logger) error { if resp.StatusCode != http.StatusOK { log.Warn("Token refresh response status is not OK", zap.Int("StatusCode", resp.StatusCode)) - return c.HandleAPIError(resp) + return errors.HandleAPIError(resp) } tokenResp := &TokenResponse{} diff --git a/internal/httpclient/http_client_oauth.go b/internal/httpclient/http_client_oauth.go index 8301d04..e3b591b 100644 --- a/internal/httpclient/http_client_oauth.go +++ b/internal/httpclient/http_client_oauth.go @@ -43,7 +43,10 @@ func (c *Client) SetOAuthCredentials(credentials OAuthCredentials) { // ObtainOAuthToken fetches an OAuth access token using the provided OAuthCredentials (Client ID and Client Secret). // It updates the client's Token and Expiry fields with the obtained values. func (c *Client) ObtainOAuthToken(credentials AuthConfig, log logger.Logger) error { - authenticationEndpoint := c.ConstructAPIAuthEndpoint(OAuthTokenEndpoint) + + oauthTokenEndpoint := c.APIHandler.GetOAuthTokenEndpoint() + authenticationEndpoint := c.APIHandler.ConstructAPIAuthEndpoint(oauthTokenEndpoint, c.Logger) + data := url.Values{} data.Set("client_id", credentials.ClientID) data.Set("client_secret", credentials.ClientSecret) @@ -101,10 +104,13 @@ func (c *Client) ObtainOAuthToken(credentials AuthConfig, log logger.Logger) err return nil } +/* // InvalidateOAuthToken invalidates the current OAuth access token. // After invalidation, the token cannot be used for further API requests. func (c *Client) InvalidateOAuthToken(log logger.Logger) error { - invalidateTokenEndpoint := c.ConstructAPIAuthEndpoint(TokenInvalidateEndpoint) + + tokenInvalidateEndpoint := c.APIHandler.GetTokenInvalidateEndpoint() + invalidateTokenEndpoint := APIHandler.ConstructAPIAuthEndpoint(tokenInvalidateEndpoint, log) log.Debug("Attempting to invalidate OAuth token", zap.String("Endpoint", invalidateTokenEndpoint)) @@ -132,3 +138,4 @@ func (c *Client) InvalidateOAuthToken(log logger.Logger) error { return nil } +*/ From 7db7418ab1e8d61ad0a01aec5877468d6617cf63 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:57:32 +0000 Subject: [PATCH 12/16] Add error handling for non-success status codes in JamfPro API handler --- .../jamfpro/jamfpro_api_handler.go | 57 ++++++++++++------- .../http_client_bearer_token_auth.go | 2 +- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 2eb7ab3..1641dfc 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -49,6 +49,7 @@ import ( _ "embed" + "github.com/deploymenttheory/go-api-http-client/internal/errors" "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -277,29 +278,37 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, if strings.Contains(contentType, "text/html") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) log.Warn("Received HTML content", zap.String("error_message", errMsg), zap.Int("status_code", resp.StatusCode)) - return &APIError{ + return &errors.APIError{ StatusCode: resp.StatusCode, Message: errMsg, } } // Check for non-success status codes before attempting to unmarshal - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - // Parse the error details from the response body for JSON content type - if strings.Contains(contentType, "application/json") { +if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Parse the error details from the response body for JSON content type + if strings.Contains(contentType, "application/json") { description, err := ParseJSONErrorResponse(bodyBytes) if err != nil { - log.Error("Failed to parse JSON error response", "error", err) - return fmt.Errorf("received non-success status code: %d and failed to parse error response", resp.StatusCode) + // Log the error using the structured logger and return the error + return nil, c.Logger.Error("Failed to parse JSON error response", + zap.Error(err), + zap.Int("status_code", resp.StatusCode), + ) } - return fmt.Errorf("received non-success status code: %d, error: %s", resp.StatusCode, description) - } - - // If the response is not JSON or another error occurs, return a generic error message - log.Error("Received non-success status code", "status_code", resp.StatusCode) - return fmt.Errorf("received non-success status code: %d", resp.StatusCode) + // Log the error with description using the structured logger and return the error + return nil, c.Logger.Error("Received non-success status code with JSON response", + zap.Int("status_code", resp.StatusCode), + zap.String("error_description", description), + ) } + // If the response is not JSON or another error occurs, log a generic error message and return an error + return nil, c.Logger.Error("Received non-success status code without JSON response", + zap.Int("status_code", resp.StatusCode), + ) +} + // Determine whether the content type is JSON or XML and unmarshal accordingly switch { case strings.Contains(contentType, "application/json"): @@ -312,19 +321,23 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, } // Handle any errors that occurred during unmarshaling - if err != nil { - // If unmarshalling fails, check if the content might be HTML - if strings.Contains(string(bodyBytes), "") { +if err != nil { + // If unmarshalling fails, check if the content might be HTML + if strings.Contains(string(bodyBytes), "") { errMsg := ExtractErrorMessageFromHTML(string(bodyBytes)) - log.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode) - return fmt.Errorf(errMsg) - } - - // Log the error and return it - log.Error("Failed to unmarshal response", "error", err) - return fmt.Errorf("failed to unmarshal response: %v", err) + + // Log the warning and return an error using the structured logger + return nil, log.Warn("Received HTML content instead of expected format", + zap.String("error_message", errMsg), + zap.Int("status_code", resp.StatusCode), + ) } + // Log the error using the structured logger and return the error + return nil, log.Error("Failed to unmarshal response", + zap.Error(err), + ) + return nil } diff --git a/internal/httpclient/http_client_bearer_token_auth.go b/internal/httpclient/http_client_bearer_token_auth.go index 0b784f3..2748b61 100644 --- a/internal/httpclient/http_client_bearer_token_auth.go +++ b/internal/httpclient/http_client_bearer_token_auth.go @@ -140,7 +140,7 @@ func (c *Client) RefreshToken(log logger.Logger) error { if resp.StatusCode != http.StatusOK { log.Warn("Token refresh response status is not OK", zap.Int("StatusCode", resp.StatusCode)) - return errors.HandleAPIError(resp) + return errors.HandleAPIError(resp, log) } tokenResp := &TokenResponse{} From 11ad03f8cb19826c46b1d05ed76a43bb12051e4e Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:02:56 +0000 Subject: [PATCH 13/16] Refactor APIHandler interface and remove unnecessary code --- internal/apihandlers/jamfpro/jamfpro_api_handler.go | 1 - internal/httpclient/api_handler.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/apihandlers/jamfpro/jamfpro_api_handler.go b/internal/apihandlers/jamfpro/jamfpro_api_handler.go index 1641dfc..7a15a73 100644 --- a/internal/apihandlers/jamfpro/jamfpro_api_handler.go +++ b/internal/apihandlers/jamfpro/jamfpro_api_handler.go @@ -338,7 +338,6 @@ if err != nil { zap.Error(err), ) - return nil } // GetAcceptHeader constructs and returns a weighted Accept header string for HTTP requests. diff --git a/internal/httpclient/api_handler.go b/internal/httpclient/api_handler.go index f723a35..ebd41d3 100644 --- a/internal/httpclient/api_handler.go +++ b/internal/httpclient/api_handler.go @@ -9,7 +9,7 @@ import ( "go.uber.org/zap" ) -// APIHandler is an interface for encoding, decoding, and determining content types for different API implementations. +// APIHandler is an interface for encoding, decoding, and implenting contexual api functions for different API implementations. // It encapsulates behavior for encoding and decoding requests and responses. type APIHandler interface { GetBaseDomain() string From 487b5297edfd1d762369c0fcbfd6375868bc53a8 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:45:40 +0000 Subject: [PATCH 14/16] Add deprecation header check and logging --- internal/httpclient/http_helpers.go | 8 ++-- internal/httpclient/http_request.go | 62 +++++++++++++++-------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/internal/httpclient/http_helpers.go b/internal/httpclient/http_helpers.go index 877f1d9..d221e25 100644 --- a/internal/httpclient/http_helpers.go +++ b/internal/httpclient/http_helpers.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/deploymenttheory/go-api-http-client/internal/logger" "go.uber.org/zap" ) @@ -26,12 +27,11 @@ func EnsureHTTPScheme(url string) string { } // CheckDeprecationHeader checks the response headers for the Deprecation header and logs a warning if present. -func CheckDeprecationHeader(resp *http.Response, logger Logger) { +func CheckDeprecationHeader(resp *http.Response, log logger.Logger) { deprecationHeader := resp.Header.Get("Deprecation") if deprecationHeader != "" { - // logger is an instance of defaultLogger that wraps a *zap.Logger - zapLogger := logger.(*defaultLogger).logger // Type assertion to access the underlying *zap.Logger - zapLogger.Warn("API endpoint is deprecated", + + log.Warn("API endpoint is deprecated", zap.String("Date", deprecationHeader), zap.String("Endpoint", resp.Request.URL.String()), ) diff --git a/internal/httpclient/http_request.go b/internal/httpclient/http_request.go index c066aae..3dce6e4 100644 --- a/internal/httpclient/http_request.go +++ b/internal/httpclient/http_request.go @@ -7,6 +7,8 @@ import ( "fmt" "net/http" "time" + + "github.com/deploymenttheory/go-api-http-client/internal/logger" ) // DoRequest constructs and executes a standard HTTP request with support for retry logic. @@ -40,9 +42,9 @@ import ( // Note: // The function assumes that retryable HTTP methods have been properly defined in the retryableHTTPMethods map. // It is the caller's responsibility to close the response body when the request is successful to avoid resource leaks. -func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*http.Response, error) { +func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log logger.Logger) (*http.Response, error) { // Auth Token validation check - valid, err := c.ValidAuthTokenCheck() + valid, err := c.ValidAuthTokenCheck(log) if err != nil || !valid { return nil, fmt.Errorf("validity of the authentication token failed with error: %w", err) } @@ -70,7 +72,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt apiHandler := c.APIHandler // Marshal Request with correct encoding - requestData, err := apiHandler.MarshalRequest(body, method, endpoint, c.logger) + requestData, err := apiHandler.MarshalRequest(body, method, endpoint, log) if err != nil { return nil, err } @@ -90,9 +92,9 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt } // Define header content type based on url and http method - contentType := apiHandler.GetContentTypeHeader(endpoint, c.logger) + contentType := apiHandler.GetContentTypeHeader(endpoint, log) // Define Request Headers dynamically based on handler logic - acceptHeader := apiHandler.GetAcceptHeader(c.logger) + acceptHeader := apiHandler.GetAcceptHeader() // Set Headers req.Header.Add("Authorization", "Bearer "+c.Token) @@ -101,7 +103,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt req.Header.Set("User-Agent", GetUserAgentHeader()) // Debug: Print request headers if in debug mode - c.logger.Debug("HTTP Request Headers:", req.Header) + log.Debug("HTTP Request Headers:", req.Header) // Define if request is retryable retryableHTTPMethods := map[string]bool{ @@ -131,7 +133,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to send multipart request", + log.Error("Failed to send multipart request", "method", method, "endpoint", endpoint, "error", err.Error(), @@ -146,15 +148,15 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt // Checks for the presence of a deprecation header in the HTTP response and logs if found. if i == 0 { - CheckDeprecationHeader(resp, c.logger) + CheckDeprecationHeader(resp, log) } // Handle (unmarshal) response with API Handler - if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { + if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { switch e := err.(type) { case *APIError: // Assuming APIError is a type that includes StatusCode and Message // Log the API error with structured logging - c.logger.Error("Received an API error", + log.Error("Received an API error", "method", method, "endpoint", endpoint, "status_code", e.StatusCode, @@ -163,7 +165,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt return resp, e default: // Log the default error with structured logging - c.logger.Error("Failed to unmarshal HTTP response", + log.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err.Error(), // Convert error to string to log as a value @@ -174,7 +176,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt if resp.StatusCode >= 200 && resp.StatusCode < 300 { // Log successful HTTP request - c.logger.Info("HTTP request succeeded", + log.Info("HTTP request succeeded", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -182,7 +184,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt return resp, nil } else if resp.StatusCode == http.StatusNotFound { // Log when resource is not found - c.logger.Warn("Resource not found", + log.Warn("Resource not found", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -192,7 +194,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt // Retry Logic if IsNonRetryableError(resp) { - c.logger.Warn("Encountered a non-retryable error", + log.Warn("Encountered a non-retryable error", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -201,7 +203,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt return resp, c.HandleAPIError(resp) } else if IsRateLimitError(resp) { waitDuration := parseRateLimitHeaders(resp) // Parses headers to determine wait duration - c.logger.Warn("Encountered a rate limit error. Retrying after wait duration.", + log.Warn("Encountered a rate limit error. Retrying after wait duration.", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -212,7 +214,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt continue // This will restart the loop, effectively "retrying" the request } else if IsTransientError(resp) { waitDuration := calculateBackoff(i) // Calculates backoff duration - c.logger.Warn("Encountered a transient error. Retrying after backoff.", + log.Warn("Encountered a transient error. Retrying after backoff.", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -223,7 +225,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt i++ continue // This will restart the loop, effectively "retrying" the request } else { - c.logger.Error("Received unexpected error status from HTTP request", + log.Error("Received unexpected error status from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -240,7 +242,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to send multipart request", + log.Error("Failed to send multipart request", "method", method, "endpoint", endpoint, "error", err.Error(), @@ -254,14 +256,14 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt c.PerfMetrics.TotalResponseTime += responseDuration c.PerfMetrics.lock.Unlock() - CheckDeprecationHeader(resp, c.logger) + CheckDeprecationHeader(resp, log) // Handle (unmarshal) response with API Handler - if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { + if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { switch e := err.(type) { case *APIError: // Assuming APIError is a type that includes StatusCode and Message // Log the API error with structured logging - c.logger.Error("Received an API error", + log.Error("Received an API error", "method", method, "endpoint", endpoint, "status_code", e.StatusCode, @@ -270,7 +272,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt return resp, e default: // Log the default error with structured logging - c.logger.Error("Failed to unmarshal HTTP response", + log.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err.Error(), // Convert error to string to log as a value @@ -288,7 +290,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt statusDescription := TranslateStatusCode(resp.StatusCode) // Log the error with structured context - c.logger.Error("Received non-success status code from HTTP request", + log.Error("Received non-success status code from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -340,7 +342,7 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s apiHandler := c.APIHandler // Marshal the multipart form data - requestData, contentType, err := apiHandler.MarshalMultipartRequest(fields, files, c.logger) + requestData, contentType, err := apiHandler.MarshalMultipartRequest(fields, files, log) if err != nil { return nil, err } @@ -362,15 +364,15 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s // Debug: Print request headers if in debug mode // Check if logging level is DEBUG or higher before logging headers - if c.logger.GetLogLevel() <= LogLevelDebug { + if log.GetLogLevel() <= LogLevelDebug { // Debug: Print request headers without hiding sensitive information - LogHTTPHeaders(c.logger, req.Header, false) // Use false to display all headers + LogHTTPHeaders(log, req.Header, false) // Use false to display all headers } // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - c.logger.Error("Failed to send multipart request", + log.Error("Failed to send multipart request", "method", method, "endpoint", endpoint, "error", err.Error(), @@ -380,7 +382,7 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s // Check for successful status code if resp.StatusCode < 200 || resp.StatusCode >= 300 { - c.logger.Error("Received non-success status code from multipart request", + log.Error("Received non-success status code from multipart request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, @@ -390,8 +392,8 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s } // Unmarshal the response - if err := apiHandler.UnmarshalResponse(resp, out, c.logger); err != nil { - c.logger.Error("Failed to unmarshal HTTP response", + if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { + log.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err.Error(), From 75c75fdc35c83ceb3f1fcda259399fa84b3a95dd Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:27:43 +0000 Subject: [PATCH 15/16] Refactor LogHTTPHeaders function to convert http.Header to string representation --- internal/httpclient/http_headers.go | 33 ++--- internal/httpclient/http_request.go | 208 +++++++++++++++------------- 2 files changed, 122 insertions(+), 119 deletions(-) diff --git a/internal/httpclient/http_headers.go b/internal/httpclient/http_headers.go index 555b284..0af366e 100644 --- a/internal/httpclient/http_headers.go +++ b/internal/httpclient/http_headers.go @@ -1,30 +1,21 @@ package httpclient import ( + "fmt" "net/http" - - "github.com/deploymenttheory/go-api-http-client/internal/logger" + "strings" ) -// LogHTTPHeaders logs the HTTP headers of an HTTP request or response, with an option to hide sensitive information like the token in secure mode. -func LogHTTPHeaders(log logger.Logger, headers http.Header, secureMode bool) { - var keysAndValues []interface{} - if secureMode { - for key, values := range headers { - if key != "Authorization" { // Exclude the token header - // Assuming each header has a single value for simplicity - keysAndValues = append(keysAndValues, key, values[0]) - } - } - } else { - for key, values := range headers { - // Assuming each header has a single value for simplicity - keysAndValues = append(keysAndValues, key, values[0]) - } - } +// HeadersToString converts an http.Header map to a single string representation. +func HeadersToString(headers http.Header) string { + var headerStrings []string - // Log the headers using the logger from the httpclient package - if len(keysAndValues) > 0 { - log.Debug("HTTP Headers", keysAndValues...) + // Iterate over the map and append each key-value pair to the slice + for name, values := range headers { + // Combine each header's key with its values, which are joined by a comma + headerStrings = append(headerStrings, fmt.Sprintf("%s: %s", name, strings.Join(values, ", "))) } + + // Join all header strings into a single string + return strings.Join(headerStrings, "; ") } diff --git a/internal/httpclient/http_request.go b/internal/httpclient/http_request.go index 3dce6e4..459a73a 100644 --- a/internal/httpclient/http_request.go +++ b/internal/httpclient/http_request.go @@ -8,7 +8,9 @@ import ( "net/http" "time" + "github.com/deploymenttheory/go-api-http-client/internal/errors" "github.com/deploymenttheory/go-api-http-client/internal/logger" + "go.uber.org/zap" ) // DoRequest constructs and executes a standard HTTP request with support for retry logic. @@ -78,7 +80,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l } // Construct URL using the ConstructAPIResourceEndpoint function - url := apiHandler.ConstructAPIResourceEndpoint(endpoint) + url := apiHandler.ConstructAPIResourceEndpoint(endpoint, log) // Initialize total request counter c.PerfMetrics.lock.Lock() @@ -103,7 +105,8 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l req.Header.Set("User-Agent", GetUserAgentHeader()) // Debug: Print request headers if in debug mode - log.Debug("HTTP Request Headers:", req.Header) + headersStr := HeadersToString(req.Header) + log.Debug("HTTP Multipart Request Headers:", zap.String("headers", headersStr)) // Define if request is retryable retryableHTTPMethods := map[string]bool{ @@ -133,10 +136,11 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - log.Error("Failed to send multipart request", - "method", method, - "endpoint", endpoint, - "error", err.Error(), + log.Error("Failed to send retryable request", + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("status_text", http.StatusText(resp.StatusCode)), ) return nil, err } @@ -153,85 +157,88 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l // Handle (unmarshal) response with API Handler if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { - switch e := err.(type) { - case *APIError: // Assuming APIError is a type that includes StatusCode and Message - // Log the API error with structured logging + // Use type assertion to check if the error is of type *errors.APIError + if apiErr, ok := err.(*errors.APIError); ok { + // Log the API error with structured logging for specific APIError handling log.Error("Received an API error", - "method", method, - "endpoint", endpoint, - "status_code", e.StatusCode, - "message", e.Message, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", apiErr.StatusCode), + zap.String("message", apiErr.Message), ) - return resp, e - default: - // Log the default error with structured logging + return resp, apiErr // Return the typed error for further handling if needed + } else { + // Log other errors with structured logging for general error handling log.Error("Failed to unmarshal HTTP response", - "method", method, - "endpoint", endpoint, - "error", err.Error(), // Convert error to string to log as a value + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Error(err), // Use zap.Error to log the error object ) return resp, err } } + // Successful response if resp.StatusCode >= 200 && resp.StatusCode < 300 { - // Log successful HTTP request log.Info("HTTP request succeeded", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), ) return resp, nil - } else if resp.StatusCode == http.StatusNotFound { - // Log when resource is not found + } else if + // Resource not found + resp.StatusCode == http.StatusNotFound { log.Warn("Resource not found", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), ) - return resp, fmt.Errorf("resource not found: %s", endpoint) + // Use a centralized method for handling not found error + return resp, err } // Retry Logic - if IsNonRetryableError(resp) { + // Non-retryable error + if errors.IsNonRetryableError(resp) { log.Warn("Encountered a non-retryable error", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "description", TranslateStatusCode(resp.StatusCode), + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("description", errors.TranslateStatusCode(resp.StatusCode)), ) - return resp, c.HandleAPIError(resp) - } else if IsRateLimitError(resp) { + return resp, errors.HandleAPIError(resp, log) // Assume this method logs the error internally + } else if errors.IsRateLimitError(resp) { waitDuration := parseRateLimitHeaders(resp) // Parses headers to determine wait duration log.Warn("Encountered a rate limit error. Retrying after wait duration.", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "wait_duration", waitDuration, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.Duration("wait_duration", waitDuration), ) time.Sleep(waitDuration) i++ continue // This will restart the loop, effectively "retrying" the request - } else if IsTransientError(resp) { + } else if errors.IsTransientError(resp) { waitDuration := calculateBackoff(i) // Calculates backoff duration log.Warn("Encountered a transient error. Retrying after backoff.", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "wait_duration", waitDuration, - "attempt", i, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.Duration("wait_duration", waitDuration), + zap.Int("attempt", i), ) time.Sleep(waitDuration) i++ continue // This will restart the loop, effectively "retrying" the request } else { log.Error("Received unexpected error status from HTTP request", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "description", TranslateStatusCode(resp.StatusCode), + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("description", errors.TranslateStatusCode(resp.StatusCode)), ) - return resp, c.HandleAPIError(resp) + return resp, errors.HandleAPIError(resp, log) } } } else { @@ -242,10 +249,11 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l // Execute the request resp, err := c.httpClient.Do(req) if err != nil { - log.Error("Failed to send multipart request", - "method", method, - "endpoint", endpoint, - "error", err.Error(), + log.Error("Failed to send request", + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("status_text", http.StatusText(resp.StatusCode)), ) return nil, err } @@ -260,24 +268,24 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l // Handle (unmarshal) response with API Handler if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { - switch e := err.(type) { - case *APIError: // Assuming APIError is a type that includes StatusCode and Message - // Log the API error with structured logging + // Use type assertion to check if the error is of type *errors.APIError + if apiErr, ok := err.(*errors.APIError); ok { + // Log the API error with structured logging for specific APIError handling log.Error("Received an API error", - "method", method, - "endpoint", endpoint, - "status_code", e.StatusCode, - "message", e.Message, + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", apiErr.StatusCode), + zap.String("message", apiErr.Message), ) - return resp, e - default: - // Log the default error with structured logging + return resp, apiErr // Return the typed error for further handling if needed + } else { + // Log other errors with structured logging for general error handling log.Error("Failed to unmarshal HTTP response", - "method", method, - "endpoint", endpoint, - "error", err.Error(), // Convert error to string to log as a value + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Error(err), // Use zap.Error to log the error object ) - return resp, err + return resp, err // Return the original error } } @@ -287,15 +295,16 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l return resp, nil } else { // Translate the status code to a human-readable description - statusDescription := TranslateStatusCode(resp.StatusCode) - - // Log the error with structured context - log.Error("Received non-success status code from HTTP request", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "description", statusDescription, - ) + statusDescription := errors.TranslateStatusCode(resp.StatusCode) + if apiErr, ok := err.(*errors.APIError); ok { + // Log the API error with structured logging for specific APIError handling + log.Error("Received an API error", + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", apiErr.StatusCode), + zap.String("message", apiErr.Message), + ) + } // Return an error with the status code and its description return resp, fmt.Errorf("Error status code: %d - %s", resp.StatusCode, statusDescription) @@ -331,9 +340,9 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l // // Note: // The caller should handle closing the response body when successful. -func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]string, files map[string]string, out interface{}) (*http.Response, error) { +func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]string, files map[string]string, out interface{}, log logger.Logger) (*http.Response, error) { // Auth Token validation check - valid, err := c.ValidAuthTokenCheck() + valid, err := c.ValidAuthTokenCheck(log) if err != nil || !valid { return nil, fmt.Errorf("validity of the authentication token failed with error: %w", err) } @@ -348,7 +357,7 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s } // Construct URL using the ConstructAPIResourceEndpoint function - url := apiHandler.ConstructAPIResourceEndpoint(endpoint) + url := apiHandler.ConstructAPIResourceEndpoint(endpoint, log) // Create the request req, err := http.NewRequest(method, url, bytes.NewBuffer(requestData)) @@ -362,41 +371,44 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s req.Header.Set("User-Agent", GetUserAgentHeader()) // Debug: Print request headers if in debug mode - - // Check if logging level is DEBUG or higher before logging headers - if log.GetLogLevel() <= LogLevelDebug { - // Debug: Print request headers without hiding sensitive information - LogHTTPHeaders(log, req.Header, false) // Use false to display all headers - } + headersStr := HeadersToString(req.Header) + log.Debug("HTTP Multipart Request Headers:", zap.String("headers", headersStr)) // Execute the request resp, err := c.httpClient.Do(req) if err != nil { log.Error("Failed to send multipart request", - "method", method, - "endpoint", endpoint, - "error", err.Error(), + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("status_text", http.StatusText(resp.StatusCode)), ) return nil, err } // Check for successful status code if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Use HandleAPIError to process the error response and log it accordingly + apiErr := errors.HandleAPIError(resp, log) + + // Log additional context about the request that led to the error log.Error("Received non-success status code from multipart request", - "method", method, - "endpoint", endpoint, - "status_code", resp.StatusCode, - "status_text", http.StatusText(resp.StatusCode), + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.Int("status_code", resp.StatusCode), + zap.String("status_text", http.StatusText(resp.StatusCode)), ) - return resp, fmt.Errorf("received non-success status code: %d - %s", resp.StatusCode, http.StatusText(resp.StatusCode)) + + // Return the original HTTP response and the API error + return resp, apiErr } // Unmarshal the response if err := apiHandler.UnmarshalResponse(resp, out, log); err != nil { log.Error("Failed to unmarshal HTTP response", - "method", method, - "endpoint", endpoint, - "error", err.Error(), + zap.String("method", method), + zap.String("endpoint", endpoint), + zap.String("error", err.Error()), ) return resp, err } From 2feeef6689a51da86bde704def282a72e259f344 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:32:13 +0000 Subject: [PATCH 16/16] Remove main.go file --- main.go | 1 - 1 file changed, 1 deletion(-) delete mode 100644 main.go diff --git a/main.go b/main.go deleted file mode 100644 index 06ab7d0..0000000 --- a/main.go +++ /dev/null @@ -1 +0,0 @@ -package main