Merge pull request #98 from deploymenttheory/dev

ShocOne · web-flow · commit 22a260ee412d · 2024-03-01T10:43:33.000Z
Dev
diff --git a/httpclient/httpclient_auth_bearer_token.go b/httpclient/httpclient_auth_bearer_token.go
@@ -6,6 +6,7 @@ package httpclient
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -39,21 +40,21 @@ func (c *Client) ObtainToken(log logger.Logger) error {
 
 	req, err := http.NewRequest("POST", authenticationEndpoint, nil)
 	if err != nil {
-		log.LogError("POST", authenticationEndpoint, 0, err, "Failed to create new request for token")
+		log.LogError("authentication_request_creation_error", "POST", authenticationEndpoint, 0, err, "Failed to create new request for token")
 		return err
 	}
 	req.SetBasicAuth(c.BearerTokenAuthCredentials.Username, c.BearerTokenAuthCredentials.Password)
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		log.LogError("POST", authenticationEndpoint, 0, err, "Failed to make request for token")
+		log.LogError("authentication_request_error", "POST", authenticationEndpoint, 0, err, "Failed to make request for token")
 		return err
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		log.Error("Received non-OK response while obtaining token", zap.Int("StatusCode", resp.StatusCode))
-		return err
+		log.LogError("token_authentication_failed", "POST", authenticationEndpoint, resp.StatusCode, fmt.Errorf("authentication failed with status code: %d", resp.StatusCode), "Token acquisition attempt resulted in a non-OK response")
+		return fmt.Errorf("received non-OK response status: %d", resp.StatusCode)
 	}
 
 	tokenResp := &TokenResponse{}
diff --git a/httpclient/httpclient_error_response.go b/httpclient/httpclient_error_response.go
@@ -9,7 +9,6 @@ import (
 	"net/http"
 
 	"github.com/deploymenttheory/go-api-http-client/logger"
-	"go.uber.org/zap"
 )
 
 // APIError represents a more flexible structure for API error responses.
@@ -35,68 +34,47 @@ func (e *APIError) Error() string {
 	return fmt.Sprintf("API Error (Type: %s, Code: %d): %s", e.Type, e.StatusCode, e.Message)
 }
 
-// handleAPIErrorResponse attempts to parse the error response from the API and logs using zap logger.
+// handleAPIErrorResponse attempts to parse the error response from the API and logs using the zap logger.
 func handleAPIErrorResponse(resp *http.Response, log logger.Logger) *APIError {
-	apiError := &APIError{StatusCode: resp.StatusCode}
-
-	// Attempt to parse the response into a StructuredError
-	var structuredErr StructuredError
-	if err := json.NewDecoder(resp.Body).Decode(&structuredErr); err == nil && structuredErr.Error.Message != "" {
-		apiError.Type = structuredErr.Error.Code
-		apiError.Message = structuredErr.Error.Message
-
-		// Log the structured error details with zap logger
-		log.Warn("API returned structured error",
-			zap.String("error_code", structuredErr.Error.Code),
-			zap.String("error_message", structuredErr.Error.Message),
-			zap.Int("status_code", resp.StatusCode),
-		)
-
-		return apiError
+	apiError := &APIError{
+		StatusCode: resp.StatusCode,
+		Type:       "APIError",          // Default error type
+		Message:    "An error occurred", // Default error message
 	}
 
-	// If the structured error parsing fails, attempt a more generic parsing
 	bodyBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
-		// If reading the response body fails, store the error message and log the error
-		apiError.Raw = "Failed to read API error response body"
-		apiError.Message = err.Error()
-		apiError.Type = "ReadError"
-
-		log.Error("Failed to read API error response body",
-			zap.Error(err),
-		)
-
+		// Log and return an error if reading the body fails
+		log.LogError("api_response_read_error", "READ", resp.Request.URL.String(), resp.StatusCode, err, "")
 		return apiError
 	}
 
-	if err := json.Unmarshal(bodyBytes, &apiError.Errors); err != nil {
-		// If generic parsing also fails, store the raw response body and log the error
-		apiError.Raw = string(bodyBytes)
-		apiError.Message = "Failed to parse API error response"
-		apiError.Type = "UnexpectedError"
-
-		log.Error("Failed to parse API error response",
-			zap.String("raw_response", apiError.Raw),
-		)
-
+	if err := json.Unmarshal(bodyBytes, &apiError); err == nil && apiError.Message != "" {
+		// Log the structured error
+		log.LogError("api_structured_error", "API", resp.Request.URL.String(), resp.StatusCode, fmt.Errorf(apiError.Message), "")
 		return apiError
 	}
 
-	// Extract fields from the generic error map and log the error with extracted details
-	if msg, ok := apiError.Errors["message"].(string); ok {
-		apiError.Message = msg
-	}
-	if detail, ok := apiError.Errors["detail"].(string); ok {
-		apiError.Detail = detail
+	// If structured parsing fails, attempt to parse into a generic error map
+	var genericErr map[string]interface{}
+	if err := json.Unmarshal(bodyBytes, &genericErr); err == nil {
+		apiError.updateFromGenericError(genericErr)
+		// Log the error with extracted details
+		log.LogError("api_generic_error", "API", resp.Request.URL.String(), resp.StatusCode, fmt.Errorf(apiError.Message), "")
+		return apiError
 	}
 
-	log.Error("API error",
-		zap.Int("status_code", apiError.StatusCode),
-		zap.String("type", apiError.Type),
-		zap.String("message", apiError.Message),
-		zap.String("detail", apiError.Detail),
-	)
-
+	// If all parsing attempts fail, log the raw response
+	log.LogError("api_unexpected_error", "API", resp.Request.URL.String(), resp.StatusCode, fmt.Errorf("failed to parse API error response"), string(bodyBytes))
 	return apiError
 }
+
+func (e *APIError) updateFromGenericError(genericErr map[string]interface{}) {
+	if msg, ok := genericErr["message"].(string); ok {
+		e.Message = msg
+	}
+	if detail, ok := genericErr["detail"].(string); ok {
+		e.Detail = detail
+	}
+	// Add more fields as needed
+}
diff --git a/httpclient/httpclient_mocklogger.go b/httpclient/httpclient_mocklogger.go
@@ -69,26 +69,30 @@ func (m *MockLogger) GetLogLevel() logger.LogLevel {
 
 // Mock implementations for structured logging methods
 
-func (m *MockLogger) LogRequestStart(requestID string, userID string, method string, url string, headers map[string][]string) {
-	m.Called(requestID, userID, method, url, headers)
+func (m *MockLogger) LogRequestStart(event string, requestID string, userID string, method string, url string, headers map[string][]string) {
+	m.Called(event, requestID, userID, method, url, headers)
 }
 
-func (m *MockLogger) LogRequestEnd(method string, url string, statusCode int, duration time.Duration) {
-	m.Called(method, url, statusCode, duration)
+func (m *MockLogger) LogRequestEnd(event string, method string, url string, statusCode int, duration time.Duration) {
+	m.Called(event, method, url, statusCode, duration)
 }
 
-func (m *MockLogger) LogError(method string, url string, statusCode int, err error, stacktrace string) {
-	m.Called(method, url, statusCode, err, stacktrace)
+func (m *MockLogger) LogError(event string, method string, url string, statusCode int, err error, stacktrace string) {
+	m.Called(event, method, url, statusCode, err, stacktrace)
 }
 
-func (m *MockLogger) LogRetryAttempt(method string, url string, attempt int, reason string, waitDuration time.Duration, err error) {
-	m.Called(method, url, attempt, reason, waitDuration, err)
+func (m *MockLogger) LogAuthTokenError(event string, method string, url string, statusCode int, err error) {
+	m.Called(event, method, url, statusCode, err)
 }
 
-func (m *MockLogger) LogRateLimiting(method string, url string, retryAfter string, waitDuration time.Duration) {
-	m.Called(method, url, retryAfter, waitDuration)
+func (m *MockLogger) LogRetryAttempt(event string, method string, url string, attempt int, reason string, waitDuration time.Duration, err error) {
+	m.Called(event, method, url, attempt, reason, waitDuration, err)
 }
 
-func (m *MockLogger) LogResponse(method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration) {
-	m.Called(method, url, statusCode, responseBody, responseHeaders, duration)
+func (m *MockLogger) LogRateLimiting(event string, method string, url string, retryAfter string, waitDuration time.Duration) {
+	m.Called(event, method, url, retryAfter, waitDuration)
+}
+
+func (m *MockLogger) LogResponse(event string, method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration) {
+	m.Called(event, method, url, statusCode, responseBody, responseHeaders, duration)
 }
diff --git a/httpclient/httpclient_request.go b/httpclient/httpclient_request.go
@@ -207,7 +207,8 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in
 			if apiErr := handleAPIErrorResponse(resp, log); apiErr != nil {
 				err = apiErr
 			}
-			log.LogError(method, endpoint, resp.StatusCode, err, status.TranslateStatusCode(resp))
+			log.LogError("request_error", method, endpoint, resp.StatusCode, err, status.TranslateStatusCode(resp))
+
 			break
 		}
 	}
diff --git a/logger/zaplogger_logfields.go b/logger/zaplogger_logfields.go
@@ -7,10 +7,10 @@ import (
 )
 
 // LogRequestStart logs the initiation of an HTTP request if the current log level permits.
-func (d *defaultLogger) LogRequestStart(requestID string, userID string, method string, url string, headers map[string][]string) {
+func (d *defaultLogger) LogRequestStart(event string, requestID string, userID string, method string, url string, headers map[string][]string) {
 	if d.logLevel <= LogLevelInfo {
 		fields := []zap.Field{
-			zap.String("event", "request_start"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.String("request_id", requestID),
@@ -22,10 +22,10 @@ func (d *defaultLogger) LogRequestStart(requestID string, userID string, method
 }
 
 // LogRequestEnd logs the completion of an HTTP request if the current log level permits.
-func (d *defaultLogger) LogRequestEnd(method string, url string, statusCode int, duration time.Duration) {
+func (d *defaultLogger) LogRequestEnd(event string, method string, url string, statusCode int, duration time.Duration) {
 	if d.logLevel <= LogLevelInfo {
 		fields := []zap.Field{
-			zap.String("event", "request_end"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.Int("status_code", statusCode),
@@ -35,31 +35,31 @@ func (d *defaultLogger) LogRequestEnd(method string, url string, statusCode int,
 	}
 }
 
-// LogError logs an error that occurs during the processing of an HTTP request if the current log level permits.
-func (d *defaultLogger) LogError(method string, url string, statusCode int, err error, stacktrace string) {
+// LogError logs an error that occurs during the processing of an HTTP request or any other event, if the current log level permits.
+func (d *defaultLogger) LogError(event string, method, url string, statusCode int, err error, stacktrace string) {
 	if d.logLevel <= LogLevelError {
 		errorMessage := ""
 		if err != nil {
 			errorMessage = err.Error()
 		}
 
 		fields := []zap.Field{
-			zap.String("event", "request_error"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.Int("status_code", statusCode),
 			zap.String("error_message", errorMessage),
 			zap.String("stacktrace", stacktrace),
 		}
-		d.logger.Error("Error during HTTP request", fields...)
+		d.logger.Error("Error occurred", fields...)
 	}
 }
 
 // LogAuthTokenError logs issues encountered during the authentication token acquisition process.
-func (d *defaultLogger) LogAuthTokenError(method string, url string, statusCode int, err error) {
+func (d *defaultLogger) LogAuthTokenError(event string, method string, url string, statusCode int, err error) {
 	if d.logLevel <= LogLevelError {
 		fields := []zap.Field{
-			zap.String("event", "auth_token_error"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.Int("status_code", statusCode),
@@ -70,10 +70,10 @@ func (d *defaultLogger) LogAuthTokenError(method string, url string, statusCode
 }
 
 // LogRetryAttempt logs a retry attempt for an HTTP request if the current log level permits, including wait duration and the error that triggered the retry.
-func (d *defaultLogger) LogRetryAttempt(method string, url string, attempt int, reason string, waitDuration time.Duration, err error) {
+func (d *defaultLogger) LogRetryAttempt(event string, method string, url string, attempt int, reason string, waitDuration time.Duration, err error) {
 	if d.logLevel <= LogLevelWarn {
 		fields := []zap.Field{
-			zap.String("event", "retry_attempt"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.Int("attempt", attempt),
@@ -86,10 +86,10 @@ func (d *defaultLogger) LogRetryAttempt(method string, url string, attempt int,
 }
 
 // LogRateLimiting logs when an HTTP request is rate-limited, including the HTTP method, URL, the value of the 'Retry-After' header, and the actual wait duration.
-func (d *defaultLogger) LogRateLimiting(method string, url string, retryAfter string, waitDuration time.Duration) {
+func (d *defaultLogger) LogRateLimiting(event string, method string, url string, retryAfter string, waitDuration time.Duration) {
 	if d.logLevel <= LogLevelWarn {
 		fields := []zap.Field{
-			zap.String("event", "rate_limited"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.String("retry_after", retryAfter),
@@ -100,10 +100,10 @@ func (d *defaultLogger) LogRateLimiting(method string, url string, retryAfter st
 }
 
 // LogResponse logs details about an HTTP response if the current log level permits.
-func (d *defaultLogger) LogResponse(method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration) {
+func (d *defaultLogger) LogResponse(event string, method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration) {
 	if d.logLevel <= LogLevelInfo {
 		fields := []zap.Field{
-			zap.String("event", "response_received"),
+			zap.String("event", event),
 			zap.String("method", method),
 			zap.String("url", url),
 			zap.Int("status_code", statusCode),
diff --git a/logger/zaplogger_logger.go b/logger/zaplogger_logger.go
@@ -31,12 +31,14 @@ type Logger interface {
 	Panic(msg string, fields ...zapcore.Field)
 	Fatal(msg string, fields ...zapcore.Field)
 
-	LogRequestStart(requestID string, userID string, method string, url string, headers map[string][]string)
-	LogRequestEnd(method string, url string, statusCode int, duration time.Duration)
-	LogError(method string, url string, statusCode int, err error, stacktrace string)
-	LogRetryAttempt(method string, url string, attempt int, reason string, waitDuration time.Duration, err error)
-	LogRateLimiting(method string, url string, retryAfter string, waitDuration time.Duration)
-	LogResponse(method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration)
+	// Updated method signatures to include the 'event' parameter
+	LogRequestStart(event string, requestID string, userID string, method string, url string, headers map[string][]string)
+	LogRequestEnd(event string, method string, url string, statusCode int, duration time.Duration)
+	LogError(event string, method string, url string, statusCode int, err error, stacktrace string)
+	LogAuthTokenError(event string, method string, url string, statusCode int, err error)
+	LogRetryAttempt(event string, method string, url string, attempt int, reason string, waitDuration time.Duration, err error)
+	LogRateLimiting(event string, method string, url string, retryAfter string, waitDuration time.Duration)
+	LogResponse(event string, method string, url string, statusCode int, responseBody string, responseHeaders map[string][]string, duration time.Duration)
 }
 
 // GetLogLevel returns the current logging level of the logger. This allows for checking the logger's

Original file line number	Diff line number	Diff line change
`@@ -207,7 +207,8 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in`
`207`	`207`	`if apiErr := handleAPIErrorResponse(resp, log); apiErr != nil {`
`208`	`208`	`err = apiErr`
`209`	`209`	`}`
`210`		`- log.LogError(method, endpoint, resp.StatusCode, err, status.TranslateStatusCode(resp))`
	`210`	`+ log.LogError("request_error", method, endpoint, resp.StatusCode, err, status.TranslateStatusCode(resp))`
	`211`	`+`
`211`	`212`	`break`
`212`	`213`	`}`
`213`	`214`	`}`