Skip to content

Commit

Permalink
Fix infinite hang on schema refresh by adding timeout parameter (#2895)
Browse files Browse the repository at this point in the history
Co-authored-by: Anders Forsman <anders.forsman@csc.fi>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Jan 16, 2023
1 parent dc42168 commit cf44c41
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 745
PYTEST_REQPASS: 747

steps:
- name: Activate WSL1
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def refresh_schemas(min_age_seconds: int = 3600 * 24) -> int:
if etag:
request.add_header("If-None-Match", f'"{data.get("etag")}"')
try:
with urllib.request.urlopen(request) as response:
with urllib.request.urlopen(request, timeout=10) as response:
if response.status == 200:
content = response.read().decode("utf-8").rstrip()
etag = response.headers["etag"].strip('"')
Expand Down
33 changes: 33 additions & 0 deletions test/test_schemas.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
"""Test schemas modules."""
import logging
import urllib
from time import sleep
from typing import Any
from unittest.mock import DEFAULT, MagicMock, patch

import pytest

Expand Down Expand Up @@ -42,3 +46,32 @@ def test_schema(
assert len(matches) == len(expected_tags)
for i, match in enumerate(matches):
assert match.tag == expected_tags[i]


def urlopen_side_effect(*_args: Any, **kwargs: Any) -> DEFAULT:
"""Actual test that timeout parameter is defined."""
assert "timeout" in kwargs
assert kwargs["timeout"] > 0
return DEFAULT


@patch("urllib.request")
def test_requests_uses_timeout(mock_request: MagicMock) -> None:
"""Test that schema refresh uses timeout."""
mock_request.urlopen.side_effect = urlopen_side_effect
refresh_schemas(min_age_seconds=0)
mock_request.urlopen.assert_called()


@patch("urllib.request")
def test_request_timeouterror_handling(
mock_request: MagicMock, caplog: pytest.LogCaptureFixture
) -> None:
"""Test that schema refresh can handle time out errors."""
error_msg = "Simulating handshake operation time out."
mock_request.urlopen.side_effect = urllib.error.URLError(TimeoutError(error_msg))
with caplog.at_level(logging.DEBUG):
assert refresh_schemas(min_age_seconds=0) == 1
mock_request.urlopen.assert_called()
assert "Skipped schema refresh due to unexpected exception: " in caplog.text
assert error_msg in caplog.text

0 comments on commit cf44c41

Please sign in to comment.