Skip to content

Commit

Permalink
Correctly handle HTTP errors in RequestLogPolicy
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pharaujo committed Nov 3, 2020
1 parent 48358e1 commit 8c5ecd9
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions azblob/zc_policy_request_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 8c5ecd9

Please sign in to comment.