diff --git a/src/modelscope_hub/api.py b/src/modelscope_hub/api.py index 3ce9a35..071d449 100644 --- a/src/modelscope_hub/api.py +++ b/src/modelscope_hub/api.py @@ -156,7 +156,7 @@ def __init__( base._endpoint_overridden = was_overridden self._config = base if endpoint is not None: - self._config.endpoint = endpoint.rstrip("/") + self._config.endpoint = HubConfig.normalize_endpoint(endpoint) self._config._endpoint_overridden = True if token is not None: self._config.token = token @@ -188,7 +188,9 @@ def legacy(self) -> LegacyClient: endpoint=self._config.endpoint or DEFAULT_ENDPOINT, user_agent=build_user_agent(self._config.get_session_id()), ) - elif self._legacy.token != self._config.token and self._config.token: + elif self._legacy.token != self._config.token: + # Clears propagate as well as changes: a cached client left holding a + # revoked token would keep authenticating with it. self._legacy.token = self._config.token return self._legacy @@ -483,8 +485,19 @@ def login(self, token: str) -> UserInfo: InvalidParameter When ``token`` is empty or whitespace-only. AuthenticationError - When the server rejects the token. The bad token is cleared - from local storage before re-raising. + When the server rejects the token. The server's own explanation is + preserved, and an endpoint hint is appended when the token turns + out to be valid on the peer ModelScope site. + HubError + Transport, timeout and server-side failures propagate unchanged -- + they are never reported as a rejected token. + + Notes + ----- + A failed attempt leaves persisted credentials untouched. Until the + server has accepted the new token, the stored credential is still the + caller's only working one, so revoking it on failure would turn a + mistyped token into an unintended logout. Examples -------- @@ -497,6 +510,8 @@ def login(self, token: str) -> UserInfo: raise InvalidParameter("token must be a non-empty string") token = token.strip() + previous_token = self._config.token + previous_logged_out = self._config._logged_out self._config.token = token self._config._logged_out = False self._openapi = None @@ -505,12 +520,12 @@ def login(self, token: str) -> UserInfo: try: data, cookies = self.legacy.login(token) - except (AuthenticationError, HubError) as exc: - self._config.clear_token() - raise AuthenticationError( - "Login failed: the provided token was rejected by the server.", - status_code=getattr(exc, "status_code", None), - ) from exc + except HubError as exc: + self._restore_credential_state(previous_token, previous_logged_out) + explained = self._explain_login_failure(token, exc) + if explained is exc: + raise + raise explained from exc git_token = data.get("AccessToken", "") username = data.get("Username", "") @@ -524,6 +539,91 @@ def login(self, token: str) -> UserInfo: return self.whoami() + def _restore_credential_state(self, token: str | None, logged_out: bool) -> None: + """Roll the in-memory credential back to its pre-login value. + + Persisted credentials are deliberately left alone; only this instance's + transient state is rewound, so a failed attempt leaves the object + exactly as it was found instead of poisoning it with a rejected token. + """ + self._config.token = token + self._config._logged_out = logged_out + self._openapi = None + if self._legacy is not None: + self._legacy.token = token + + def _explain_login_failure(self, token: str, exc: HubError) -> HubError: + """Return the exception to surface for a failed login attempt. + + Only authentication failures are re-worded. Network, timeout and + server-side errors are handed back untouched, because presenting them + as a rejected token would send the caller after the wrong remedy. + + The two ModelScope sites keep separate account systems and answer an + unknown token with the same business code, so the server cannot tell + "invalid token" apart from "token issued by the other site". Only the + client knows which site it addressed, which is why that disambiguation + has to happen here. + """ + if not isinstance(exc, AuthenticationError): + return exc + peer = self._peer_site_endpoint() + if peer is None or not self._token_valid_on(token, peer): + return exc + return AuthenticationError( + f"{exc.message} This token is valid on {peer} instead; retry with " + f"--endpoint {peer} (or set MODELSCOPE_ENDPOINT={peer}).", + status_code=exc.status_code, + request_id=exc.request_id, + response_body=exc.response_body, + url=exc.url, + method=exc.method, + ) + + def _peer_site_endpoint(self) -> str | None: + """Return the sibling ModelScope site, or ``None`` when not applicable. + + An explicitly configured endpoint is always respected, mirroring + :meth:`resolve_endpoint_for_read`: when the caller has pinned a site we + do not second-guess it. + """ + if self._config._endpoint_overridden: + return None + from .constants import DEFAULT_INTL_ENDPOINT + + def site_key(url: str) -> str: + host = (urlparse(url).hostname or "").lower() + return host[4:] if host.startswith("www.") else host + + current = site_key(self._config.endpoint or DEFAULT_ENDPOINT) + for candidate in (DEFAULT_ENDPOINT, DEFAULT_INTL_ENDPOINT): + if site_key(candidate) != current: + return candidate + return None + + @staticmethod + def _token_valid_on(token: str, endpoint: str) -> bool: + """Best-effort check of whether *token* authenticates against *endpoint*. + + Runs on the failure path only and is strictly advisory: any error means + "cannot confirm", so a probe outage degrades to the plain server message + rather than producing a misleading hint. Retries are disabled to keep + the failure path responsive. + """ + from .constants import API_CONNECT_TIMEOUT + + probe = LegacyClient( + token=None, + endpoint=endpoint, + timeout=API_CONNECT_TIMEOUT, + max_retries=0, + ) + try: + probe.login(token) + except Exception: # advisory only -- never mask the original failure + return False + return True + def logout(self) -> None: """Clear the locally persisted token. diff --git a/src/modelscope_hub/cli/main.py b/src/modelscope_hub/cli/main.py index 6fa219d..1bd198e 100644 --- a/src/modelscope_hub/cli/main.py +++ b/src/modelscope_hub/cli/main.py @@ -89,7 +89,7 @@ def _build_parser() -> argparse.ArgumentParser: "-v", "--verbose", action="store_true", - help="Enable verbose (DEBUG) logging.", + help="Enable DEBUG logging and print the full error cause chain.", ) subparsers = parser.add_subparsers(dest="command", metavar="COMMAND") @@ -191,6 +191,45 @@ def _discover_plugins(subparsers) -> None: logging.getLogger(__name__).debug("Failed to load CLI plugin %r: %s", ep.name, exc) +# --------------------------------------------------------------------------- +# Error reporting +# --------------------------------------------------------------------------- +def _next_cause(exc: BaseException) -> BaseException | None: + """Return what *exc* was raised from, honouring ``raise ... from None``.""" + if exc.__cause__ is not None: + return exc.__cause__ + if exc.__suppress_context__: + return None + return exc.__context__ + + +def _report_hub_error(exc: HubError, *, verbose: bool, max_depth: int = 5) -> None: + """Print a structured report for an SDK error. + + ``str(exc)`` already carries the error code, HTTP status, request id and -- + for API errors -- the request/response detail. Verbose mode additionally + unwinds the cause chain: wrapping an exception is convenient for callers but + otherwise hides the originating failure from whoever has to diagnose it. + + The walk is bounded by *max_depth* and skips exceptions already visited, so + a self-referential chain cannot stall the error path. + """ + error(str(exc)) + if exc.suggestion and exc.error_code != "E9001": + info(f"Suggestion: {exc.suggestion}") + if not verbose: + return + + seen = {id(exc)} + cause = _next_cause(exc) + depth = 1 + while cause is not None and id(cause) not in seen and depth <= max_depth: + info(f"{' ' * depth}Caused by: {cause.__class__.__name__}: {cause}") + seen.add(id(cause)) + cause = _next_cause(cause) + depth += 1 + + # --------------------------------------------------------------------------- # Entry point # --------------------------------------------------------------------------- @@ -200,8 +239,9 @@ def run_cmd(argv: Sequence[str] | None = None) -> int: parser = _build_parser() args = parser.parse_args(argv) + verbose = bool(getattr(args, "verbose", False)) logging.basicConfig( - level=logging.DEBUG if getattr(args, "verbose", False) else logging.INFO, + level=logging.DEBUG if verbose else logging.INFO, format="%(levelname)s %(name)s: %(message)s", ) @@ -218,14 +258,10 @@ def run_cmd(argv: Sequence[str] | None = None) -> int: except SystemExit as exc: # honour explicit SystemExit from subcommands return int(exc.code) if isinstance(exc.code, int) else (0 if exc.code is None else 1) except (InvalidParameter, NotSupportedError) as exc: - error(str(exc)) - if exc.suggestion: - info(f"Suggestion: {exc.suggestion}") + _report_hub_error(exc, verbose=verbose) return 2 except HubError as exc: - error(str(exc)) - if exc.suggestion and exc.error_code != "E9001": - info(f"Suggestion: {exc.suggestion}") + _report_hub_error(exc, verbose=verbose) return 1 except ValueError as exc: error(str(exc)) @@ -235,7 +271,7 @@ def run_cmd(argv: Sequence[str] | None = None) -> int: return 2 except Exception as exc: # pragma: no cover - unexpected error(f"Unexpected error: {exc.__class__.__name__}: {exc}") - if getattr(args, "verbose", False): + if verbose: raise return 1 diff --git a/src/modelscope_hub/config.py b/src/modelscope_hub/config.py index 4d3056f..9fbbadc 100644 --- a/src/modelscope_hub/config.py +++ b/src/modelscope_hub/config.py @@ -38,6 +38,14 @@ ENV_TOKEN = "MODELSCOPE_API_TOKEN" ENV_HOME = "MODELSCOPE_HOME" +# Files that together constitute a persisted login. ``session`` is deliberately +# excluded: it is an anonymous SDK install identifier, not a credential. +_CREDENTIAL_FILE_NAMES: tuple[str, ...] = ( + COOKIES_FILE_NAME, + GIT_TOKEN_FILE_NAME, + USER_INFO_FILE_NAME, +) + def _expand(path: str | os.PathLike[str]) -> Path: return Path(path).expanduser().resolve() @@ -85,10 +93,7 @@ def __post_init__(self) -> None: self._endpoint_overridden = True else: self.endpoint = DEFAULT_ENDPOINT - # Ensure endpoint always has a scheme - if self.endpoint and not self.endpoint.startswith(("http://", "https://")): - self.endpoint = f"https://{self.endpoint}" - self.endpoint = (self.endpoint or DEFAULT_ENDPOINT).rstrip("/") + self.endpoint = self.normalize_endpoint(self.endpoint) # Token precedence: explicit arg > MODELSCOPE_API_TOKEN env var > # persisted credential. An explicitly provided value wins even when # empty ("" means "use no token"), so an explicit override never @@ -101,6 +106,25 @@ def __post_init__(self) -> None: else: self.token = self.load_token() + @staticmethod + def normalize_endpoint(endpoint: str | None) -> str: + """Return *endpoint* with a scheme guaranteed and no trailing slash. + + Bare domains such as ``modelscope.ai`` are common input, especially from + the CLI. Without a scheme every request built from them fails deep in + the transport layer instead of surfacing a usable error, so the + normalisation lives here and is reused by every entry point that + accepts an endpoint. + + Scheme detection is case-insensitive because URI schemes are, so an + input like ``HTTPS://host`` is recognised instead of being prefixed a + second time. + """ + value = (endpoint or "").strip() or DEFAULT_ENDPOINT + if not value.lower().startswith(("http://", "https://")): + value = f"https://{value}" + return value.rstrip("/") + # ------------------------------------------------------------------ # Path helpers # ------------------------------------------------------------------ @@ -186,14 +210,19 @@ def load_token(self) -> str | None: return None def clear_token(self) -> None: - """Remove persisted credentials (deletes ``credentials/cookies``).""" + """Remove every persisted credential artefact. + + All login artefacts are dropped together. Removing only the session + cookie would leave the git token and the cached identity behind, a + half-logged-out state that later reads can still pick up. + """ self.token = None self._logged_out = True - path = self.credentials_dir / COOKIES_FILE_NAME - try: - path.unlink(missing_ok=True) - except OSError: - pass + for name in _CREDENTIAL_FILE_NAMES: + try: + (self.credentials_dir / name).unlink(missing_ok=True) + except OSError: + pass # ------------------------------------------------------------------ # Credentials persistence (compat with old modelscope SDK) diff --git a/src/modelscope_hub/errors.py b/src/modelscope_hub/errors.py index b4c9db3..52a685b 100644 --- a/src/modelscope_hub/errors.py +++ b/src/modelscope_hub/errors.py @@ -394,6 +394,40 @@ class NotSupportedError(HubError): } +# --------------------------------------------------------------------------- +# Server business-code -> exception mapping +# +# The HTTP status is not always faithful to the failure semantics: the legacy +# login endpoint answers 400 while meaning "authentication failed". Where the +# server publishes a business code, trust it over the status code. Register new +# codes in this table (and in the ModelScope error-code spec) rather than +# branching at the call site. +# --------------------------------------------------------------------------- +_BUSINESS_CODE_MAP: dict[int, type[APIError]] = { + # -> E3001, served with HTTP 400 by POST /api/v1/login on both sites + 10010103009: AuthenticationError, # AccessToken 无效或过期 + # -> E3026 + 10020101001: AlreadyExistsError, # 国内站 - 数据集已存在 + 10010101001: AlreadyExistsError, # 国内站 - 模型已存在 + 10010202004: AlreadyExistsError, # 国际站 - 名称已被使用 +} + + +def _business_code(body: Any) -> int | None: + """Return the numeric business code carried by a response body, if any.""" + if not isinstance(body, dict): + return None + raw = body.get("Code") + if raw is None: + raw = body.get("code") + if raw is None: + return None + try: + return int(raw) + except (TypeError, ValueError): + return None + + _CN_TO_EN: dict[str, str] = { "该名称已被注册使用，请重新命名": "Repository name already exists. Please choose a different name.", "用户未登录": "User not logged in.", @@ -479,21 +513,20 @@ def raise_for_status(response: Response) -> None: else: exc_cls = _STATUS_MAP.get(status, APIError) - # Detect "already exists" errors before falling back to InvalidParameter - if exc_cls is InvalidParameter and isinstance(body, dict): - code = body.get("Code") or body.get("code") + # A published business code describes a client-side failure more faithfully + # than the HTTP status, so it wins for 4xx. It deliberately does not apply to + # 5xx: a server outage must stay retryable even if the body happens to carry + # a known code, and reclassifying it would silently drop that retryability. + business_cls: type[APIError] | None = None + if status < 500: + code = _business_code(body) + business_cls = _BUSINESS_CODE_MAP.get(code) if code is not None else None + if business_cls is not None: + exc_cls = business_cls + elif exc_cls is InvalidParameter and isinstance(body, dict): + # Older servers signal "already exists" through the message only. msg_text = (body.get("Message") or body.get("message") or body.get("msg") or body.get("Msg") or "").lower() - is_exists = False - if code is not None: - try: - if int(code) in _ALREADY_EXISTS_CODES: - is_exists = True - except (TypeError, ValueError): - pass - if not is_exists: - if any(kw in msg_text for kw in _ALREADY_EXISTS_KEYWORDS): - is_exists = True - if is_exists: + if any(kw in msg_text for kw in _ALREADY_EXISTS_KEYWORDS): exc_cls = AlreadyExistsError kwargs: dict[str, Any] = dict( @@ -523,11 +556,12 @@ def raise_for_status(response: Response) -> None: # --------------------------------------------------------------------------- # Repo-exists detection (shared by cli/repo.py and compat/hub_api.py) # --------------------------------------------------------------------------- -_ALREADY_EXISTS_CODES: set[int] = { - 10020101001, # 国内站 - 数据集已存在 - 10010101001, # 国内站 - 模型已存在 - 10010202004, # 国际站 - 名称已被使用 -} +# Derived from the business-code table so the two never drift apart. Retained +# as a module-level name because :func:`is_repo_exists_error` still consults it +# when handling exceptions that pre-date the structured hierarchy. +_ALREADY_EXISTS_CODES: frozenset[int] = frozenset( + code for code, exc in _BUSINESS_CODE_MAP.items() if exc is AlreadyExistsError +) _ALREADY_EXISTS_KEYWORDS: frozenset[str] = frozenset( { diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index 4abce64..96e51a7 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -7,7 +7,7 @@ import pytest -from modelscope_hub.cli.main import run_cmd +from modelscope_hub.cli.main import _report_hub_error, run_cmd from modelscope_hub.errors import HubError, InvalidParameter, NetworkError, NotSupportedError from .conftest import run_cli @@ -85,6 +85,82 @@ def test_global_flags_before_subcommand(self, parser): assert args.verbose is True +# --------------------------------------------------------------------------- +# Error cause reporting (pure unit -- no API, so it runs in every mode) +# --------------------------------------------------------------------------- +class TestErrorCauseReporting: + """``--verbose`` must expose the failure that a wrapper exception hides. + + Without this, a wrapped SDK error showed only the outermost message and the + originating cause was unreachable from the CLI in any mode. + """ + + @staticmethod + def _wrapped_error() -> HubError: + """Build a three-level chain the way the SDK layers do.""" + try: + try: + try: + raise ValueError("socket closed") + except ValueError as root: + raise NetworkError("connection refused") from root + except NetworkError as mid: + raise HubError("outer failure") from mid + except HubError as exc: + return exc + + def test_cause_chain_hidden_without_verbose(self, capsys): + _report_hub_error(self._wrapped_error(), verbose=False) + captured = capsys.readouterr() + assert "outer failure" in captured.err + assert "Caused by" not in captured.out + + def test_cause_chain_shown_with_verbose(self, capsys): + _report_hub_error(self._wrapped_error(), verbose=True) + out = capsys.readouterr().out + # SDK errors render their own error code, so match on class + message. + assert "Caused by: NetworkError:" in out + assert "connection refused" in out + assert "Caused by: ValueError: socket closed" in out + + def test_suppressed_context_is_respected(self, capsys): + """``raise ... from None`` deliberately hides the context.""" + try: + try: + raise ValueError("hidden detail") + except ValueError: + raise HubError("clean failure") from None + except HubError as exc: + _report_hub_error(exc, verbose=True) + captured = capsys.readouterr() + assert "clean failure" in captured.err + assert "Caused by" not in captured.out + + def test_self_referential_chain_terminates(self, capsys): + """A cyclic chain must not stall the error path.""" + exc = HubError("looping failure") + exc.__cause__ = exc + + _report_hub_error(exc, verbose=True) + + assert "looping failure" in capsys.readouterr().err + + def test_chain_depth_is_bounded(self, capsys): + """Only the first *max_depth* causes are rendered.""" + exc = HubError("level-0") + current: BaseException = exc + for level in range(1, 8): + nested = ValueError(f"level-{level}") + current.__cause__ = nested + current = nested + + _report_hub_error(exc, verbose=True, max_depth=3) + + out = capsys.readouterr().out + assert out.count("Caused by") == 3 + assert "level-4" not in out + + # --------------------------------------------------------------------------- # Exception handling (unit tests with mocks — no API needed) # --------------------------------------------------------------------------- diff --git a/tests/test_credential_lifecycle.py b/tests/test_credential_lifecycle.py new file mode 100644 index 0000000..9c87cfe --- /dev/null +++ b/tests/test_credential_lifecycle.py @@ -0,0 +1,112 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Regression guards for credential teardown. + +Covers two defects that left the SDK half-authenticated: + +* ``clear_token`` deleted only the session cookie, so the git token and the + cached identity outlived the login they belonged to. +* :attr:`HubApi.legacy` refused to propagate a *cleared* token to an already + constructed client, which then kept authenticating with the revoked + credential. +""" + +from __future__ import annotations + +import pytest + +from modelscope_hub.api import HubApi +from modelscope_hub.config import _CREDENTIAL_FILE_NAMES, HubConfig +from modelscope_hub.constants import ( + COOKIES_FILE_NAME, + GIT_TOKEN_FILE_NAME, + SESSION_FILE_NAME, + USER_INFO_FILE_NAME, +) + +TOKEN = "ms-token-under-test" +GIT_TOKEN = "git-token-value" + + +@pytest.fixture(autouse=True) +def isolated_home(tmp_path, monkeypatch): + """Redirect credential storage and drop ambient endpoint/token overrides.""" + for name in ("MODELSCOPE_ENDPOINT", "MODELSCOPE_API_TOKEN", "MODELSCOPE_DOMAIN"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("MODELSCOPE_HOME", str(tmp_path)) + return tmp_path + + +def fully_logged_in(home) -> HubConfig: + """Write every artefact that a successful login leaves behind.""" + config = HubConfig(config_dir=home) + config.save_token(TOKEN) + config.save_git_token(GIT_TOKEN) + config.save_user_info("alice", "alice@example.com") + config.get_session_id() # materialises the session file + for name in (COOKIES_FILE_NAME, GIT_TOKEN_FILE_NAME, USER_INFO_FILE_NAME, SESSION_FILE_NAME): + assert (config.credentials_dir / name).exists(), name + return config + + +# --------------------------------------------------------------------------- +# All-or-nothing teardown +# --------------------------------------------------------------------------- +def test_clear_token_removes_every_credential_artefact(isolated_home): + """A partial wipe used to leave a working git token behind.""" + config = fully_logged_in(isolated_home) + + config.clear_token() + + for name in _CREDENTIAL_FILE_NAMES: + assert not (config.credentials_dir / name).exists(), name + assert config.load_token() is None + assert config.load_git_token() is None + + +def test_clear_token_keeps_the_anonymous_session_id(isolated_home): + """The session id is an install identifier, not a credential.""" + config = fully_logged_in(isolated_home) + session_before = config.get_session_id() + + config.clear_token() + + assert (config.credentials_dir / SESSION_FILE_NAME).exists() + assert config.get_session_id() == session_before + + +def test_logout_clears_persisted_state(isolated_home): + """``HubApi.logout`` goes through the same all-or-nothing teardown.""" + fully_logged_in(isolated_home) + api = HubApi(config=HubConfig(config_dir=isolated_home)) + assert api._config.token == TOKEN + + api.logout() + + reloaded = HubConfig(config_dir=isolated_home) + assert reloaded.load_token() is None + assert reloaded.load_git_token() is None + + +# --------------------------------------------------------------------------- +# Cached client stays in step with the configured token +# --------------------------------------------------------------------------- +def test_cleared_token_propagates_to_the_cached_legacy_client(isolated_home): + """A cached client must not keep using a credential that was revoked.""" + fully_logged_in(isolated_home) + api = HubApi(config=HubConfig(config_dir=isolated_home)) + assert api.legacy.token == TOKEN # materialise the client + + api._config.clear_token() + + assert api.legacy.token is None + + +def test_rotated_token_propagates_to_the_cached_legacy_client(isolated_home): + """The pre-existing propagation path keeps working.""" + fully_logged_in(isolated_home) + api = HubApi(config=HubConfig(config_dir=isolated_home)) + assert api.legacy.token == TOKEN + + api._config.token = "ms-rotated" + + assert api.legacy.token == "ms-rotated" diff --git a/tests/test_login_failure_paths.py b/tests/test_login_failure_paths.py new file mode 100644 index 0000000..cfb880c --- /dev/null +++ b/tests/test_login_failure_paths.py @@ -0,0 +1,251 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Regression guards for the ``HubApi.login`` failure paths. + +These tests stub the ``requests`` transport only, so the real +:class:`~modelscope_hub.config.HubConfig`, :class:`~modelscope_hub.api.HubApi`, +:class:`~modelscope_hub._legacy_api.LegacyClient` and error-translation layers +all take part. Mocking any higher would hide precisely the defects covered +here: a rejected login used to surface as a fabricated "token rejected" +message -- losing the server's own explanation and request id -- while +deleting the credential the caller already had on disk. + +The payloads below are the ones ``POST /api/v1/login`` actually returns. +""" + +from __future__ import annotations + +import json + +import pytest +import requests +import responses + +from modelscope_hub.api import HubApi +from modelscope_hub.config import HubConfig +from modelscope_hub.constants import DEFAULT_ENDPOINT, DEFAULT_INTL_ENDPOINT +from modelscope_hub.errors import ( + AlreadyExistsError, + AuthenticationError, + InvalidParameter, + NetworkError, + ServerError, + raise_for_status, +) + +CN_LOGIN = f"{DEFAULT_ENDPOINT}/api/v1/login" +AI_LOGIN = f"{DEFAULT_INTL_ENDPOINT}/api/v1/login" + +# Both sites answer an unknown token with this same business code, which is why +# the server cannot tell "invalid token" apart from "token issued elsewhere". +TOKEN_REJECTED_BODY = { + "Code": 10010103009, + "Message": "登录失败，AccessToken错误，请从用户中心获取AccessToken或刷新", + "RequestId": "8a039827-3f7c-4378-9b7c-3f8341b73649", + "Success": False, +} + +LOGIN_OK_BODY = { + "Code": 200, + "Data": {"AccessToken": "git-token", "Email": "alice@example.com", "Username": "alice"}, + "Message": "success", + "RequestId": "b008966a-942f-4c05-8f8a-696d1b6cc2e2", + "Success": True, +} + +PRIOR_TOKEN = "ms-previously-working" + + +@pytest.fixture(autouse=True) +def isolated_home(tmp_path, monkeypatch): + """Redirect credential storage and drop ambient endpoint/token overrides.""" + for name in ("MODELSCOPE_ENDPOINT", "MODELSCOPE_API_TOKEN", "MODELSCOPE_DOMAIN"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("MODELSCOPE_HOME", str(tmp_path)) + return tmp_path + + +def unpinned_api(home) -> HubApi: + """Build a HubApi with no explicit endpoint, so the peer probe may run.""" + return HubApi(config=HubConfig(config_dir=home)) + + +def seed_stored_credential(home) -> None: + """Persist a working credential the way a previous login would have.""" + config = HubConfig(config_dir=home) + config.save_token(PRIOR_TOKEN) + assert config.load_token() == PRIOR_TOKEN + + +def stored_token(home) -> str | None: + return HubConfig(config_dir=home).load_token() + + +# --------------------------------------------------------------------------- +# Faithful attribution +# --------------------------------------------------------------------------- +@responses.activate +def test_rejected_token_keeps_server_explanation(isolated_home): + """The server's message, business code and request id must survive.""" + responses.add(responses.POST, CN_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + responses.add(responses.POST, AI_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + + with pytest.raises(AuthenticationError) as excinfo: + unpinned_api(isolated_home).login("ms-bad-token") + + exc = excinfo.value + # Business code 10010103009 outranks the HTTP 400 the server chose. + assert exc.error_code == "E3001" + assert exc.status_code == 400 + assert exc.request_id == TOKEN_REJECTED_BODY["RequestId"] + assert TOKEN_REJECTED_BODY["Message"] in exc.message + + +@responses.activate +def test_network_failure_is_not_reported_as_bad_token(isolated_home): + """Transport errors keep their own identity instead of blaming the token.""" + seed_stored_credential(isolated_home) + responses.add(responses.POST, CN_LOGIN, body=requests.ConnectionError("connection reset")) + + with pytest.raises(NetworkError) as excinfo: + HubApi(config=HubConfig(config_dir=isolated_home)).login("ms-any-token") + + assert not isinstance(excinfo.value, AuthenticationError) + assert stored_token(isolated_home) == PRIOR_TOKEN + + +@responses.activate +def test_server_error_propagates_unchanged(isolated_home): + """A 5xx is a server outage, not a credential problem.""" + responses.add(responses.POST, CN_LOGIN, json={"Code": 500, "Message": "internal"}, status=500) + + with pytest.raises(ServerError) as excinfo: + unpinned_api(isolated_home).login("ms-any-token") + + assert excinfo.value.error_code == "E1002" + + +# --------------------------------------------------------------------------- +# Stored credentials are never collateral damage +# --------------------------------------------------------------------------- +@responses.activate +def test_failed_login_keeps_stored_credential(isolated_home): + """A mistyped token must not log the user out of a working session.""" + seed_stored_credential(isolated_home) + responses.add(responses.POST, CN_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + responses.add(responses.POST, AI_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + + api = HubApi(config=HubConfig(config_dir=isolated_home)) + with pytest.raises(AuthenticationError): + api.login("ms-bad-token") + + assert stored_token(isolated_home) == PRIOR_TOKEN + # The instance is also rewound, not left holding the rejected token. + assert api._config.token == PRIOR_TOKEN + + +# --------------------------------------------------------------------------- +# Site disambiguation +# --------------------------------------------------------------------------- +@responses.activate +def test_token_valid_on_peer_site_yields_endpoint_hint(isolated_home): + """A token issued by the other site gets an actionable hint, not a verdict.""" + responses.add(responses.POST, CN_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + responses.add(responses.POST, AI_LOGIN, json=LOGIN_OK_BODY, status=200) + + with pytest.raises(AuthenticationError) as excinfo: + unpinned_api(isolated_home).login("ms-intl-token") + + message = excinfo.value.message + assert DEFAULT_INTL_ENDPOINT in message + assert "--endpoint" in message + + +@responses.activate +def test_pinned_endpoint_is_not_second_guessed(isolated_home): + """An explicit endpoint is respected: no peer probe and no hint.""" + responses.add(responses.POST, CN_LOGIN, json=TOKEN_REJECTED_BODY, status=400) + + api = HubApi(config=HubConfig(config_dir=isolated_home), endpoint=DEFAULT_ENDPOINT) + with pytest.raises(AuthenticationError) as excinfo: + api.login("ms-bad-token") + + assert "--endpoint" not in excinfo.value.message + assert all(AI_LOGIN not in call.request.url for call in responses.calls) + + +# --------------------------------------------------------------------------- +# Classification table +# --------------------------------------------------------------------------- +def json_response(status: int, body: dict) -> requests.Response: + """Build a minimal response the error layer can classify.""" + resp = requests.Response() + resp.status_code = status + resp._content = json.dumps(body).encode() + resp.headers["Content-Type"] = "application/json" + resp.url = CN_LOGIN + return resp + + +@pytest.mark.parametrize( + "body, expected", + [ + ({"Code": 10010103009, "Message": "token bad"}, AuthenticationError), + ({"Code": 10010101001, "Message": "model exists"}, AlreadyExistsError), + ({"Code": 99999999999, "Message": "something else"}, InvalidParameter), + ({"Message": "no code at all"}, InvalidParameter), + ], +) +def test_business_code_outranks_http_status(body, expected): + """A published business code classifies the failure; status is the fallback.""" + with pytest.raises(expected) as excinfo: + raise_for_status(json_response(400, body)) + # AlreadyExistsError subclasses InvalidParameter, so assert the exact type. + assert type(excinfo.value) is expected + + +@pytest.mark.parametrize("code", [10010103009, 10010101001]) +def test_business_code_does_not_override_a_server_outage(code): + """A 5xx stays a retryable ServerError even when the body carries a known code. + + Reclassifying it would flip ``retryable`` to False and silently turn a + transient outage into a permanent failure. + """ + with pytest.raises(ServerError) as excinfo: + raise_for_status(json_response(500, {"Code": code, "Message": "upstream failure"})) + + assert type(excinfo.value) is ServerError + assert excinfo.value.retryable is True + + +# --------------------------------------------------------------------------- +# Endpoint normalisation +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "given, expected", + [ + ("modelscope.ai", "https://modelscope.ai"), + ("https://modelscope.cn/", "https://modelscope.cn"), + (" modelscope.cn ", "https://modelscope.cn"), + ("http://localhost:8080", "http://localhost:8080"), + # URI schemes are case-insensitive: recognise, do not prefix again. + ("HTTPS://modelscope.cn", "HTTPS://modelscope.cn"), + ("Http://localhost:8080", "Http://localhost:8080"), + ], +) +def test_bare_endpoint_gains_a_scheme(isolated_home, given, expected): + """Bare domains used to reach the transport layer and fail there.""" + api = HubApi(config=HubConfig(config_dir=isolated_home), endpoint=given) + assert api._config.endpoint == expected + + +@responses.activate +def test_bare_endpoint_reaches_the_expected_url(isolated_home): + """End-to-end proof that a scheme-less endpoint now resolves correctly.""" + url = "https://modelscope.ai/api/v1/login" + responses.add(responses.POST, url, json=TOKEN_REJECTED_BODY, status=400) + + api = HubApi(config=HubConfig(config_dir=isolated_home), endpoint="modelscope.ai") + with pytest.raises(AuthenticationError): + api.login("ms-bad-token") + + assert responses.calls[0].request.url == url