-
Notifications
You must be signed in to change notification settings - Fork 68
Add debug logging messages on retry #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5a94611
5d69160
ba7a65a
c4741ba
f960817
f5c8cc7
95ccc75
d8a00dd
5b62567
5b6c716
fcc394b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -110,6 +110,19 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str | |||||||||
| return nothing | ||||||||||
| end | ||||||||||
|
|
||||||||||
| check = (s, e) -> begin | ||||||||||
| if isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500 | ||||||||||
| @debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=AWS._http_status(e) exception=e | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||
| return true | ||||||||||
| elseif isa(e, Downloads.RequestError) | ||||||||||
| @debug "AWS.jl Downloads inner retry for Downloads.RequestError" retry=true reason="Downloads.RequestError" exception=e | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||
| return true | ||||||||||
| else | ||||||||||
| @debug "AWS.jl Downloads inner retry declined" retry=false reason="Exception passed onwards" exception=e | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||
| return false | ||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| try | ||||||||||
| retry(get_response; check=check, delays=delays)() | ||||||||||
| finally | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -147,52 +147,71 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers | |||||||||||||||||||||||||||||||||||||||||||||||
| if e isa HTTP.StatusError | ||||||||||||||||||||||||||||||||||||||||||||||||
| e = AWSException(e, stream) | ||||||||||||||||||||||||||||||||||||||||||||||||
| rethrow(e) | ||||||||||||||||||||||||||||||||||||||||||||||||
| elseif !(e isa AWSException) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
| rethrow() | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| check = function (s, e) | ||||||||||||||||||||||||||||||||||||||||||||||||
| check = (s, e) -> begin | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Pass on non-AWS exceptions. | ||||||||||||||||||||||||||||||||||||||||||||||||
| if !(e isa AWSException) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+157
to
162
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| occursin("Signature expired", e.message) && return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| if occursin("Signature expired", e.message) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry for signature expired" retry=true reason="Signature expired" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+164
to
+167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Handle ExpiredToken... | ||||||||||||||||||||||||||||||||||||||||||||||||
| # https://github.com/aws/aws-sdk-go/blob/v1.31.5/aws/request/retryer.go#L98 | ||||||||||||||||||||||||||||||||||||||||||||||||
| if e isa AWSException && e.code in EXPIRED_ERROR_CODES | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry for expired credentials" retry=true reason="Credentials expired" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| check_credentials(credentials(aws); force_refresh=true) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
169
to
175
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Throttle handling | ||||||||||||||||||||||||||||||||||||||||||||||||
| # https://github.com/boto/botocore/blob/1.16.17/botocore/data/_retry.json | ||||||||||||||||||||||||||||||||||||||||||||||||
| # https://docs.aws.amazon.com/general/latest/gr/api-retries.html | ||||||||||||||||||||||||||||||||||||||||||||||||
| if _http_status(e.cause) == TOO_MANY_REQUESTS || e.code in THROTTLING_ERROR_CODES | ||||||||||||||||||||||||||||||||||||||||||||||||
| if _http_status(e.cause) == TOO_MANY_REQUESTS | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry too many requests" retry=true reason="too many requests" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| elseif e.code in THROTTLING_ERROR_CODES | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry for throttling" retry=true reason="throttled" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
177
to
186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Handle BadDigest error and CRC32 check sum failure | ||||||||||||||||||||||||||||||||||||||||||||||||
| if _header(e.cause, "crc32body") == "x-amz-crc32" || | ||||||||||||||||||||||||||||||||||||||||||||||||
| e.code in ("BadDigest", "RequestTimeout", "RequestTimeoutException") | ||||||||||||||||||||||||||||||||||||||||||||||||
| if _header(e.cause, "crc32body") == "x-amz-crc32" | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry for check sum failure" retry=true reason="Check sum failure" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
188
to
+192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if e.code in ("BadDigest", "RequestTimeout", "RequestTimeoutException") | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl retry for $(e.code)" retry=true reason="$(e.code)" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+194
to
197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if occursin("Missing Authentication Token", e.message) && | ||||||||||||||||||||||||||||||||||||||||||||||||
| aws.credentials === nothing | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl declined to retry request without credentials" retry=false reason="No credentials" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
199
to
+201
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return throw( | ||||||||||||||||||||||||||||||||||||||||||||||||
| NoCredentials( | ||||||||||||||||||||||||||||||||||||||||||||||||
| "You're attempting to perform a request without credentials set." | ||||||||||||||||||||||||||||||||||||||||||||||||
| "You're attempting to perform a request without credentials set.", | ||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
203
to
208
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl declined to retry uncaught error" retry=false reason="Error unhandled" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+210
to
212
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| delays = AWSExponentialBackoff(; max_attempts=max_attempts(aws)) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| retry(upgrade_error(get_response); check=check, delays=delays)() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if request.use_response_type | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -234,14 +253,24 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str | |||||||||||||||||||||||||||||||||||||||||||||||
| return nothing | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| check = function (s, e) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return isa(e, HTTP.ConnectError) || | ||||||||||||||||||||||||||||||||||||||||||||||||
| isa(e, HTTP.RequestError) || | ||||||||||||||||||||||||||||||||||||||||||||||||
| (isa(e, HTTP.StatusError) && _http_status(e) >= 500) | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| delays = AWSExponentialBackoff(; max_attempts=4) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| check = (s, e) -> begin | ||||||||||||||||||||||||||||||||||||||||||||||||
| errors = (Sockets.DNSError, HTTP.ParseError, Base.IOError) | ||||||||||||||||||||||||||||||||||||||||||||||||
| for error in errors | ||||||||||||||||||||||||||||||||||||||||||||||||
| if isa(e, error) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl HTTP inner retry for $error" retry=true reason="$error" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+258
to
+262
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if (isa(e, HTTP.StatusError) && _http_status(e) >= 500) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl HTTP inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
| @debug "AWS.jl HTTP inner retry declined" retry=false reason="Exception passed onwards" exception=e | ||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+266
to
+272
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||||||||||||||||||||||
| retry(get_response; check=check, delays=delays)() | ||||||||||||||||||||||||||||||||||||||||||||||||
| finally | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,14 @@ | |||||||
|
|
||||||||
| BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() | ||||||||
|
|
||||||||
| # We try to be insensitive to the existence of other logging messages, so we test | ||||||||
| # by counting the number of messages that meet some criteria | ||||||||
| function log_is_retry(successful) | ||||||||
| return record -> | ||||||||
| record.level == Logging.Debug && | ||||||||
| get(record.kwargs, :retry, nothing) == successful | ||||||||
|
Comment on lines
+9
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [JuliaFormatter] reported by reviewdog 🐶
Suggested change
|
||||||||
| end | ||||||||
|
|
||||||||
| try | ||||||||
| S3.create_bucket(BUCKET_NAME) | ||||||||
|
|
||||||||
|
|
@@ -202,12 +210,24 @@ try | |||||||
| config = AWSConfig(; creds=nothing) | ||||||||
|
|
||||||||
| @testset "Fail 2 attempts then succeed" begin | ||||||||
| apply(_incomplete_patch(; data=data, num_attempts_to_fail=2)) do | ||||||||
| retrieved = S3.get_object(bucket, key; aws_config=config) | ||||||||
| logger = Test.TestLogger(; min_level=Logging.Debug) | ||||||||
| with_logger(logger) do | ||||||||
| apply(_incomplete_patch(; data=data, num_attempts_to_fail=2)) do | ||||||||
| retrieved = S3.get_object(bucket, key; aws_config=config) | ||||||||
|
|
||||||||
| @test length(retrieved) == n | ||||||||
| @test retrieved == data | ||||||||
| @test length(retrieved) == n | ||||||||
| @test retrieved == data | ||||||||
| end | ||||||||
| end | ||||||||
| logs = logger.logs | ||||||||
|
|
||||||||
| println("Fail two ") | ||||||||
| println(logs) | ||||||||
|
|
||||||||
| # Two successful retries | ||||||||
| @test count(log_is_retry(true), logs) == 2 | ||||||||
| # No unsuccessful ones | ||||||||
| @test count(log_is_retry(false), logs) == 0 | ||||||||
| end | ||||||||
|
|
||||||||
| @testset "Fail all 4 attempts then throw" begin | ||||||||
|
|
@@ -218,15 +238,27 @@ try | |||||||
| end | ||||||||
| io = IOBuffer() | ||||||||
|
|
||||||||
| apply(_incomplete_patch(; data=data, num_attempts_to_fail=4)) do | ||||||||
| params = Dict("response_stream" => io) | ||||||||
| @test_throws err_t S3.get_object(bucket, key, params; aws_config=config) | ||||||||
| logger = Test.TestLogger(; min_level=Logging.Debug) | ||||||||
| with_logger(logger) do | ||||||||
| apply(_incomplete_patch(; data=data, num_attempts_to_fail=4)) do | ||||||||
| params = Dict("response_stream" => io) | ||||||||
| @test_throws err_t S3.get_object(bucket, key, params; aws_config=config) | ||||||||
|
|
||||||||
| seekstart(io) | ||||||||
| retrieved = read(io) | ||||||||
| @test length(retrieved) == n - 1 | ||||||||
| @test retrieved == data[1:(n - 1)] | ||||||||
| seekstart(io) | ||||||||
| retrieved = read(io) | ||||||||
| @test length(retrieved) == n - 1 | ||||||||
| @test retrieved == data[1:(n - 1)] | ||||||||
| end | ||||||||
| end | ||||||||
| logs = logger.logs | ||||||||
|
|
||||||||
| println("Fail 4") | ||||||||
| println(logs) | ||||||||
|
|
||||||||
| # Three successful retries - from the inner retry loop | ||||||||
| @test count(log_is_retry(true), logs) == 3 | ||||||||
| # One unsuccessful one - from the outer loop where we pass it on | ||||||||
| @test count(log_is_retry(false), logs) == 1 | ||||||||
| end | ||||||||
| end | ||||||||
|
|
||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaFormatter] reported by reviewdog 🐶