Skip to content

Commit 461e762

Browse files
simplify error handling
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
1 parent 3172ab8 commit 461e762

File tree

1 file changed

+7
-71
lines changed

1 file changed

+7
-71
lines changed

src/databricks/sql/backend/sea/utils/http_client.py

Lines changed: 7 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ def _make_request(
220220
method: str,
221221
path: str,
222222
data: Optional[Dict[str, Any]] = None,
223-
params: Optional[Dict[str, Any]] = None,
224223
) -> Dict[str, Any]:
225224
"""
226225
Make an HTTP request to the SEA endpoint.
@@ -229,7 +228,6 @@ def _make_request(
229228
method: HTTP method (GET, POST, DELETE)
230229
path: API endpoint path
231230
data: Request payload data
232-
params: Query parameters
233231
234232
Returns:
235233
Dict[str, Any]: Response data parsed from JSON
@@ -238,12 +236,6 @@ def _make_request(
238236
RequestError: If the request fails after retries
239237
"""
240238

241-
# Build full URL
242-
if path.startswith("/"):
243-
url = path
244-
else:
245-
url = f"/{path.lstrip('/')}"
246-
247239
# Prepare headers
248240
headers = {**self.headers, **self._get_auth_headers()}
249241

@@ -257,7 +249,7 @@ def _make_request(
257249
self.set_retry_command_type(command_type)
258250
self.start_retry_timer()
259251

260-
logger.debug(f"Making {method} request to {url}")
252+
logger.debug(f"Making {method} request to {path}")
261253

262254
# When v3 retries are enabled, urllib3 handles retries internally via DatabricksRetryPolicy
263255
# When disabled, we let exceptions bubble up (similar to Thrift backend approach)
@@ -267,7 +259,7 @@ def _make_request(
267259
try:
268260
response = self._pool.request(
269261
method=method.upper(),
270-
url=url,
262+
url=path,
271263
body=body,
272264
headers=headers,
273265
preload_content=False,
@@ -277,90 +269,34 @@ def _make_request(
277269
# MaxRetryDurationError is raised directly by DatabricksRetryPolicy
278270
# when duration limits are exceeded (like in test_retry_exponential_backoff)
279271
error_message = f"Request failed due to retry duration limit: {e}"
280-
# Construct RequestError with message, context, and specific error (like Thrift backend)
272+
# Construct RequestError with message, context, and specific error
281273
raise RequestError(error_message, None, e)
282274
except (SessionAlreadyClosedError, CursorAlreadyClosedError) as e:
283275
# These exceptions are raised by DatabricksRetryPolicy when detecting
284276
# "already closed" scenarios (404 responses with retry history)
285277
error_message = f"Request failed: {e}"
286-
# Construct RequestError with proper 3-argument format (message, context, error) like Thrift backend
278+
# Construct RequestError with proper 3-argument format (message, context, error)
287279
raise RequestError(error_message, None, e)
288280
except MaxRetryError as e:
289281
# urllib3 MaxRetryError should bubble up for redirect tests to catch
290-
# Don't convert to RequestError, let the test framework handle it
291282
logger.error(f"SEA HTTP request failed with MaxRetryError: {e}")
292283
raise
293284
except Exception as e:
294-
# Broad exception handler like Thrift backend to catch any unexpected errors
295-
# (including test mocking issues like StopIteration)
296285
logger.error(f"SEA HTTP request failed with exception: {e}")
297286
error_message = f"Error during request to server. {e}"
298-
# Construct RequestError with proper 3-argument format (message, context, error) like Thrift backend
287+
# Construct RequestError with proper 3-argument format (message, context, error)
299288
raise RequestError(error_message, None, e)
300289

301290
logger.debug(f"Response status: {response.status}")
302291

303292
# Handle successful responses
304293
if 200 <= response.status < 300:
305-
if response.data:
306-
try:
307-
result = json.loads(response.data.decode("utf-8"))
308-
logger.debug("Successfully parsed JSON response")
309-
return result
310-
except (json.JSONDecodeError, UnicodeDecodeError) as e:
311-
logger.error(f"Failed to parse JSON response: {e}")
312-
raise RequestError(f"Invalid JSON response: {e}", e)
313-
return {}
314-
315-
# Handle error responses
316-
error_message = f"SEA HTTP request failed with status {response.status}"
294+
return response.json()
317295

318-
# Try to extract additional error details from response, but don't fail if we can't
319-
error_message = self._try_add_error_details_to_message(response, error_message)
296+
error_message = f"SEA HTTP request failed with status {response.status}"
320297

321298
raise RequestError(error_message, None)
322299

323-
def _try_add_error_details_to_message(self, response, error_message: str) -> str:
324-
"""
325-
Try to extract error details from response and add to error message.
326-
This method is defensive and will not raise exceptions if parsing fails.
327-
It handles mock objects and malformed responses gracefully.
328-
"""
329-
try:
330-
# Check if response.data exists and is accessible
331-
if not hasattr(response, "data") or response.data is None:
332-
return error_message
333-
334-
# Try to decode the response data
335-
try:
336-
decoded_data = response.data.decode("utf-8")
337-
except (AttributeError, UnicodeDecodeError, TypeError):
338-
# response.data might be a mock object or not bytes
339-
return error_message
340-
341-
# Ensure we have a string before attempting JSON parsing
342-
if not isinstance(decoded_data, str):
343-
return error_message
344-
345-
# Try to parse as JSON
346-
try:
347-
error_details = json.loads(decoded_data)
348-
if isinstance(error_details, dict) and "message" in error_details:
349-
enhanced_message = f"{error_message}: {error_details['message']}"
350-
logger.error(f"Request failed: {error_details}")
351-
return enhanced_message
352-
except json.JSONDecodeError:
353-
# Not valid JSON, log what we can
354-
logger.debug(
355-
f"Request failed with non-JSON response: {decoded_data[:200]}"
356-
)
357-
358-
except Exception:
359-
# Catch-all for any unexpected issues (e.g., mock objects with unexpected behavior)
360-
logger.debug("Could not parse error response data")
361-
362-
return error_message
363-
364300
def _get_command_type_from_path(self, path: str, method: str) -> CommandType:
365301
"""
366302
Determine the command type based on the API path and method.

0 commit comments

Comments
 (0)