Skip to content
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

new: retry on common errors #505

Merged
merged 14 commits into from
Aug 14, 2023
Merged
8 changes: 8 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,14 @@ in addition to its normal output. If these warnings can interfere with your
scripts or you otherwise want them disabled, simply add the ``--suppress-warnings``
flag to prevent them from being emitted.

Suppressing Warnings
""""""""""""""""""""

Sometimes the API responds with a error that can be ignored. For example a timeout
or nginx response that can't be parsed correctly, by default the CLI will retry
calls on these errors we've identified. If you'd like to disable this behavior for
any reason use the ``--no-retry`` flag.

Shell Completion
""""""""""""""""

Expand Down
3 changes: 2 additions & 1 deletion linodecli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
)
from .cli import CLI
from .completion import bake_completions, get_completions
from .configuration import ENV_TOKEN_NAME
from .helpers import handle_url_overrides
from .output import OutputMode

Expand Down Expand Up @@ -83,6 +82,8 @@ def main(): # pylint: disable=too-many-branches,too-many-statements
cli.output_handler.columns = parsed.format

cli.defaults = not parsed.no_defaults
cli.retry_count = 0
cli.no_retry = parsed.no_retry
cli.suppress_warnings = parsed.suppress_warnings

if parsed.all_columns or parsed.all:
Expand Down
31 changes: 31 additions & 0 deletions linodecli/api_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import itertools
import json
import sys
import time
from sys import version_info
from typing import Iterable, List, Optional

Expand Down Expand Up @@ -92,6 +93,11 @@ def do_request(
if ctx.debug_request:
_print_response_debug_info(result)

if _check_retry(result) and ctx.no_retry and ctx.retry_count < 3:
time.sleep(_get_retry_after(result.headers))
ctx.retry_count += 1
do_request(ctx, operation, args, filter_header, skip_error_handling)

_attempt_warn_old_version(ctx, result)

if not 199 < result.status_code < 399 and not skip_error_handling:
Expand Down Expand Up @@ -361,3 +367,28 @@ def _handle_error(ctx, response):
columns=["field", "reason"],
)
sys.exit(1)


def _check_retry(response):
"""
Check for valid retry scenario, returns true if retry is valid
"""
if response.status_code in (408, 429):
# request timed out or rate limit exceeded
return True

if (
response.headers
and response.status_code == 400
and response.headers.get("Server") == "nginx"
and response.headers.get("Content-Type") == "text/html"
):
# nginx html response
return True

return False


def _get_retry_after(headers):
retry_str = headers.get("Retry-After", "")
return int(retry_str) if retry_str else 0
5 changes: 5 additions & 0 deletions linodecli/arg_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ def register_args(parser):
default=False,
help="Prevent the truncation of long values in command outputs.",
)
parser.add_argument(
"--no-retry",
action="store_true",
help="Skip retrying on common errors like timeouts.",
)
parser.add_argument(
"--column-width",
type=int,
Expand Down
45 changes: 45 additions & 0 deletions tests/unit/test_api_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from types import SimpleNamespace
from unittest.mock import Mock, patch

import pytest
import requests

from linodecli import api_request
Expand Down Expand Up @@ -475,3 +476,47 @@ def json_func():

output = stderr_buf.getvalue()
assert "" == output

def test_do_request_recursion(self, mock_cli, list_operation):
mock_response = Mock(status_code=408)
with patch(
"linodecli.api_request.requests.get", return_value=mock_response
) and pytest.raises(SystemExit):
_ = api_request.do_request(mock_cli, list_operation, None)
assert mock_cli.retry_count == 3

def test_check_retry(self):
mock_response = Mock(status_code=200)
output = api_request._check_retry(mock_response)
assert not output

mock_response = Mock(status_code=408)
output = api_request._check_retry(mock_response)
assert output

mock_response = Mock(status_code=429)
output = api_request._check_retry(mock_response)
assert output

mock_response = Mock(
status_code=400,
headers={
"Server": "nginx",
"Content-Type": "text/html",
},
)
output = api_request._check_retry(mock_response)
assert output

def test_get_retry_after(self):
headers = {"Retry-After": "10"}
output = api_request._get_retry_after(headers)
assert output == 10

headers = {"Retry-After": ""}
output = api_request._get_retry_after(headers)
assert output == 0

headers = {}
output = api_request._get_retry_after(headers)
assert output == 0
8 changes: 7 additions & 1 deletion tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@

class MockResponse:
def __init__(
self, page: int, pages: int, results: int, status_code: int = 200
self,
page: int,
pages: int,
results: int,
status_code: int = 200,
headers: dict = None,
):
self.page = page
self.pages = pages
self.headers = headers
self.results = results
self.status_code = status_code

Expand Down