From 724673ba0b73cbc9d4795e31d2cc941e5027c69c Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:08:43 -0400 Subject: [PATCH] fix: one retry layer for background fetches; CI installs the web requirements; one Discord invite (#657) - BackgroundDataService: the session adapter retried connection errors 3x inside each attempt of the service's own retry loop (up to 16 connection attempts per request on a dead network). The adapter no longer retries; ESPN date chunks, which bypass the loop and skip a failed chunk, get a small connection retry of their own (_ConnectionRetryingSession). - CI installs web_interface/requirements.txt. The brotli header test now checks its intent (core never hand-sets br; requests may advertise it when a decoder is installed) instead of failing whenever brotli is present. - Every Discord link uses the LEDMatrix server's invite (RdrC37rEag). Co-authored-by: Claude Opus 5.5 --- .github/workflows/test.yml | 4 +-- CHANGELOG.md | 4 +++ CODE_OF_CONDUCT.md | 2 +- CONTRIBUTING.md | 2 +- README.md | 2 +- SECURITY.md | 2 +- docs/PLUGIN_DEVELOPMENT_GUIDE.md | 2 +- src/background_data_service.py | 40 +++++++++++++++++++++++++--- test/test_background_data_service.py | 40 ++++++++++++++++++++++++++++ test/test_http_headers.py | 9 ++++++- 10 files changed, 95 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7f91e726..ede4d7ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,7 +35,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install -r requirements.txt -r requirements-test.txt + pip install -r requirements.txt -r web_interface/requirements.txt -r requirements-test.txt pip install RGBMatrixEmulator - name: Run plugin safety harness @@ -58,7 +58,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install -r requirements.txt -r requirements-test.txt + pip install -r requirements.txt -r web_interface/requirements.txt -r requirements-test.txt pip install RGBMatrixEmulator # Run the ENTIRE test tree (except test/plugins, which the diff --git a/CHANGELOG.md b/CHANGELOG.md index 10fa767e..51c48443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +- Background data fetches retry at one level instead of two. The session adapter retried a connection error three times inside every attempt of the service's own retry loop, so a dead network cost up to 16 connection attempts per request and held one of the few worker threads throughout; now it is the loop's `max_retries + 1` attempts. ESPN date-range chunks, which don't go through that loop and skip a chunk that fails, keep a small connection retry of their own so a brief blip doesn't drop a month from a cached season. +- CI installs `web_interface/requirements.txt` too, so flask-limiter, flask-compress and the web floors are tested. `test_api_helper_does_not_hand_set_brotli` now checks what it meant: core doesn't add `br` itself, and `requests` may advertise it when a brotli decoder is installed. +- All Discord links point to the LEDMatrix server's invite. + - Web backend and WiFi fixes: - Plugins installed as `ledmatrix-` (or in a directory not named after their id) work in the installed list, the update button, recorded versions, the plugin config form and plugin web UI pages. Those routes built `plugins_dir/` themselves instead of asking the plugin manager. - The captive-portal checks (`/generate_204` and friends) also detect an access point brought up through NetworkManager, the fallback `enable_ap_mode` uses without hostapd; only hostapd was checked, so phones on that AP were told the internet worked. diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index a1594b71..49a59112 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -63,7 +63,7 @@ ChuckBuilds, and any other forums hosted by or affiliated with the project. Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the community leaders responsible for enforcement on the -[LEDMatrix Discord](https://discord.gg/uW36dVAtcT) (DM a moderator or +[LEDMatrix Discord](https://discord.gg/RdrC37rEag) (DM a moderator or ChuckBuilds directly) or by opening a private GitHub Security Advisory if the issue involves account safety. All complaints will be reviewed and investigated promptly and fairly. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bd1e364d..dd0522ca 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,7 +9,7 @@ improvements, and code changes. - **Bugs / feature requests**: open an issue using one of the templates in [`.github/ISSUE_TEMPLATE/`](.github/ISSUE_TEMPLATE/). - **Real-time discussion**: the - [LEDMatrix Discord](https://discord.gg/uW36dVAtcT). + [LEDMatrix Discord](https://discord.gg/RdrC37rEag). - **Plugin development**: [`docs/PLUGIN_DEVELOPMENT_GUIDE.md`](docs/PLUGIN_DEVELOPMENT_GUIDE.md) and the [`ledmatrix-plugins`](https://github.com/ChuckBuilds/ledmatrix-plugins) diff --git a/README.md b/README.md index d628122e..b642d423 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ I'm trying to be open to constructive criticism and support, as long as it's a r - Show support on Youtube: https://www.youtube.com/@ChuckBuilds - Check out the write-up on my website: https://www.chuck-builds.com/led-matrix/ - Stay in touch on Instagram: https://www.instagram.com/ChuckBuilds/ -- Want to chat? Reach out on the LEDMatrix Discord: [https://discord.com/invite/uW36dVAtcT](https://discord.gg/dfFwsasa6W) +- Want to chat? Reach out on the LEDMatrix Discord: [https://discord.gg/RdrC37rEag](https://discord.gg/RdrC37rEag) - Feeling Generous? Consider sponsoring this project or sending a donation (these AI credits aren't cheap!) ----------------------------------------------------------------------------------- diff --git a/SECURITY.md b/SECURITY.md index 52124d5f..96b8e88c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -16,7 +16,7 @@ Use one of these channels, in order of preference: maintainer. - Direct link: 2. **Discord DM**. Send a direct message to a moderator on the - [LEDMatrix Discord](https://discord.gg/uW36dVAtcT). Don't post in + [LEDMatrix Discord](https://discord.gg/RdrC37rEag). Don't post in public channels. Please include: diff --git a/docs/PLUGIN_DEVELOPMENT_GUIDE.md b/docs/PLUGIN_DEVELOPMENT_GUIDE.md index da844d57..d46de13b 100644 --- a/docs/PLUGIN_DEVELOPMENT_GUIDE.md +++ b/docs/PLUGIN_DEVELOPMENT_GUIDE.md @@ -645,7 +645,7 @@ To have your plugin added to the official plugin store: 3. **Contact maintainers** (own-repository plugins): - Open a GitHub issue in the [ledmatrix-plugins](https://github.com/ChuckBuilds/ledmatrix-plugins) repository - - Or reach out on Discord: https://discord.gg/uW36dVAtcT + - Or reach out on Discord: https://discord.gg/RdrC37rEag - Include: Repository URL, plugin description, why it's useful 4. **Review process**: diff --git a/src/background_data_service.py b/src/background_data_service.py index 0e368b42..bca6a8b5 100644 --- a/src/background_data_service.py +++ b/src/background_data_service.py @@ -103,6 +103,32 @@ class FetchResult: # FAILED, which turns "you cancelled this" into "this errored". final_status: Optional[FetchStatus] = None +class _ConnectionRetryingSession: + """``session.get`` that retries a connection error a few times. + + For ESPN date chunks, which bypass _make_request_with_retry: a failed + chunk is logged and skipped, so a brief network blip would otherwise drop + a month from a cached season. That protection used to come from the + session adapter's own retries, which every other request stacked with the + retry loop. + """ + + ATTEMPTS = 3 + DELAY = 0.5 + + def __init__(self, session): + self._session = session + + def get(self, *args, **kwargs): + for attempt in range(self.ATTEMPTS): + try: + return self._session.get(*args, **kwargs) + except requests.ConnectionError: + if attempt == self.ATTEMPTS - 1: + raise + time.sleep(self.DELAY * (attempt + 1)) + + class BackgroundDataService: """ Background data service for fetching season data without blocking the main thread. @@ -163,10 +189,16 @@ class BackgroundDataService: 'average_fetch_time': 0.0 } - # Session for HTTP requests + # Session for HTTP requests. No retries at the adapter: a fetch goes + # through _make_request_with_retry (max_retries + 1 attempts with + # exponential backoff, logged), and date-range chunks through + # _ConnectionRetryingSession. With the adapter also retrying + # connection errors three times, a dead network cost up to 16 + # connection attempts per request and held one of the few worker + # threads for all of them. self.session = requests.Session() - self.session.mount('http://', requests.adapters.HTTPAdapter(max_retries=3)) - self.session.mount('https://', requests.adapters.HTTPAdapter(max_retries=3)) + self.session.mount('http://', requests.adapters.HTTPAdapter(max_retries=0)) + self.session.mount('https://', requests.adapters.HTTPAdapter(max_retries=0)) # Default headers: core's shared set (real User-Agent, no hand-set # Accept-Encoding) -- see src/common/api_helper.py. @@ -564,7 +596,7 @@ class BackgroundDataService: """ logger.info("Recovering %s %s from a rejected date range", request.sport, request.year) return fetch_espn_date_chunks( - self.session, + _ConnectionRetryingSession(self.session), request.url, params=request.params, headers=request.headers, diff --git a/test/test_background_data_service.py b/test/test_background_data_service.py index c2a7068c..b1725af3 100644 --- a/test/test_background_data_service.py +++ b/test/test_background_data_service.py @@ -389,3 +389,43 @@ class TestPriorityIsAcceptedAndIgnored: rid = service.submit_fetch_request( "nfl", 2026, "http://example.invalid/x", cache_key="k", priority=5) assert service.get_result(rid).cached is True + + +class TestOneRetryLayer: + """Each fetch used to retry at two levels: the session adapter retried a + connection error three times inside every attempt of the service's own + retry loop, so a dead network cost up to 16 connection attempts per + request. Only the loop retries now; date chunks, which bypass it, get a + small connection retry of their own.""" + + def test_the_session_adapters_do_not_retry(self, service): + for prefix in ("http://", "https://"): + assert service.session.get_adapter(prefix).max_retries.total == 0 + + def test_a_chunk_request_retries_a_connection_error(self, monkeypatch): + import requests + from src.background_data_service import _ConnectionRetryingSession + monkeypatch.setattr(bds_module.time, "sleep", lambda s: None) + inner = MagicMock() + inner.get.side_effect = [requests.ConnectionError("blip"), "ok"] + assert _ConnectionRetryingSession(inner).get("http://x") == "ok" + assert inner.get.call_count == 2 + + def test_a_chunk_request_gives_up_after_its_attempts(self, monkeypatch): + import requests + from src.background_data_service import _ConnectionRetryingSession + monkeypatch.setattr(bds_module.time, "sleep", lambda s: None) + inner = MagicMock() + inner.get.side_effect = requests.ConnectionError("down") + with pytest.raises(requests.ConnectionError): + _ConnectionRetryingSession(inner).get("http://x") + assert inner.get.call_count == _ConnectionRetryingSession.ATTEMPTS + + def test_other_errors_are_not_retried(self, monkeypatch): + import requests + from src.background_data_service import _ConnectionRetryingSession + inner = MagicMock() + inner.get.side_effect = requests.Timeout("slow") + with pytest.raises(requests.Timeout): + _ConnectionRetryingSession(inner).get("http://x") + assert inner.get.call_count == 1 diff --git a/test/test_http_headers.py b/test/test_http_headers.py index bc4f55c6..6defc508 100644 --- a/test/test_http_headers.py +++ b/test/test_http_headers.py @@ -34,7 +34,14 @@ class TestSharedHeaders: assert APIHelper().session.headers['User-Agent'] == USER_AGENT def test_api_helper_does_not_hand_set_brotli(self): - assert 'br' not in APIHelper().session.headers.get('Accept-Encoding', '') + # Accept-Encoding is left to requests, which advertises br only when + # a brotli decoder is installed (flask-compress pulls one in, so the + # web requirements do). What must not happen is core adding it by + # hand, advertising a body the client may not be able to read. + import requests + assert 'accept-encoding' not in _lower_keys(DEFAULT_HTTP_HEADERS) + assert (APIHelper().session.headers.get('Accept-Encoding') + == requests.utils.default_headers()['Accept-Encoding']) def test_logo_helper_sends_the_same_user_agent(self): from src.common.logo_helper import LogoHelper