From 86752e9f665060cb5d85aefe90e0bfe6bdc2c9e1 Mon Sep 17 00:00:00 2001 From: Zio Gabber <78922322+Gabrymi93@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:59:41 +0100 Subject: [PATCH] fix(sparql): fallback GET anche su timeout/rete + errore esplicito su GET 5xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completa il fix #447 (POST 403 → GET fallback) con i casi che il docstring prometteva ma il codice non copriva: - POST con errore di rete/timeout (post_status None) → prova GET fallback - GET fallback che restituisce 5xx → DownloadError esplicito con status Test: 4 casi coperti (403→GET, timeout→GET, GET 500→errore, POST 500→errore). 26/26 pass, mypy ok, ruff ok. End-to-end sul Senato reale: OK. --- tests/test_sparql_plugin.py | 44 +++++++++++++++++++++++++++++++++++++ toolkit/plugins/sparql.py | 15 +++++++++---- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/tests/test_sparql_plugin.py b/tests/test_sparql_plugin.py index 66048b7..7d10c02 100644 --- a/tests/test_sparql_plugin.py +++ b/tests/test_sparql_plugin.py @@ -113,6 +113,50 @@ def test_sparql_fetch_post_403_falls_back_to_get(): assert mock_cls.return_value.get.called # il fallback GET è scattato +def test_sparql_fetch_get_fallback_5xx_raises(): + """POST 403 → GET fallback 500 → DownloadError con status del GET. + + Il fallback GET non deve mascherare un errore server del GET stesso. + """ + with patch("toolkit.plugins.sparql.HttpClient") as mock_cls: + mock_cls.return_value.post.return_value = _http_ok( + status=403, + text="Forbidden", + ) + mock_cls.return_value.get.return_value = _http_ok( + status=500, + text="Internal Server Error", + ) + source = SparqlSource() + with pytest.raises(DownloadError, match="GET fallback returned HTTP 500"): + source.fetch("https://example.test/sparql", "SELECT * WHERE { }") + """POST con errore di rete/timeout (response None) deve cadere sul GET. + + Estensione del fix 403: il docstring prometteva il fallback anche su + timeout — il POST con err (connection refused) produce post_status=None + e prima del fix andava dritto al raise senza provare il GET. + """ + from lab_connectors.http import HttpResult + + with patch("toolkit.plugins.sparql.HttpClient") as mock_cls: + mock_cls.return_value.post.return_value = HttpResult( + response=None, err=TimeoutError("connection refused") + ) + mock_cls.return_value.get.return_value = _http_ok( + status=200, + text="name,value\nfoo,123\n", + headers={"Content-Type": "text/csv"}, + ) + source = SparqlSource() + payload, origin = source.fetch( + "https://dati.senato.it/sparql", + "SELECT ?name ?value WHERE { }", + accept_format="csv", + ) + assert b"foo" in payload + assert mock_cls.return_value.get.called # il fallback GET è scattato + + def test_sparql_fetch_network_error(): """Network error raises DownloadError.""" with patch("toolkit.plugins.sparql.HttpClient") as mock_cls: diff --git a/toolkit/plugins/sparql.py b/toolkit/plugins/sparql.py index de749fc..a203528 100644 --- a/toolkit/plugins/sparql.py +++ b/toolkit/plugins/sparql.py @@ -50,16 +50,18 @@ def _do_fetch(self, endpoint: str, q: str, accept_format: str) -> bytes: ) # is_ok è True anche su errori HTTP (response presente, err None) — # ma una risposta 4xx/5xx NON è un risultato SPARQL valido. - # Fallback GET solo su 4xx (WAF/Virtuoso rifiutano POST): 403 è il - # caso tipico. Su 5xx il fallback non deve mascherare l'errore server. + # Fallback GET quando il POST non produce dati validi: + # - 4xx (WAF/Virtuoso rifiutano POST, es. 403 Senato) + # - errore di rete/timeout (post_status None) + # Su 5xx NON si fa fallback (errore server reale, non va mascherato). post_status = ( result.response.status_code if result.is_ok and result.response is not None else None ) if post_status is not None and post_status < 400: return self._parse_response(result.response, is_json) - # --- Tentativo 2: GET fallback (solo se POST è fallito con 4xx) --- - if post_status is not None and 400 <= post_status < 500: + # --- Tentativo 2: GET fallback (4xx o errore di rete/timeout) --- + if post_status is None or 400 <= post_status < 500: url = f"{endpoint}?query={urllib.parse.quote(q)}" get_headers = { "Accept": ( @@ -75,6 +77,11 @@ def _do_fetch(self, endpoint: str, q: str, accept_format: str) -> bytes: ) if get_status is not None and get_status < 400: return self._parse_response(result.response, is_json) + if get_status is not None and get_status >= 500: + body = (result.response.text or "")[:200] if result.response is not None else "" + raise DownloadError( + f"SPARQL GET fallback returned HTTP {get_status} for {endpoint}: {body}" + ) if post_status is not None and post_status >= 500: body = (result.response.text or "")[:200] if result.response is not None else ""