diff --git a/plugin-repos/starlark-apps/tronbyte_repository.py b/plugin-repos/starlark-apps/tronbyte_repository.py index ba647de0..1be7c981 100644 --- a/plugin-repos/starlark-apps/tronbyte_repository.py +++ b/plugin-repos/starlark-apps/tronbyte_repository.py @@ -5,6 +5,7 @@ Fetches app listings, metadata, and downloads .star files. """ +import json import logging import time import requests @@ -49,6 +50,13 @@ def __init__(self, github_token: Optional[str] = None): self.base_url = "https://api.github.com" self.raw_url = "https://raw.githubusercontent.com" + # Why the last GitHub API call failed, in words a user can act on. + # _make_request used to log the reason and return a bare None, so + # every caller up the stack knew only that "something" went wrong -- + # which is how an exhausted rate limit reached the store page as an + # empty grid with no explanation. + self.last_error: Optional[str] = None + self.session = requests.Session() if github_token: self.session.headers.update({ @@ -70,29 +78,50 @@ def _make_request(self, url: str, timeout: int = 10) -> Optional[Dict[str, Any]] Returns: JSON response or None on error """ + self.last_error = None try: response = self.session.get(url, timeout=timeout) - if response.status_code == 403: - # Rate limit exceeded - logger.warning("[Tronbyte Repo] GitHub API rate limit exceeded") + if response.status_code in (403, 429): + # 403 is both "rate limited" and "forbidden"; the remaining + # counter is what tells them apart, and the difference matters + # to whoever reads the message -- one is fixed by waiting or + # adding a token, the other is not. + remaining = response.headers.get('X-RateLimit-Remaining') + if remaining == '0': + self.last_error = ( + "GitHub API rate limit exceeded" + f" ({response.headers.get('X-RateLimit-Limit', '?')} requests/hour" + f"{'' if self.github_token else ', unauthenticated'})." + " Add a GitHub token in settings, or wait for the limit to reset." + ) + else: + self.last_error = f"GitHub refused the request ({response.status_code})" + logger.warning(f"[Tronbyte Repo] {self.last_error}") return None elif response.status_code == 404: + self.last_error = "Not found on GitHub" logger.warning(f"[Tronbyte Repo] Resource not found: {url}") return None elif response.status_code != 200: + self.last_error = f"GitHub API error {response.status_code}" logger.error(f"[Tronbyte Repo] GitHub API error: {response.status_code}") return None return response.json() except requests.Timeout: + self.last_error = "Timed out reaching GitHub" logger.error(f"[Tronbyte Repo] Request timeout: {url}") return None except requests.RequestException as e: + self.last_error = f"Network error reaching GitHub: {e.__class__.__name__}" logger.error(f"[Tronbyte Repo] Request error: {e}", exc_info=True) return None except (json.JSONDecodeError, ValueError) as e: + # Reachable whenever something on the path answers with HTML -- + # a captive portal, a proxy error page, a DNS-hijacking router. + self.last_error = "GitHub returned a response that was not JSON" logger.error(f"[Tronbyte Repo] JSON parse error for {url}: {e}", exc_info=True) return None @@ -125,33 +154,79 @@ def _fetch_raw_file(self, file_path: str, branch: Optional[str] = None, binary: logger.error(f"[Tronbyte Repo] Network error fetching raw file {file_path}: {e}", exc_info=True) return None - def list_apps(self) -> Tuple[bool, Optional[List[Dict[str, Any]]], Optional[str]]: - """ - List all available apps in the repository. + def _list_app_dirs_via_trees(self) -> Optional[List[Dict[str, Any]]]: + """App directories via the git trees API, or None on failure. - Returns: - Tuple of (success, apps_list, error_message) + The contents API caps a directory listing at 1000 entries and says + nothing about having truncated it, so the store showed the first 1000 + apps of a repository that has more and looked complete while doing it. + The trees API caps far higher and sets `truncated` when it does, at + the cost of one extra call to resolve the `apps` tree. """ + repo = f"{self.base_url}/repos/{self.REPO_OWNER}/{self.REPO_NAME}" + + root = self._make_request(f"{repo}/git/trees/{self.DEFAULT_BRANCH}") + if not isinstance(root, dict): + return None + + apps_sha = next( + (e.get('sha') for e in root.get('tree', []) or [] + if e.get('path') == self.APPS_PATH and e.get('type') == 'tree'), + None) + if not apps_sha: + self.last_error = f"No '{self.APPS_PATH}' directory in the repository" + return None + + tree = self._make_request(f"{repo}/git/trees/{apps_sha}") + if not isinstance(tree, dict): + return None + + if tree.get('truncated'): + logger.warning( + "[Tronbyte Repo] GitHub truncated the app tree; the listing is incomplete") + + return [ + {'id': e['path'], 'path': f"{self.APPS_PATH}/{e['path']}", 'url': None} + for e in tree.get('tree', []) or [] + if e.get('type') == 'tree' and e.get('path') and not e['path'].startswith('.') + ] + + def _list_app_dirs_via_contents(self) -> Optional[List[Dict[str, Any]]]: + """App directories via the contents API. Capped at 1000 entries.""" url = f"{self.base_url}/repos/{self.REPO_OWNER}/{self.REPO_NAME}/contents/{self.APPS_PATH}" data = self._make_request(url) if data is None: - return False, None, "Failed to fetch repository contents" - + return None if not isinstance(data, list): - return False, None, "Invalid response format" + self.last_error = "GitHub returned an unexpected listing format" + return None - # Filter directories (apps) - apps = [] - for item in data: - if item.get('type') == 'dir': - app_id = item.get('name') - if app_id and not app_id.startswith('.'): - apps.append({ - 'id': app_id, - 'path': item.get('path'), - 'url': item.get('url') - }) + return [ + {'id': item.get('name'), 'path': item.get('path'), 'url': item.get('url')} + for item in data + if item.get('type') == 'dir' and item.get('name') + and not item['name'].startswith('.') + ] + + def list_apps(self) -> Tuple[bool, Optional[List[Dict[str, Any]]], Optional[str]]: + """ + List all available apps in the repository. + + Returns: + Tuple of (success, apps_list, error_message) + """ + apps = self._list_app_dirs_via_trees() + if apps is None: + # Fall back rather than fail: the contents API was what shipped, + # so a trees-only outage should not take the store down with it. + trees_error = self.last_error + logger.warning( + f"[Tronbyte Repo] Trees listing failed ({trees_error}); " + "falling back to the contents API") + apps = self._list_app_dirs_via_contents() + if apps is None: + return False, None, self.last_error or trees_error or "Failed to fetch repository contents" logger.info(f"Found {len(apps)} apps in repository") return True, apps, None @@ -267,14 +342,22 @@ def list_all_apps_cached(self) -> Dict[str, Any]: 'categories': _apps_cache['categories'], 'authors': _apps_cache['authors'], 'count': len(_apps_cache['data']), - 'cached': True + 'cached': True, + 'error': None, } - # Fetch directory listing (1 GitHub API call) + # Fetch directory listing (a small number of GitHub API calls) success, app_dirs, error = self.list_apps() if not success or not app_dirs: - logger.error(f"Failed to list apps for bulk fetch: {error}") - return {'apps': [], 'categories': [], 'authors': [], 'count': 0, 'cached': False} + # Returning an empty list here used to read downstream as "the + # repository has no apps", and the route reported that as a + # success -- so a rate limit, a DNS failure and an empty + # repository were all drawn as the same blank grid. Hand the + # reason back instead and let the caller surface it. + reason = error or "No apps found in the repository" + logger.error(f"Failed to list apps for bulk fetch: {reason}") + return {'apps': [], 'categories': [], 'authors': [], + 'count': 0, 'cached': False, 'error': reason} logger.info(f"Bulk-fetching manifests for {len(app_dirs)} apps...") @@ -341,7 +424,8 @@ def fetch_one(app_info): 'categories': categories, 'authors': authors, 'count': len(apps_with_metadata), - 'cached': False + 'cached': False, + 'error': None, } def download_star_file(self, app_id: str, output_path: Path, filename: Optional[str] = None) -> Tuple[bool, Optional[str]]: diff --git a/test/test_web_error_detail.py b/test/test_web_error_detail.py index 5362943a..55f81db8 100644 --- a/test/test_web_error_detail.py +++ b/test/test_web_error_detail.py @@ -116,7 +116,56 @@ def test_no_api_v3_handler_discards_its_exception(self): src = open("web_interface/blueprints/api_v3.py").read() tree = ast.parse(src) - generic = "An error occurred; see logs for details" + + # This used to match one exact message string, so a handler that wrote + # its own wording was never checked. All thirteen Starlark routes did + # -- "Failed to browse repository" and friends -- and every one of them + # answered a 500 with no detail at all, which is how the app store + # spent three releases failing for reasons nobody could read. The rule + # is now the shape that matters: if it returns 5xx, it says why. + PRE_EXISTING = { + # Not part of this change. This set may shrink, never grow. + 'backup_delete', 'backup_export', 'backup_list', 'backup_preview', + 'backup_restore', 'backup_validate', 'checkout_branch', + 'execute_system_action', 'get_git_branches', 'get_git_info', + 'get_hardware_status', 'get_logs', 'get_system_status', + 'get_system_version', 'scan_wifi_networks', + } + + def enclosing_function(handler): + """Innermost function containing `handler`.""" + best = None + for fn in [n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]: + if any(h is handler for h in ast.walk(fn)): + if best is None or fn.lineno > best.lineno: + best = fn + return best.name if best else '' + + def only_catches_importerror(handler): + """An `except ImportError` arm and nothing else. + + A missing optional dependency is a configuration fact, not a + crash: the module name is the whole diagnosis and it is already + in the response, so a stack trace would be noise. Detail is still + required -- only the traceback log is excused. + """ + t = handler.type + names = ([t] if isinstance(t, ast.Name) + else list(t.elts) if isinstance(t, ast.Tuple) else []) + return bool(names) and all( + isinstance(n, ast.Name) and n.id == 'ImportError' for n in names) + + def returns_5xx(handler): + for r in [n for n in ast.walk(handler) if isinstance(n, ast.Return)]: + v = r.value + if isinstance(v, ast.Tuple) and len(v.elts) == 2: + code = v.elts[1] + if (isinstance(code, ast.Constant) + and isinstance(code.value, int) + and 500 <= code.value < 600): + return True + return False def logs_a_traceback(handler): """An error/exception-level log call carrying exc_info.""" @@ -161,11 +210,12 @@ def returns_the_detail(handler): offenders = [] for h in [n for n in ast.walk(tree) if isinstance(n, ast.ExceptHandler)]: - seg = ast.get_source_segment(src, h) or "" - if generic not in seg: + if not returns_5xx(h): + continue + if enclosing_function(h) in PRE_EXISTING: continue missing = [] - if not logs_a_traceback(h): + if not logs_a_traceback(h) and not only_catches_importerror(h): missing.append("error-level log with exc_info") if not returns_the_detail(h): missing.append("describe_exception(e) in the response") diff --git a/test/web_interface/test_starlark_pixlet_routes.py b/test/web_interface/test_starlark_pixlet_routes.py index 82c13ef9..24fab51e 100644 --- a/test/web_interface/test_starlark_pixlet_routes.py +++ b/test/web_interface/test_starlark_pixlet_routes.py @@ -393,3 +393,358 @@ def test_the_value_resolves_against_the_app_directory(self, starlark_dir, tmp_pa def test_it_matches_the_default_a_reader_falls_back_to(self, starlark_dir, tmp_path): """Stored and defaulted values must mean the same thing.""" assert self._install(tmp_path)['star_file'] == 'demo.star' + + +# --------------------------------------------------------------------------- +# The store loaded, then stopped loading, and nothing anywhere said why. +# +# #535 restored the routes, so the 404 was gone -- but two failure modes +# underneath it produce the same blank grid, and neither could be read from +# outside. On the device this was diagnosed on, /repository/browse answered +# 200 with 1000 apps in 27s while GitHub reported 18 of 60 unauthenticated +# requests remaining, with 48 installed plugins checking for updates against +# the same budget. When that budget runs out the store goes blank and says +# nothing at all. +# --------------------------------------------------------------------------- + +def _repository_module(): + """Load tronbyte_repository.py the way the blueprint does.""" + import importlib.util + import sys + from pathlib import Path + + path = (Path(__file__).resolve().parents[2] + / 'plugin-repos' / 'starlark-apps' / 'tronbyte_repository.py') + spec = importlib.util.spec_from_file_location('_tronbyte_repo_under_test', path) + module = importlib.util.module_from_spec(spec) + sys.modules['_tronbyte_repo_under_test'] = module + spec.loader.exec_module(module) + return module + + +class _Resp: + """Enough of requests.Response for the paths under test.""" + + def __init__(self, status_code=200, payload=None, headers=None, raises=None): + self.status_code = status_code + self._payload = payload + self.headers = headers or {} + self._raises = raises + + def json(self): + if self._raises is not None: + raise self._raises + return self._payload + + +class TestTheJsonGuardIsNotItselfACrash: + """`except (json.JSONDecodeError, ValueError)` with no `import json`. + + Evaluating that tuple raises NameError, so the guard written for exactly + this case never ran: a non-JSON body -- a captive portal, a proxy error + page, a DNS-hijacking router answering for api.github.com -- came out as + a 500 instead of the None the caller was written to handle. + """ + + def test_a_non_json_body_returns_none(self): + repo = _repository_module().TronbyteRepository() + repo.session.get = lambda *a, **k: _Resp( + raises=ValueError("Expecting value: line 1 column 1 (char 0)")) + + assert repo._make_request("https://api.github.com/anything") is None + + def test_it_says_the_response_was_not_json(self): + repo = _repository_module().TronbyteRepository() + repo.session.get = lambda *a, **k: _Resp(raises=ValueError("nope")) + + repo._make_request("https://api.github.com/anything") + assert 'JSON' in (repo.last_error or ''), repo.last_error + + +class TestAnExhaustedRateLimitSaysSo: + """60 requests/hour unauthenticated, shared with every update check.""" + + def test_the_message_names_the_rate_limit(self): + repo = _repository_module().TronbyteRepository() + repo.session.get = lambda *a, **k: _Resp( + status_code=403, + headers={'X-RateLimit-Remaining': '0', 'X-RateLimit-Limit': '60'}) + + assert repo._make_request("https://api.github.com/anything") is None + assert 'rate limit' in (repo.last_error or '').lower(), repo.last_error + + def test_it_mentions_being_unauthenticated_when_there_is_no_token(self): + repo = _repository_module().TronbyteRepository() + repo.session.get = lambda *a, **k: _Resp( + status_code=403, + headers={'X-RateLimit-Remaining': '0', 'X-RateLimit-Limit': '60'}) + + repo._make_request("https://api.github.com/anything") + assert 'unauthenticated' in (repo.last_error or ''), repo.last_error + + def test_a_plain_403_is_not_reported_as_a_rate_limit(self): + repo = _repository_module().TronbyteRepository() + repo.session.get = lambda *a, **k: _Resp( + status_code=403, headers={'X-RateLimit-Remaining': '57'}) + + repo._make_request("https://api.github.com/anything") + assert 'rate limit' not in (repo.last_error or '').lower(), repo.last_error + + +class TestAFailedFetchIsNotAnEmptyRepository: + """list_all_apps_cached turned every failure into an empty app list. + + The route then reported that as a success, so a rate limit, a DNS failure + and a genuinely empty repository were all drawn as the same blank grid. + """ + + def test_the_reason_comes_back_with_the_empty_list(self): + module = _repository_module() + repo = module.TronbyteRepository() + repo.list_apps = lambda: (False, None, "GitHub API rate limit exceeded") + + result = repo.list_all_apps_cached() + assert result['count'] == 0 + assert 'rate limit' in result['error'].lower(), result + + def test_a_failure_is_not_cached_as_an_empty_repository(self): + module = _repository_module() + repo = module.TronbyteRepository() + repo.list_apps = lambda: (False, None, "boom") + repo.list_all_apps_cached() + + assert module._apps_cache['data'] is None, \ + "a failed fetch was cached, so the store stays empty for 2 hours" + + def test_a_successful_fetch_reports_no_error(self): + module = _repository_module() + repo = module.TronbyteRepository() + repo.list_apps = lambda: (True, [{'id': 'a', 'path': 'apps/a'}], None) + repo._fetch_raw_file = lambda *a, **k: "name: A\nsummary: s\n" + + assert repo.list_all_apps_cached().get('error') is None + + +class TestTheStoreReportsWhyItIsEmpty: + """The route's half of the same failure.""" + + @pytest.fixture + def failing_repo(self): + repo = MagicMock() + repo.return_value.list_all_apps_cached.return_value = { + 'apps': [], 'categories': [], 'authors': [], 'count': 0, + 'cached': False, + 'error': 'GitHub API rate limit exceeded (60 requests/hour, ' + 'unauthenticated).', + } + repo.return_value.get_rate_limit_info.return_value = {'remaining': 0} + with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class', + return_value=repo): + yield repo + + def test_browse_does_not_call_a_failure_a_success(self, client, failing_repo): + body = client.get('/api/v3/starlark/repository/browse').get_json() + assert body['status'] == 'error', body + + def test_browse_answers_502_not_200(self, client, failing_repo): + resp = client.get('/api/v3/starlark/repository/browse') + assert resp.status_code == 502, resp.get_json() + + def test_the_reason_reaches_the_page(self, client, failing_repo): + body = client.get('/api/v3/starlark/repository/browse').get_json() + assert 'rate limit' in body['message'].lower(), body + + def test_categories_reports_it_too(self, client, failing_repo): + resp = client.get('/api/v3/starlark/repository/categories') + assert resp.status_code == 502 + assert 'rate limit' in resp.get_json()['message'].lower() + + @pytest.fixture + def working_repo(self): + repo = MagicMock() + repo.return_value.list_all_apps_cached.return_value = { + 'apps': [{'id': 'quoteoftheday'}], 'categories': [], 'authors': [], + 'count': 1, 'cached': False, 'error': None, + } + repo.return_value.get_rate_limit_info.return_value = {'remaining': 57} + with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class', + return_value=repo): + yield repo + + def test_a_working_fetch_is_still_a_success(self, client, working_repo): + resp = client.get('/api/v3/starlark/repository/browse') + assert resp.status_code == 200 + assert resp.get_json()['status'] == 'success' + + +class TestACrashCarriesItsDetail: + """Seventeen Starlark handlers answered 5xx with no detail at all.""" + + def test_browse_returns_the_exception_detail(self, client): + with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class', + side_effect=ImportError("No module named 'yaml'")): + body = client.get('/api/v3/starlark/repository/browse').get_json() + + assert 'yaml' in body.get('details', ''), body + + def test_status_returns_the_exception_detail(self, client): + with patch('web_interface.blueprints.api_v3._get_starlark_plugin', + side_effect=RuntimeError("plugin manager is not attached")): + body = client.get('/api/v3/starlark/status').get_json() + + assert 'plugin manager is not attached' in body.get('details', ''), body + + +class TestTheListingIsNotCappedAtOneThousand: + """The contents API caps a directory at 1000 entries and does not say so. + + tronbyt/apps returns exactly 1000 through that endpoint, which is the cap + rather than the app count -- the store looked complete while showing a + truncated repository. + """ + + def _repo_with_tree(self, module, count): + repo = module.TronbyteRepository() + entries = [{'path': 'app%04d' % i, 'type': 'tree'} for i in range(count)] + + def fake_request(url, timeout=10): + if url.endswith('/git/trees/main'): + return {'tree': [{'path': 'apps', 'type': 'tree', 'sha': 'deadbeef'}]} + if url.endswith('/git/trees/deadbeef'): + return {'tree': entries, 'truncated': False} + raise AssertionError("unexpected request: %s" % url) + + repo._make_request = fake_request + return repo + + def test_more_than_a_thousand_apps_are_listed(self): + module = _repository_module() + repo = self._repo_with_tree(module, 1400) + + ok, apps, err = repo.list_apps() + assert ok, err + assert len(apps) == 1400 + + def test_the_path_is_still_the_one_manifest_fetches_use(self): + module = _repository_module() + repo = self._repo_with_tree(module, 3) + + _, apps, _ = repo.list_apps() + assert apps[0]['path'] == 'apps/app0000', apps[0] + + def test_dotfiles_and_files_are_skipped(self): + module = _repository_module() + repo = module.TronbyteRepository() + + def fake_request(url, timeout=10): + if url.endswith('/git/trees/main'): + return {'tree': [{'path': 'apps', 'type': 'tree', 'sha': 'x'}]} + return {'tree': [{'path': '.github', 'type': 'tree'}, + {'path': 'realapp', 'type': 'tree'}, + {'path': 'README.md', 'type': 'blob'}]} + + repo._make_request = fake_request + _, apps, _ = repo.list_apps() + assert [a['id'] for a in apps] == ['realapp'] + + def test_it_falls_back_to_the_contents_api(self): + """A trees outage must not take the store down with it.""" + module = _repository_module() + repo = module.TronbyteRepository() + + def fake_request(url, timeout=10): + if '/git/trees/' in url: + repo.last_error = "GitHub API error 500" + return None + return [{'name': 'fallbackapp', 'path': 'apps/fallbackapp', 'type': 'dir'}] + + repo._make_request = fake_request + ok, apps, err = repo.list_apps() + assert ok, err + assert [a['id'] for a in apps] == ['fallbackapp'] + + def test_both_paths_failing_reports_the_reason(self): + module = _repository_module() + repo = module.TronbyteRepository() + + def fake_request(url, timeout=10): + repo.last_error = "Timed out reaching GitHub" + return None + + repo._make_request = fake_request + ok, apps, err = repo.list_apps() + assert not ok + assert 'Timed out' in err, err + + +class TestTheStoreUsesTheTokenTheUserConfigured: + """The store authenticated with a key nothing ever writes. + + The three repository routes read `github_token` off config.json. Nothing + writes that key: config.template.json has no such field, no setting + offers it, and the token the user actually configures goes to + config_secrets.json as `github.api_token`, which PluginStoreManager loads + and every other GitHub caller uses. + + So the store ran unauthenticated at 60 requests/hour on the same per-IP + budget as 48 plugins' update checks, while the configured token sat + unused raising that same budget to 5000. On the device this was found on, + /plugins/store/github-status reported `authenticated: true` with a + rate_limit of 5000 while /starlark/repository/browse reported a limit of + 60 -- the store going blank was that 60 running out. + """ + + def test_the_store_managers_token_is_used(self): + from web_interface.blueprints import api_v3 as mod + + with patch.object(mod.api_v3, 'plugin_store_manager', + MagicMock(github_token='ghp_configured')): + assert mod._starlark_github_token() == 'ghp_configured' + + def test_a_hand_edited_config_key_still_works(self): + from web_interface.blueprints import api_v3 as mod + + cfg = MagicMock() + cfg.load_config.return_value = {'github_token': 'ghp_by_hand'} + with patch.object(mod.api_v3, 'plugin_store_manager', + MagicMock(github_token=None)), \ + patch.object(mod.api_v3, 'config_manager', cfg): + assert mod._starlark_github_token() == 'ghp_by_hand' + + def test_no_token_anywhere_is_not_an_error(self): + from web_interface.blueprints import api_v3 as mod + + cfg = MagicMock() + cfg.load_config.return_value = {} + with patch.object(mod.api_v3, 'plugin_store_manager', + MagicMock(github_token=None)), \ + patch.object(mod.api_v3, 'config_manager', cfg): + assert mod._starlark_github_token() is None + + def test_an_unreadable_config_does_not_take_the_store_down(self): + from web_interface.blueprints import api_v3 as mod + + cfg = MagicMock() + cfg.load_config.side_effect = OSError("config.json is unreadable") + with patch.object(mod.api_v3, 'plugin_store_manager', + MagicMock(github_token=None)), \ + patch.object(mod.api_v3, 'config_manager', cfg): + assert mod._starlark_github_token() is None + + def test_browse_hands_the_token_to_the_repository(self, client): + from web_interface.blueprints import api_v3 as mod + + repo = MagicMock() + repo.return_value.list_all_apps_cached.return_value = { + 'apps': [], 'categories': [], 'authors': [], 'count': 0, + 'cached': False, 'error': None, + } + repo.return_value.get_rate_limit_info.return_value = {'remaining': 4999} + + with patch.object(mod.api_v3, 'plugin_store_manager', + MagicMock(github_token='ghp_configured')), \ + patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class', + return_value=repo): + client.get('/api/v3/starlark/repository/browse') + + repo.assert_called_once_with(github_token='ghp_configured') diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index a9079b05..865789f0 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -8858,7 +8858,7 @@ def get_starlark_status(): except Exception as e: logger.exception("[Starlark] get_starlark_status failed") - return jsonify({'status': 'error', 'message': 'Failed to get Starlark status'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to get Starlark status', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/install-pixlet', methods=['POST']) def install_pixlet(): @@ -8884,11 +8884,13 @@ def install_pixlet(): else: return jsonify({'status': 'error', 'message': f'Failed to download Pixlet: {result.stderr}'}), 500 - except subprocess.TimeoutExpired: - return jsonify({'status': 'error', 'message': 'Download timed out'}), 500 + except subprocess.TimeoutExpired as err: + logger.exception("[Starlark] Pixlet download timed out") + return jsonify({'status': 'error', 'message': 'Download timed out', + 'details': describe_exception(err)}), 500 except Exception as e: logger.exception("[Starlark] install_pixlet failed") - return jsonify({'status': 'error', 'message': 'Failed to install Pixlet'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to install Pixlet', 'details': describe_exception(e)}), 500 # The remaining eleven routes #330 dropped, restored the same way: apps CRUD, @@ -8897,6 +8899,32 @@ def install_pixlet(): # these the store lists nothing and installing anything answers with the same # generic 404 the Pixlet button did. +def _starlark_github_token() -> Optional[str]: + """The GitHub token the Starlark store should authenticate with. + + These routes used to read `github_token` off config.json, a key that is + written nowhere and offered by no setting -- so the store always ran + unauthenticated at 60 requests/hour, on the same per-IP budget every + plugin update check spends, while the token the user had actually + configured sat in config_secrets.json raising the same budget to 5000. + The store going blank was that budget running out. + + Prefer the store manager's token, which is the one the settings UI + writes and validates; keep the config.json key as a fallback so a + hand-edited config still works. + """ + token = getattr(api_v3.plugin_store_manager, 'github_token', None) + if token: + return token + + try: + config = api_v3.config_manager.load_config() if api_v3.config_manager else {} + return config.get('github_token') + except Exception: + logger.warning("[Starlark] Could not read config for a GitHub token", exc_info=True) + return None + + def _get_tronbyte_repository_class() -> Type[Any]: """Import TronbyteRepository from plugin-repos directory.""" import importlib.util @@ -9271,7 +9299,7 @@ def get_starlark_apps(): except Exception as e: logger.exception("[Starlark] get_starlark_apps failed") - return jsonify({'status': 'error', 'message': 'Failed to get Starlark apps'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to get Starlark apps', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/apps/', methods=['GET']) def get_starlark_app(app_id): @@ -9337,7 +9365,7 @@ def get_starlark_app(app_id): except Exception as e: logger.exception("[Starlark] get_starlark_app failed") - return jsonify({'status': 'error', 'message': 'Failed to get Starlark app'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to get Starlark app', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/upload', methods=['POST']) def upload_starlark_app(): @@ -9404,17 +9432,19 @@ def upload_starlark_app(): pass except (OSError, IOError) as err: - # The detail goes to the log, not the response: it names absolute - # paths on the device, which the caller has no business seeing. The - # generic Exception arm below already did this; these two did not. + # This used to withhold the detail because it names absolute paths on + # the device. A full disk and a bad permission are indistinguishable + # without it, though, and describe_exception redacts credentials and + # truncates -- the same trade-off every other handler here makes. logger.exception("[Starlark] File error uploading starlark app: %s", err) - return jsonify({'status': 'error', 'message': 'File error during upload'}), 500 + return jsonify({'status': 'error', 'message': 'File error during upload', + 'details': describe_exception(err)}), 500 except ImportError as err: logger.exception("[Starlark] Module load error uploading starlark app: %s", err) - return jsonify({'status': 'error', 'message': 'Failed to load app module'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to load app module', 'details': describe_exception(err)}), 500 except Exception as err: logger.exception("[Starlark] Unexpected error uploading starlark app: %s", err) - return jsonify({'status': 'error', 'message': 'Failed to upload app'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to upload app', 'details': describe_exception(err)}), 500 @api_v3.route('/starlark/apps/', methods=['DELETE']) def uninstall_starlark_app(app_id): @@ -9445,7 +9475,7 @@ def uninstall_starlark_app(app_id): except Exception as e: logger.exception("[Starlark] uninstall_starlark_app failed") - return jsonify({'status': 'error', 'message': 'Failed to uninstall Starlark app'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to uninstall Starlark app', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/apps//config', methods=['GET']) def get_starlark_app_config(app_id): @@ -9492,7 +9522,7 @@ def get_starlark_app_config(app_id): except Exception as e: logger.exception("[Starlark] get_starlark_app_config failed") - return jsonify({'status': 'error', 'message': 'Failed to get Starlark app config'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to get Starlark app config', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/apps//config', methods=['PUT']) def update_starlark_app_config(app_id): @@ -9609,7 +9639,8 @@ def update_fn(manifest): except Exception as e: logger.error(f"Failed to save config.json for {app_id}: {e}") logger.exception("Failed to save Starlark configuration for %r", app_id) - return jsonify({'status': 'error', 'message': 'Failed to save configuration'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to save configuration', + 'details': describe_exception(e)}), 500 # Also update manifest for backward compatibility app_data.setdefault('config', {}).update(data) @@ -9621,7 +9652,7 @@ def update_fn(manifest): except Exception as e: logger.exception("[Starlark] update_starlark_app_config failed") - return jsonify({'status': 'error', 'message': 'Failed to update Starlark app config'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to update Starlark app config', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/apps//toggle', methods=['POST']) def toggle_starlark_app(app_id): @@ -9661,7 +9692,7 @@ def update_fn(manifest): except Exception as e: logger.exception("[Starlark] toggle_starlark_app failed") - return jsonify({'status': 'error', 'message': 'Failed to toggle Starlark app'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to toggle Starlark app', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/apps//render', methods=['POST']) def render_starlark_app(app_id): @@ -9690,7 +9721,7 @@ def render_starlark_app(app_id): except Exception as e: logger.exception("[Starlark] render_starlark_app failed") - return jsonify({'status': 'error', 'message': 'Failed to render Starlark app'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to render Starlark app', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/repository/browse', methods=['GET']) def browse_tronbyte_repository(): @@ -9703,14 +9734,22 @@ def browse_tronbyte_repository(): try: TronbyteRepository = _get_tronbyte_repository_class() - config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - github_token = config.get('github_token') - repo = TronbyteRepository(github_token=github_token) + repo = TronbyteRepository(github_token=_starlark_github_token()) result = repo.list_all_apps_cached() rate_limit = repo.get_rate_limit_info() + # An upstream failure used to arrive here as an empty app list and go + # out as 'success', so the store drew an empty grid and said nothing. + # 502: the request was fine, GitHub was not. + if result.get('error'): + return jsonify({ + 'status': 'error', + 'message': result['error'], + 'rate_limit': rate_limit, + }), 502 + return jsonify({ 'status': 'success', 'apps': result['apps'], @@ -9723,7 +9762,7 @@ def browse_tronbyte_repository(): except Exception as e: logger.exception("[Starlark] browse_tronbyte_repository failed") - return jsonify({'status': 'error', 'message': 'Failed to browse repository'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to browse repository', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/repository/install', methods=['POST']) def install_from_tronbyte_repository(): @@ -9740,9 +9779,7 @@ def install_from_tronbyte_repository(): TronbyteRepository = _get_tronbyte_repository_class() import tempfile - config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - github_token = config.get('github_token') - repo = TronbyteRepository(github_token=github_token) + repo = TronbyteRepository(github_token=_starlark_github_token()) success, metadata, error = repo.get_app_metadata(data['app_id']) if not success: @@ -9811,23 +9848,25 @@ def install_from_tronbyte_repository(): except Exception as e: logger.exception("[Starlark] install_from_tronbyte_repository failed") - return jsonify({'status': 'error', 'message': 'Failed to install from repository'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to install from repository', 'details': describe_exception(e)}), 500 @api_v3.route('/starlark/repository/categories', methods=['GET']) def get_tronbyte_categories(): """Get list of available app categories (uses bulk cache).""" try: TronbyteRepository = _get_tronbyte_repository_class() - config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - repo = TronbyteRepository(github_token=config.get('github_token')) + repo = TronbyteRepository(github_token=_starlark_github_token()) result = repo.list_all_apps_cached() + if result.get('error'): + return jsonify({'status': 'error', 'message': result['error']}), 502 + return jsonify({'status': 'success', 'categories': result['categories']}) except Exception as e: logger.exception("[Starlark] get_tronbyte_categories failed") - return jsonify({'status': 'error', 'message': 'Failed to fetch categories'}), 500 + return jsonify({'status': 'error', 'message': 'Failed to fetch categories', 'details': describe_exception(e)}), 500 def _starlark_virtual_plugins() -> list: