From 8c5ecd9e2a82c38ab86751f79f3098919f67da98 Mon Sep 17 00:00:00 2001 From: Pedro Araujo Date: Mon, 19 Oct 2020 16:44:00 +0100 Subject: [PATCH] Correctly handle HTTP errors in RequestLogPolicy The condition that tests the HTTP status code to decide whether or not to upgrade log severity never sees the 4xx and 5xx responses because they are encapsulated in a `StorageError`. Addresses #214 --- azblob/zc_policy_request_log.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/azblob/zc_policy_request_log.go b/azblob/zc_policy_request_log.go index 0a362ea..622b3cb 100644 --- a/azblob/zc_policy_request_log.go +++ b/azblob/zc_policy_request_log.go @@ -62,15 +62,21 @@ func NewRequestLogPolicyFactory(o RequestLogOptions) pipeline.Factory { logLevel, forceLog = pipeline.LogWarning, true } - if err == nil { // We got a response from the service - sc := response.Response().StatusCode - if ((sc >= 400 && sc <= 499) && sc != http.StatusNotFound && sc != http.StatusConflict && sc != http.StatusPreconditionFailed && sc != http.StatusRequestedRangeNotSatisfiable) || (sc >= 500 && sc <= 599) { - logLevel, forceLog = pipeline.LogError, true // Promote to Error any 4xx (except those listed is an error) or any 5xx + var sc int + if err == nil { // We got a valid response from the service + sc = response.Response().StatusCode + } else { // We got an error, so we should inspect if we got a response + if r := err.(StorageError).Response(); r != nil { + sc = r.StatusCode } else { - // For other status codes, we leave the level as is. + sc = -1 } - } else { // This error did not get an HTTP response from the service; upgrade the severity to Error - logLevel, forceLog = pipeline.LogError, true + } + + if ((sc >= 400 && sc <= 499) && sc != http.StatusNotFound && sc != http.StatusConflict && sc != http.StatusPreconditionFailed && sc != http.StatusRequestedRangeNotSatisfiable) || (sc >= 500 && sc <= 599) || sc == -1 { + logLevel, forceLog = pipeline.LogError, true // Promote to Error any 4xx (except those listed is an error) or any 5xx + } else { + // For other status codes, we leave the level as is. } if shouldLog := po.ShouldLog(logLevel); forceLog || shouldLog {