From 6c539b847f4068155be1db207f96567b4e4d6f3e Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Tue, 20 Feb 2024 08:50:35 +0000 Subject: [PATCH 1/2] Fix non-retryable error check in executeRequestWithRetries function --- errors/http_error_handling.go | 97 +++++++++++++++++++++++--------- httpclient/httpclient_request.go | 2 +- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/errors/http_error_handling.go b/errors/http_error_handling.go index 6a0b18a..3403890 100644 --- a/errors/http_error_handling.go +++ b/errors/http_error_handling.go @@ -76,21 +76,43 @@ func HandleAPIError(resp *http.Response, log logger.Logger) error { // TranslateStatusCode provides a human-readable message for HTTP status codes. func TranslateStatusCode(statusCode int) string { messages := map[int]string{ - http.StatusOK: "Request successful.", - http.StatusCreated: "Request to create or update resource successful.", - http.StatusAccepted: "The request was accepted for processing, but the processing has not completed.", - http.StatusNoContent: "Request successful. Resource successfully deleted.", - http.StatusBadRequest: "Bad request. Verify the syntax of the request.", - http.StatusUnauthorized: "Authentication failed. Verify the credentials being used for the request.", - http.StatusForbidden: "Invalid permissions. Verify the account being used has the proper permissions for the resource you are trying to access.", - http.StatusNotFound: "Resource not found. Verify the URL path is correct.", - http.StatusConflict: "Conflict. See the error response for additional details.", - http.StatusPreconditionFailed: "Precondition failed. See error description for additional details.", - http.StatusRequestEntityTooLarge: "Payload too large.", - http.StatusRequestURITooLong: "Request-URI too long.", - http.StatusInternalServerError: "Internal server error. Retry the request or contact support if the error persists.", - http.StatusBadGateway: "Bad Gateway. Generally due to a timeout issue.", - http.StatusServiceUnavailable: "Service unavailable.", + http.StatusOK: "Request successful.", + http.StatusCreated: "Request to create or update resource successful.", + http.StatusAccepted: "The request was accepted for processing, but the processing has not completed.", + http.StatusNoContent: "Request successful. No content to send for this request.", + http.StatusBadRequest: "Bad request. Verify the syntax of the request.", + http.StatusUnauthorized: "Authentication failed. Verify the credentials being used for the request.", + http.StatusPaymentRequired: "Payment required. Access to the requested resource requires payment.", + http.StatusForbidden: "Invalid permissions. Verify the account has the proper permissions for the resource.", + http.StatusNotFound: "Resource not found. Verify the URL path is correct.", + http.StatusMethodNotAllowed: "Method not allowed. The method specified is not allowed for the resource.", + http.StatusNotAcceptable: "Not acceptable. The server cannot produce a response matching the list of acceptable values.", + http.StatusProxyAuthRequired: "Proxy authentication required. You must authenticate with a proxy server before this request can be served.", + http.StatusRequestTimeout: "Request timeout. The server timed out waiting for the request.", + http.StatusConflict: "Conflict. The request could not be processed because of conflict in the request.", + http.StatusGone: "Gone. The resource requested is no longer available and will not be available again.", + http.StatusLengthRequired: "Length required. The request did not specify the length of its content, which is required by the requested resource.", + http.StatusPreconditionFailed: "Precondition failed. The server does not meet one of the preconditions specified in the request.", + http.StatusRequestEntityTooLarge: "Payload too large. The request is larger than the server is willing or able to process.", + http.StatusRequestURITooLong: "Request-URI too long. The URI provided was too long for the server to process.", + http.StatusUnsupportedMediaType: "Unsupported media type. The request entity has a media type which the server or resource does not support.", + http.StatusRequestedRangeNotSatisfiable: "Requested range not satisfiable. The client has asked for a portion of the file, but the server cannot supply that portion.", + http.StatusExpectationFailed: "Expectation failed. The server cannot meet the requirements of the Expect request-header field.", + http.StatusUnprocessableEntity: "Unprocessable entity. The server understands the content type and syntax of the request but was unable to process the contained instructions.", + http.StatusLocked: "Locked. The resource that is being accessed is locked.", + http.StatusFailedDependency: "Failed dependency. The request failed because it depended on another request and that request failed.", + http.StatusUpgradeRequired: "Upgrade required. The client should switch to a different protocol.", + http.StatusPreconditionRequired: "Precondition required. The server requires that the request be conditional.", + http.StatusTooManyRequests: "Too many requests. The user has sent too many requests in a given amount of time.", + http.StatusRequestHeaderFieldsTooLarge: "Request header fields too large. The server is unwilling to process the request because its header fields are too large.", + http.StatusUnavailableForLegalReasons: "Unavailable for legal reasons. The server is denying access to the resource as a consequence of a legal demand.", + http.StatusInternalServerError: "Internal server error. The server encountered an unexpected condition that prevented it from fulfilling the request.", + http.StatusNotImplemented: "Not implemented. The server does not support the functionality required to fulfill the request.", + http.StatusBadGateway: "Bad gateway. The server received an invalid response from the upstream server while trying to fulfill the request.", + http.StatusServiceUnavailable: "Service unavailable. The server is currently unable to handle the request due to temporary overloading or maintenance.", + http.StatusGatewayTimeout: "Gateway timeout. The server did not receive a timely response from the upstream server.", + http.StatusHTTPVersionNotSupported: "HTTP version not supported. The server does not support the HTTP protocol version used in the request.", + http.StatusNetworkAuthenticationRequired: "Network authentication required. The client needs to authenticate to gain network access.", } if message, exists := messages[statusCode]; exists { @@ -99,17 +121,34 @@ func TranslateStatusCode(statusCode int) string { return "An unexpected error occurred. Please try again later." } -// IsNonRetryableError checks if the provided response indicates a non-retryable error. -func IsNonRetryableError(resp *http.Response) bool { - // List of non-retryable HTTP status codes +// IsNonRetryableStatusCode checks if the provided response indicates a non-retryable error. +func IsNonRetryableStatusCode(resp *http.Response) bool { + // Expanded list of non-retryable HTTP status codes nonRetryableStatusCodes := map[int]bool{ - http.StatusBadRequest: true, // 400 - http.StatusUnauthorized: true, // 401 - http.StatusForbidden: true, // 403 - http.StatusNotFound: true, // 404 - http.StatusConflict: true, // 409 - http.StatusRequestEntityTooLarge: true, // 413 - http.StatusRequestURITooLong: true, // 414 + http.StatusBadRequest: true, // 400 - Bad Request + http.StatusUnauthorized: true, // 401 - Unauthorized + http.StatusPaymentRequired: true, // 402 - Payment Required + http.StatusForbidden: true, // 403 - Forbidden + http.StatusNotFound: true, // 404 - Not Found + http.StatusMethodNotAllowed: true, // 405 - Method Not Allowed + http.StatusNotAcceptable: true, // 406 - Not Acceptable + http.StatusProxyAuthRequired: true, // 407 - Proxy Authentication Required + http.StatusConflict: true, // 409 - Conflict + http.StatusGone: true, // 410 - Gone + http.StatusLengthRequired: true, // 411 - Length Required + http.StatusPreconditionFailed: true, // 412 - Precondition Failed + http.StatusRequestEntityTooLarge: true, // 413 - Request Entity Too Large + http.StatusRequestURITooLong: true, // 414 - Request-URI Too Long + http.StatusUnsupportedMediaType: true, // 415 - Unsupported Media Type + http.StatusRequestedRangeNotSatisfiable: true, // 416 - Requested Range Not Satisfiable + http.StatusExpectationFailed: true, // 417 - Expectation Failed + http.StatusUnprocessableEntity: true, // 422 - Unprocessable Entity + http.StatusLocked: true, // 423 - Locked + http.StatusFailedDependency: true, // 424 - Failed Dependency + http.StatusUpgradeRequired: true, // 426 - Upgrade Required + http.StatusPreconditionRequired: true, // 428 - Precondition Required + http.StatusRequestHeaderFieldsTooLarge: true, // 431 - Request Header Fields Too Large + http.StatusUnavailableForLegalReasons: true, // 451 - Unavailable For Legal Reasons } _, isNonRetryable := nonRetryableStatusCodes[resp.StatusCode] @@ -124,9 +163,10 @@ func IsRateLimitError(resp *http.Response) bool { // IsTransientError checks if an error or HTTP response indicates a transient error. func IsTransientError(resp *http.Response) bool { transientStatusCodes := map[int]bool{ - http.StatusInternalServerError: true, - http.StatusBadGateway: true, - http.StatusServiceUnavailable: true, + http.StatusInternalServerError: true, // 500 Internal Server Error + http.StatusBadGateway: true, // 502 Bad Gateway + http.StatusServiceUnavailable: true, // 503 Service Unavailable + http.StatusGatewayTimeout: true, // 504 - Gateway Timeout } return resp != nil && transientStatusCodes[resp.StatusCode] } @@ -134,6 +174,7 @@ func IsTransientError(resp *http.Response) bool { // IsRetryableStatusCode checks if the provided HTTP status code is considered retryable. func IsRetryableStatusCode(statusCode int) bool { retryableStatusCodes := map[int]bool{ + http.StatusRequestTimeout: true, // 408 - Request Timeout http.StatusTooManyRequests: true, // 429 http.StatusInternalServerError: true, // 500 http.StatusBadGateway: true, // 502 diff --git a/httpclient/httpclient_request.go b/httpclient/httpclient_request.go index c810bb2..e79576c 100644 --- a/httpclient/httpclient_request.go +++ b/httpclient/httpclient_request.go @@ -162,7 +162,7 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in return resp, c.handleSuccessResponse(resp, out, log, method, endpoint) } // Check for non-retryable errors - if resp != nil && errors.IsNonRetryableError(resp) { + if resp != nil && errors.IsNonRetryableStatusCode(resp) { log.Info("Non-retryable error received", zap.Int("status_code", resp.StatusCode)) return resp, errors.HandleAPIError(resp, log) } From a388d946bbd5475aeb1a2a6fcbaa426cbf67a8c4 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Tue, 20 Feb 2024 09:01:10 +0000 Subject: [PATCH 2/2] Add status message to log for non-retryable errors --- httpclient/httpclient_request.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/httpclient/httpclient_request.go b/httpclient/httpclient_request.go index e79576c..5433eb5 100644 --- a/httpclient/httpclient_request.go +++ b/httpclient/httpclient_request.go @@ -161,11 +161,16 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in if err == nil && resp.StatusCode >= 200 && resp.StatusCode < 300 { return resp, c.handleSuccessResponse(resp, out, log, method, endpoint) } + + // Leverage TranslateStatusCode for more descriptive error logging + statusMessage := errors.TranslateStatusCode(resp.StatusCode) + // Check for non-retryable errors if resp != nil && errors.IsNonRetryableStatusCode(resp) { - log.Info("Non-retryable error received", zap.Int("status_code", resp.StatusCode)) + log.Info("Non-retryable error received", zap.Int("status_code", resp.StatusCode), zap.String("status_message", statusMessage)) return resp, errors.HandleAPIError(resp, log) } + // Check for retryable errors if errors.IsRateLimitError(resp) || errors.IsTransientError(resp) { retryCount++ @@ -174,15 +179,17 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in break } waitDuration := calculateBackoff(retryCount) - log.Warn("Retrying request due to error", zap.String("method", method), zap.String("endpoint", endpoint), zap.Int("retryCount", retryCount), zap.Duration("waitDuration", waitDuration), zap.Error(err)) + log.Warn("Retrying request due to error", zap.String("method", method), zap.String("endpoint", endpoint), zap.Int("retryCount", retryCount), zap.Duration("waitDuration", waitDuration), zap.Error(err), zap.String("status_message", statusMessage)) time.Sleep(waitDuration) continue } + // Handle error responses if err != nil || !errors.IsRetryableStatusCode(resp.StatusCode) { if apiErr := errors.HandleAPIError(resp, log); apiErr != nil { err = apiErr } + log.Error("API error", zap.String("status_message", statusMessage), zap.Error(err)) break } }