Faithful login errors and safer credential handling

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.
 

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
 

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)