ai: apply changes for #876 (2 review threads)

Addresses:
  - #3636204521 at src/databricks/sql/auth/authenticators.py:66
  - #3636204532 at src/databricks/sql/auth/oauth.py:75

Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>

Author: peco-engineer-bot[bot]

src/databricks/sql/auth/authenticators.py

@@ -1,6 +1,6 @@
 import abc
 import logging
-from typing import Callable, Dict, List, Optional
+from typing import Callable, Dict, List
 from databricks.sql.common.http import HttpHeader
 from databricks.sql.auth.oauth import (
     OAuthManager,
@@ -63,7 +63,6 @@ def __init__(
         scopes: List[str],
         http_client,
         auth_type: str = "databricks-oauth",
-        redirect_callback_timeout_seconds: Optional[int] = None,
     ):
         try:
             idp_endpoint = get_oauth_endpoints(hostname, auth_type == "azure-oauth")
@@ -75,15 +74,11 @@ def __init__(
             # Convert to the corresponding scopes in the corresponding IdP
             cloud_scopes = idp_endpoint.get_scopes_mapping(scopes)
 
-            # Pass through the (optional) callback-timeout override so this
-            # private provider can surface it later if a public connection
-            # knob is added; None falls back to OAuthManager's default.
             self.oauth_manager = OAuthManager(
                 port_range=redirect_port_range,
                 client_id=client_id,
                 idp_endpoint=idp_endpoint,
                 http_client=http_client,
-                redirect_callback_timeout_seconds=redirect_callback_timeout_seconds,
             )
             self._hostname = hostname
             self._scopes_as_str = DatabricksOAuthProvider.SCOPE_DELIM.join(cloud_scopes)

src/databricks/sql/auth/oauth.py

@@ -212,6 +212,25 @@ def _on_timeout():
                 if e.errno == errno.EADDRINUSE:
                     logger.info(f"Port {port} is in use")
                     last_error = e
+            except webbrowser.Error as e:
+                # No browser could be launched at all (webbrowser.get() found no
+                # runnable browser). This is a strong, reliable headless signal
+                # (e.g. a notebook/job with no display), so fail fast with a
+                # clear error instead of waiting out the full redirect-callback
+                # timeout. We do NOT fail fast merely on a falsy open_new return
+                # value: that is unreliable across platforms (it can be falsy
+                # even when a browser did open) and would break working
+                # interactive logins. See issue #458. Placed before the generic
+                # ``except Exception`` so this RuntimeError propagates instead of
+                # being swallowed and retried against the remaining ports.
+                msg = (
+                    "Could not launch a web browser to complete the U2M OAuth "
+                    f"login flow ({e}). This looks like a headless environment "
+                    "(e.g. a notebook or job with no browser), where browser-"
+                    "based OAuth cannot complete. See issue #458."
+                )
+                logger.error(msg)
+                raise RuntimeError(msg) from e
             except Exception as e:
                 logger.error("unexpected error: %s", e)
         if self.redirect_port is None:

tests/unit/test_auth.py

@@ -381,6 +381,63 @@ def run():
             for sock in stalled_sockets:
                 sock.close()
 
+    @patch("databricks.sql.auth.oauth.webbrowser.open_new")
+    @patch.object(OAuthManager, "_OAuthManager__fetch_well_known_config")
+    def test_get_tokens_fails_fast_when_no_browser_available(
+        self, mock_fetch_config, mock_open_new
+    ):
+        """When no browser can be launched at all (webbrowser.open_new raises
+        webbrowser.Error), the flow must fail fast with a clear headless error
+        rather than waiting out the full redirect-callback timeout. See issue
+        #458."""
+        import webbrowser
+
+        mock_fetch_config.return_value = {
+            "authorization_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/authorize",
+            "token_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/token",
+        }
+        mock_open_new.side_effect = webbrowser.Error("no browser available")
+
+        # A large timeout: if the fail-fast path regressed, joining below would
+        # hit the wall-clock bound and the test would fail loudly instead of
+        # silently waiting minutes.
+        oauth_manager = OAuthManager(
+            port_range=[0],
+            client_id="mock-id",
+            idp_endpoint=InHouseOAuthEndpointCollection(),
+            http_client=MagicMock(),
+            redirect_callback_timeout_seconds=600,
+        )
+
+        import threading
+
+        result = {}
+
+        def run():
+            try:
+                oauth_manager.get_tokens(
+                    hostname="foo.cloud.databricks.com", scope="offline_access sql"
+                )
+                result["outcome"] = "returned"
+            except BaseException as e:  # noqa: BLE001
+                result["outcome"] = "raised"
+                result["error"] = e
+
+        worker = threading.Thread(target=run, daemon=True)
+        worker.start()
+        worker.join(timeout=30)
+
+        self.assertFalse(
+            worker.is_alive(),
+            "OAuth flow did not fail fast when no browser was available",
+        )
+        self.assertEqual(result.get("outcome"), "raised")
+        self.assertIsInstance(result.get("error"), RuntimeError)
+        error_message = str(result.get("error"))
+        self.assertIn("browser", error_message.lower())
+        # The fail-fast path must not surface the accept-wait timeout message.
+        self.assertNotIn("Timed out", error_message)
+
 
 class TestClientCredentialsTokenSource:
     @pytest.fixture