From 593a85062ad69e08975aac05daf29d0da1fe54a1 Mon Sep 17 00:00:00 2001 From: Dariusz L Date: Sun, 9 Aug 2026 00:14:20 +0200 Subject: [PATCH] refactor provider status message contract --- core/services/search_providers.py | 12 +---- core/sources/civarchive.py | 53 +++++++++++++------- tests/test_civarchive_failure_contract.py | 14 ++++++ tests/test_search_source_methods.mjs | 12 ++--- web/resolver/search/search_source_methods.js | 33 +++++------- 5 files changed, 68 insertions(+), 56 deletions(-) diff --git a/core/services/search_providers.py b/core/services/search_providers.py index 19b8366..ee63bac 100644 --- a/core/services/search_providers.py +++ b/core/services/search_providers.py @@ -3,6 +3,7 @@ from dataclasses import dataclass from typing import Any +from ..sources.civarchive import build_civarchive_failure_status from ..sources.civitai import build_civitai_result_payload from ..type_utils import select_primary_model_file @@ -532,16 +533,7 @@ class SearchProviderRunner: if callable(status_factory): source_status = status_factory() else: - source_status = { - "state": "unavailable", - "code": "provider_unavailable", - "retryable": True, - "http_status": None, - "message": ( - "CivArchive may be overloaded or temporarily unavailable. " - "Please try again." - ), - } + source_status = build_civarchive_failure_status() http_status = source_status.get("http_status") or "none" retryable = "yes" if source_status.get("retryable") else "no" error_type = type(error).__name__ diff --git a/core/sources/civarchive.py b/core/sources/civarchive.py index ff8b056..a5ee84a 100644 --- a/core/sources/civarchive.py +++ b/core/sources/civarchive.py @@ -93,6 +93,35 @@ CIVARCHIVE_FAILURE_MESSAGES = { "not_found": "CivArchive did not find a matching page.", "provider_rejected": "CivArchive rejected the search request.", } + + +def build_civarchive_failure_status( + code: str = "provider_unavailable", + http_status: Optional[int] = None, + retryable: bool = True, +) -> Dict[str, Any]: + """Build the public CivArchive provider status contract.""" + if code == "rate_limited": + state = "rate_limited" + elif code == "not_found": + state = "not_found" + elif retryable: + state = "unavailable" + else: + state = "error" + + return { + "state": state, + "code": code, + "retryable": retryable, + "http_status": http_status, + "message": CIVARCHIVE_FAILURE_MESSAGES.get( + code, + "CivArchive search failed. Please try again later.", + ), + } + + CIVARCHIVE_SIZE_PROBE_DOMAINS = ( "huggingface.co", "civarchive.com", @@ -140,25 +169,11 @@ class CivArchiveSearchError(Exception): def as_status(self) -> Dict[str, Any]: """Return a safe provider status payload for search responses.""" - if self.code == "rate_limited": - state = "rate_limited" - elif self.code == "not_found": - state = "not_found" - elif self.retryable: - state = "unavailable" - else: - state = "error" - - return { - "state": state, - "code": self.code, - "retryable": self.retryable, - "http_status": self.http_status, - "message": CIVARCHIVE_FAILURE_MESSAGES.get( - self.code, - "CivArchive search failed. Please try again later.", - ), - } + return build_civarchive_failure_status( + code=self.code, + http_status=self.http_status, + retryable=self.retryable, + ) def _classify_civarchive_http_status(status_code: int) -> Dict[str, Any]: diff --git a/tests/test_civarchive_failure_contract.py b/tests/test_civarchive_failure_contract.py index 8b65bcf..3a819c5 100644 --- a/tests/test_civarchive_failure_contract.py +++ b/tests/test_civarchive_failure_contract.py @@ -95,6 +95,20 @@ def test_civarchive_failure_exposes_safe_status_payload(): } +def test_civarchive_status_builder_is_the_shared_failure_contract(): + assert civarchive.build_civarchive_failure_status( + code="rate_limited", + http_status=429, + retryable=True, + ) == { + "state": "rate_limited", + "code": "rate_limited", + "retryable": True, + "http_status": 429, + "message": "CivArchive rate limit was reached. Please try again later.", + } + + def test_request_source_response_can_propagate_transport_errors(): with patch.object(network_utils.requests, "get", side_effect=requests.Timeout("timeout")), pytest.raises( requests.Timeout, match="timeout" diff --git a/tests/test_search_source_methods.mjs b/tests/test_search_source_methods.mjs index 25800ef..842853a 100644 --- a/tests/test_search_source_methods.mjs +++ b/tests/test_search_source_methods.mjs @@ -84,11 +84,11 @@ test('source normalization preserves labels, trimmed selections, and provider er ); assert.equal( getSearchSourceErrorMessage.call(dialog, ' CivArchive ', 'HTTP 502'), - 'CivArchive may be overloaded or temporarily unavailable. Please try again.' + 'HTTP 502' ); }); -test('CivArchive availability failures receive a user-facing retry message', () => { +test('unstructured provider failures preserve their technical message', () => { const dialog = { getSearchSourceLabel, getSearchSourceErrorMessage }; const temporaryFailure = getSearchSourceErrorMessage.call( dialog, @@ -98,11 +98,11 @@ test('CivArchive availability failures receive a user-facing retry message', () assert.equal( temporaryFailure, - 'CivArchive may be overloaded or temporarily unavailable. Please try again.' + 'CivArchive search failed: CivArchive search request failed (network error or timeout)' ); assert.equal( getSearchSourceErrorMessage.call(dialog, 'civarchive', 'CivArchive search failed: HTTP 502'), - temporaryFailure + 'CivArchive search failed: HTTP 502' ); assert.equal( getSearchSourceErrorMessage.call(dialog, 'civarchive', 'CivArchive search failed: HTTP 400'), @@ -114,7 +114,7 @@ test('CivArchive availability failures receive a user-facing retry message', () ); assert.equal( getSearchSourceErrorTooltip.call(dialog, 'civarchive', 'CivArchive search failed: HTTP 522'), - 'CivArchive may be overloaded or temporarily unavailable. Please try again. Details: CivArchive search failed: HTTP 522' + 'CivArchive search failed: HTTP 522' ); }); @@ -142,7 +142,7 @@ test('structured CivArchive status uses safe messages and HTTP details', () => { code: 'rate_limited', retryable: true }), - 'CivArchive rate limit was reached. Please try again later.' + 'CivArchive is rate limited. Please try again later.' ); }); diff --git a/web/resolver/search/search_source_methods.js b/web/resolver/search/search_source_methods.js index 6d3d259..f01d128 100644 --- a/web/resolver/search/search_source_methods.js +++ b/web/resolver/search/search_source_methods.js @@ -6,6 +6,14 @@ import { } from "../utils/source_labels.js"; const localStorage = safeStorage; +const SEARCH_SOURCE_STATUS_FALLBACKS = Object.freeze({ + timeout: 'search timed out. Please try again.', + network_error: 'could not be reached. Please try again.', + provider_unavailable: 'is temporarily unavailable. Please try again.', + rate_limited: 'is rate limited. Please try again later.', + not_found: 'did not return a matching result.', + provider_rejected: 'rejected the search request.', +}); export const searchSourceMethods = { getSearchResultKeysForSources(sources = []) { @@ -75,31 +83,14 @@ export const searchSourceMethods = { } const message = String(error || '').trim(); - const normalizedSource = normalizeSourceKey(source); const statusCode = String(status?.code || '').trim().toLowerCase(); - if (normalizedSource === 'civarchive') { - const messagesByCode = { - timeout: 'CivArchive did not respond in time. It may be temporarily overloaded. Please try again.', - network_error: 'CivArchive could not be reached. It may be temporarily unavailable. Please try again.', - provider_unavailable: 'CivArchive may be overloaded or temporarily unavailable. Please try again.', - rate_limited: 'CivArchive rate limit was reached. Please try again later.', - not_found: 'CivArchive did not find a matching page.', - provider_rejected: 'CivArchive rejected the search request.' - }; - if (messagesByCode[statusCode]) return messagesByCode[statusCode]; + const sourceLabel = this.getSearchSourceLabel(source); + if (SEARCH_SOURCE_STATUS_FALLBACKS[statusCode]) { + return `${sourceLabel} ${SEARCH_SOURCE_STATUS_FALLBACKS[statusCode]}`; } if (!message) { - return `${this.getSearchSourceLabel(source)} search failed.`; - } - - const indicatesTemporaryUnavailable = ( - /network error|timeout|timed out|connection (?:error|reset|refused)|temporarily unavailable/i.test(message) - || /\bHTTP\s+(?:429|5\d{2})\b/i.test(message) - ); - - if (normalizedSource === 'civarchive' && indicatesTemporaryUnavailable) { - return 'CivArchive may be overloaded or temporarily unavailable. Please try again.'; + return `${sourceLabel} search failed.`; } return message;