Skip to content

Commit 11579f5

Browse files
ai: apply changes for #876 (1 review thread)
Addresses: - #3635880990 at src/databricks/sql/auth/oauth.py:145 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 3cb80da commit 11579f5

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

src/databricks/sql/auth/oauth.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,24 @@ def __get_authorization_code(self, client, auth_url, scope, state, challenge):
136136
handler = OAuthHttpSingleRequestHandler("Databricks Sql Connector")
137137

138138
last_error = None
139+
callback_timed_out = False
139140
for port in self.port_range:
140141
try:
141142
with HTTPServer(("", port), handler) as httpd:
142143
# Bound how long we wait for the browser redirect callback so
143144
# that a headless environment (no browser to complete the
144145
# flow) fails with a clear error instead of hanging forever.
145146
httpd.timeout = self.REDIRECT_CALLBACK_TIMEOUT_SECONDS
147+
148+
# HTTPServer.handle_request() returns normally (via
149+
# handle_timeout()) when the wait elapses without a
150+
# connection, so record that case to distinguish it from a
151+
# received-but-empty callback below.
152+
def _on_timeout():
153+
nonlocal callback_timed_out
154+
callback_timed_out = True
155+
156+
httpd.handle_timeout = _on_timeout
146157
redirect_url = OAuthManager.__get_redirect_url(port)
147158
auth_req_uri, _, _ = client.prepare_authorization_request(
148159
authorization_url=auth_url,
@@ -174,7 +185,16 @@ def __get_authorization_code(self, client, auth_url, scope, state, challenge):
174185
raise last_error
175186

176187
if not handler.request_path:
177-
msg = f"No path parameters were returned to the callback at {redirect_url}"
188+
if callback_timed_out:
189+
msg = (
190+
f"Timed out after {self.REDIRECT_CALLBACK_TIMEOUT_SECONDS} "
191+
f"seconds waiting for the OAuth redirect callback at "
192+
f"{redirect_url}. No browser completed the login flow — this "
193+
"is expected in a headless environment (e.g. a notebook or "
194+
"job with no browser). See issue #458."
195+
)
196+
else:
197+
msg = f"No path parameters were returned to the callback at {redirect_url}"
178198
logger.error(msg)
179199
raise RuntimeError(msg)
180200
# This is a kludge because the parsing library expects https callbacks

tests/unit/test_auth.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,13 @@ def run():
262262
)
263263
self.assertEqual(result.get("outcome"), "raised")
264264
self.assertIsInstance(result.get("error"), RuntimeError)
265-
self.assertIn("No path parameters were returned", str(result.get("error")))
265+
# The headless timeout path must surface a clear, timeout-specific error
266+
# (not the generic received-but-empty callback message). See issue #458.
267+
error_message = str(result.get("error"))
268+
self.assertIn("Timed out", error_message)
269+
self.assertIn(
270+
str(oauth_manager.REDIRECT_CALLBACK_TIMEOUT_SECONDS), error_message
271+
)
266272

267273

268274
class TestClientCredentialsTokenSource:

0 commit comments

Comments
 (0)