Fixes open-metadata#31459: actionable errors when the OMeta client cannot start (open-metadata#31461)

* Fixes open-metadata#31459: actionable errors when the OMeta client cannot start

Every workflow starts with GET /system/version. When that call did not
return the expected JSON, the client returned the raw `Response` as if it
were the decoded body, and the user got `TypeError: 'Response' object is
not subscriptable` several frames from the real misconfiguration.

Name each failure mode instead:

- HTML body (hostPort missing `/api`, so the request hit the UI catch-all)
  now raises `HtmlResponseError` from the client itself, since no endpoint
  legitimately answers HTML. CSV and ODCS-YAML exports still get their
  `Response` back.
- `get_server_version` reads the raw response and classifies 401/403, 404,
  other non-2xx, non-JSON bodies, a missing `version` field and an
  exhausted retry budget, each naming the URL actually called.
- The version mismatch message now carries the remedy (which client
  version to install).
- `create_ometa_client` keeps the exception class, the host and the cause
  instead of flattening everything into `ValueError(str)`.
- `health_check` reuses `get_server_version` rather than repeating the
  same unguarded subscript.

`OMetaClientInitError` subclasses `ValueError` so existing callers keep
working. Drops the two subscript errors this fixes from the basedpyright
baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Address review: keep the HTML error provider-neutral in REST

Two findings on open-metadata#31461.

IceS2: `REST` is not OpenMetadata-specific — around 25 connectors import it
to call third-party APIs — so an HTML login page from, say, Superset would
have told the user to add `/api` to `hostPort`. The message raised by the
client is now provider-neutral, and callers that know which API they were
talking to pass a `hint`. `server_mixin`, which does know the target is an
OpenMetadata server, supplies the `hostPort`/`/api` advice.

gitar-bot: `_read_version` sniffed HTML with a bare `startswith("<")` while
the client also required an `<html` tag, so an XML or SVG error page was
reported as a UI page with a misleading hint. Both paths now share the
helper, promoted to `is_html_body`.

Adds tests for the neutral client message and for an XML body reaching the
"not JSON" branch rather than the HTML one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Make the HTML-response error opt-in per client

CI caught the real cost of raising from the transport layer:
test_ingestion_get_version_still_tolerates_a_non_json_reply failed because
`AirflowApiClient` shares this `REST` class and deliberately splits its two
paths — `get_version` (ingestion) parses a bad body to `{}`, while
`test_get_version` (connection gate) is strict via `get_raw`. Raising on
HTML broke the lenient one, which is exactly what that test guards.

Add `ClientConfig.raise_on_html`, default False, and set it only where the
OpenMetadata client is built. The OpenMetadata API never answers HTML, so
the guard still covers its endpoints; the ~20 connectors that construct
their own `ClientConfig` keep the behaviour they had.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: pmbrull

ingestion/.basedpyright/baseline.json

@@ -13686,24 +13686,6 @@
                 }
             }
         ],
-        "./src/metadata/ingestion/ometa/mixins/server_mixin.py": [
-            {
-                "code": "reportIndexIssue",
-                "range": {
-                    "startColumn": 26,
-                    "endColumn": 60,
-                    "lineCount": 1
-                }
-            },
-            {
-                "code": "reportOptionalSubscript",
-                "range": {
-                    "startColumn": 26,
-                    "endColumn": 60,
-                    "lineCount": 1
-                }
-            }
-        ],
         "./src/metadata/ingestion/ometa/mixins/service_mixin.py": [
             {
                 "code": "reportInvalidTypeVarUse",
@@ -15208,22 +15190,6 @@
                     "endColumn": 65,
                     "lineCount": 1
                 }
-            },
-            {
-                "code": "reportIndexIssue",
-                "range": {
-                    "startColumn": 22,
-                    "endColumn": 56,
-                    "lineCount": 1
-                }
-            },
-            {
-                "code": "reportOptionalSubscript",
-                "range": {
-                    "startColumn": 22,
-                    "endColumn": 56,
-                    "lineCount": 1
-                }
             }
         ],
         "./src/metadata/ingestion/ometa/sse_client.py": [

ingestion/src/metadata/ingestion/ometa/client.py

@@ -55,6 +55,71 @@ def __init__(self, method: str, url: object, cause: BaseException) -> None:
         self.cause = cause
 
 
+class HtmlResponseError(Exception):
+    """An HTML page came back where the API answers JSON.
+
+    The body is a web page, not an API response: the request reached a UI, a login
+    page or a proxy rather than the endpoint. Raised instead of handing the caller a
+    ``Response`` it would try to subscript.
+
+    ``REST`` is generic - connectors use it against third-party APIs too - so the
+    message stays provider-neutral. Callers that know which API they were talking to
+    pass a ``hint`` with the advice specific to it.
+    """
+
+    def __init__(self, url: object, status_code: int, hint: Optional[str] = None) -> None:  # noqa: UP045
+        super().__init__(
+            f"Got an HTML page instead of JSON from [{url}] (HTTP {status_code})."
+            " The endpoint served a web page, not an API response - check the configured"
+            " host/URL and that no proxy or login page is intercepting the call." + (f" {hint}" if hint else "")
+        )
+        self.url = url
+        self.status_code = status_code
+
+
+def is_html_body(resp: requests.Response) -> bool:
+    """Whether a non-JSON body is an HTML page.
+
+    Content type first; some proxies mislabel index.html as text/plain, so fall back
+    to sniffing an `<html` tag in the head of the body. CSV and ODCS-YAML exports are
+    legitimate non-JSON payloads and must not match.
+    """
+    if "html" in resp.headers.get("Content-Type", "").lower():
+        return True
+    head = resp.text[:2048].lstrip()
+    return head.startswith("<") and "<html" in head.lower()
+
+
+def _decode_body(resp: requests.Response, url: object, raise_on_html: bool = False):
+    """Decode a successful response body.
+
+    JSON when it parses; otherwise the ``Response`` itself, for the text payloads
+    some endpoints answer with (CSV and ODCS-YAML exports).
+
+    ``raise_on_html`` turns an HTML page into an ``HtmlResponseError`` instead. It
+    is opt-in because callers disagree on what HTML means: the OpenMetadata API
+    never answers it, but connectors share this client and some deliberately
+    tolerate a non-JSON reply on their ingestion path.
+    """
+    try:
+        return resp.json()
+    except JSONDecodeError as json_decode_error:
+        if raise_on_html and is_html_body(resp):
+            raise HtmlResponseError(url, resp.status_code) from json_decode_error
+        logger.debug(
+            "Non-JSON response (%s) from [%s] with content type [%s] returned as-is: %s",
+            resp.status_code,
+            url,
+            resp.headers.get("Content-Type", "unknown"),
+            json_decode_error,
+        )
+        return resp
+    except Exception as exc:
+        logger.debug(traceback.format_exc())
+        logger.warning(f"Unexpected error while returning response {resp} in json format - {exc}")
+    return None
+
+
 class APIError(Exception):
     """
     Represent API related error.
@@ -129,6 +194,10 @@ class ClientConfig(ConfigModel):
     user_agent: Optional[str] = None  # noqa: UP045
     raw_data: Optional[bool] = False  # noqa: UP045
     allow_redirects: Optional[bool] = False  # noqa: UP045
+    # Treat an HTML body as an error rather than handing the caller the raw
+    # Response. Off by default: connectors share this client against third-party
+    # APIs, and some tolerate a non-JSON reply on purpose.
+    raise_on_html: bool = False
     auth_token_mode: Optional[str] = "Bearer"  # noqa: UP045
     verify: Optional[Union[bool, str]] = None  # noqa: UP007, UP045
     cookies: Optional[Any] = None  # noqa: UP045
@@ -309,18 +378,7 @@ def _one_request(self, method: str, url: URL, opts: dict, retry: int, raw: bool
                 return resp
 
             if resp.text != "":
-                try:
-                    return resp.json()
-                except JSONDecodeError as json_decode_error:
-                    logger.debug(
-                        "Non-JSON response (%s) returned as-is: %s",
-                        resp.status_code,
-                        json_decode_error,
-                    )
-                    return resp
-                except Exception as exc:
-                    logger.debug(traceback.format_exc())
-                    logger.warning(f"Unexpected error while returning response {resp} in json format - {exc}")
+                return _decode_body(resp, url, self.config.raise_on_html)
 
         except HTTPError as http_error:
             # retry if we hit Rate Limit
@@ -336,6 +394,10 @@ def _one_request(self, method: str, url: URL, opts: dict, retry: int, raw: bool
                     raise APIError(error, http_error) from http_error
             else:
                 raise
+        except HtmlResponseError:
+            # Already carries the actionable message; the catch-all below would
+            # downgrade it to a warning and hand the caller a None.
+            raise
         except (
             requests.exceptions.ConnectionError,
             requests.exceptions.Timeout,

ingestion/src/metadata/ingestion/ometa/client_utils.py

@@ -27,6 +27,14 @@
 logger = ometa_logger()
 
 
+class OMetaClientInitError(ValueError):
+    """The OpenMetadata client could not be initialized.
+
+    Subclasses ValueError for backwards compatibility with callers that already
+    catch the flattened error this used to raise.
+    """
+
+
 def create_ometa_client(
     metadata_config: OpenMetadataConnection,
     user_agent: Optional[str] = None,  # noqa: UP045
@@ -50,8 +58,12 @@ def create_ometa_client(
         return metadata  # noqa: TRY300
     except Exception as exc:
         logger.debug(traceback.format_exc())
-        logger.warning(f"Wild error initialising the OMeta Client {exc}")
-        raise ValueError(exc)  # noqa: B904
+        # `raise ValueError(exc)` used to drop the class name, so a TypeError from a
+        # bad hostPort reached the user as a bare string with no hint of its origin.
+        raise OMetaClientInitError(
+            f"Could not initialize the OpenMetadata client against [{metadata_config.hostPort}]:"
+            f" {type(exc).__name__}: {exc}"
+        ) from exc
 
 
 def get_chart_entities_from_id(

ingestion/src/metadata/ingestion/ometa/mixins/server_mixin.py

@@ -16,18 +16,31 @@
 
 from typing import Optional
 
+import requests
+from requests.exceptions import JSONDecodeError
+
 from metadata.__version__ import (
     get_client_version,
     get_server_version_from_string,
     match_versions,
 )
 from metadata.generated.schema.settings.settings import Settings, SettingType
-from metadata.ingestion.ometa.client import REST
+from metadata.ingestion.ometa.client import REST, HtmlResponseError, is_html_body
 from metadata.ingestion.ometa.routes import ROUTES
 from metadata.utils.logger import ometa_logger
 
 logger = ometa_logger()
 
+VERSION_PATH = "/system/version"
+
+# `REST` cannot say this - connectors use it against third-party APIs - but here we
+# know the target is an OpenMetadata server, whose UI answers index.html for unknown
+# routes. A `hostPort` missing `/api` therefore lands on the UI with a 200.
+HOST_PORT_HINT = (
+    "The OpenMetadata UI answers index.html for unknown routes, so `hostPort` most likely"
+    " does not point at the API: check that it ends in `/api` (e.g. https://<host>/api)."
+)
+
 
 class VersionMismatchException(Exception):  # noqa: N818
     """
@@ -73,24 +86,72 @@ def get_server_version(self) -> str:
         Run endpoint /system/version to check server version
         :return: Server version
         """
+        response = self.client.get_raw(VERSION_PATH)
+        return get_server_version_from_string(self._read_version(response))
+
+    @staticmethod
+    def _read_version(response: Optional[requests.Response]) -> str:  # noqa: UP045
+        """Pull `version` out of a /system/version response, or say why we can't.
+
+        This is the first call every workflow makes, so it is where a wrong
+        `hostPort`, a bad token or an unreachable API shows up. Each case gets its
+        own message - the generic path used to end in a `TypeError` several frames
+        away from the actual misconfiguration.
+        """
+        if response is None:
+            # `_request` returns None once the 504/429 retry budget is exhausted.
+            raise VersionNotFoundException(
+                f"No response from {VERSION_PATH} after exhausting the retry budget."
+                " The server is unreachable or persistently returning 429/504."
+            )
+        url = response.url
+        if response.status_code in (401, 403):
+            raise VersionNotFoundException(
+                f"Not authorized to read [{url}] (HTTP {response.status_code})."
+                " Check the JWT token / auth provider set in `workflowConfig.openMetadataServerConfig`"
+                " and that the bot user is still active."
+            )
+        if response.status_code == 404:
+            raise VersionNotFoundException(
+                f"No OpenMetadata API found at [{url}] (HTTP 404)."
+                " Check `hostPort` and `apiVersion` in `workflowConfig.openMetadataServerConfig`."
+            )
+        if not response.ok:
+            raise VersionNotFoundException(
+                f"Cannot read the server version from [{url}] (HTTP {response.status_code}): {response.text[:500]}"
+            )
+
         try:
-            raw_version = self.client.get("/system/version")["version"]
-        except KeyError:
-            raise VersionNotFoundException(  # noqa: B904
-                "Cannot Find Version at api/v1/system/version."
-                + " If running the server in DEV mode locally, make sure to `mvn clean install`."
+            payload = response.json()
+        except JSONDecodeError as exc:
+            # `get_raw` skips the client's JSON handling, so classify the body here,
+            # with the same sniffing the client uses.
+            if is_html_body(response):
+                raise HtmlResponseError(url, response.status_code, hint=HOST_PORT_HINT) from exc
+            raise VersionNotFoundException(
+                f"The response from [{url}] is not JSON"
+                f" (content type [{response.headers.get('Content-Type', 'unknown')}]): {response.text[:500]}"
+            ) from exc
+
+        if not isinstance(payload, dict) or "version" not in payload:
+            raise VersionNotFoundException(
+                f"No `version` field in the response from [{url}]: {str(payload)[:500]}."
+                " If running the server in DEV mode locally, make sure to `mvn clean install`."
             )
-        return get_server_version_from_string(raw_version)
+        return payload["version"]
 
     def validate_versions(self) -> None:
         """
         Validate Server & Client versions. They should match.
         Otherwise, raise VersionMismatchException.
         """
         if not match_versions(self.server_version, self.client_version):
+            major_minor = ".".join(self.server_version.split(".")[:2])
             raise VersionMismatchException(
                 f"Server version is {self.server_version} vs. Client version {self.client_version}."
-                f" Major and minor versions should match."
+                f" Major and minor versions should match. Either install the matching client with"
+                f" `pip install 'openmetadata-ingestion~={major_minor}.0'` or point `hostPort` at a"
+                f" {self.client_version} server."
             )
 
     def log_server_version(self) -> None:

ingestion/src/metadata/ingestion/ometa/ometa_api.py

@@ -339,6 +339,9 @@ def __init__(
             extra_headers=extra_headers,
             auth_token=self._auth_provider.get_access_token,
             verify=get_verify_ssl(self.config.sslConfig),
+            # The OpenMetadata API never answers HTML, so a page here means the
+            # request reached the UI or a proxy instead of the API.
+            raise_on_html=True,
             **(additional_client_config_arguments or {}),
         )
 
@@ -1049,10 +1052,9 @@ def bulk_create_or_update(
 
     def health_check(self) -> bool:
         """
-        Run version api call. Return `true` if response is not None
+        Run version api call. Raises with an actionable message if the API is not reachable.
         """
-        raw_version = self.client.get("/system/version")["version"]
-        return raw_version is not None
+        return bool(self.get_server_version())
 
     def close(self):
         """