databricks / databricks-sql-python

Databricks SQL Connector for Python
Apache License 2.0
153 stars 89 forks source link

Bug in `databricks.sql.exc.Error` base class #437

Open chidifrank opened 1 week ago

chidifrank commented 1 week ago

Problem: When try except catching errors and logging them I occasionally get this TypeError

...
    try:
        with connection.cursor() as cursor:
            cursor.execute(query)
            ...
    except databricks.sql.Error as e:
        logger.debug(f"{str(e)}: failed to execute query {query}")
        ...

__str__ returned non-string (type NoneType)

Explanation:

String representations of a class must be of type string

the message attribute can be None therefore there needs to be a check/cast prior to returning.

Note: message_with_context will also fail when a message is None since you can't use the operand type + for NoneType and a str

class Error(Exception):
    """Base class for DB-API2.0 exceptions.
    `message`: An optional user-friendly error message. It should be short, actionable and stable
    `context`: Optional extra context about the error. MUST be JSON serializable
    """

    def __init__(self, message=None, context=None, *args, **kwargs):
        super().__init__(message, *args, **kwargs)
        self.message = message
        self.context = context or {}

    def __str__(self):
        return self.message

    def message_with_context(self):
        return self.message + ": " + json.dumps(self.context, default=str)
kravets-levko commented 1 week ago

@chidifrank thank you for reporting this! May I ask you to share a stacktrace for this error so I can understand where it comes from? None of that errors should be raised without a message, so your case indicates that something is wrong elsewhere

chidifrank commented 1 week ago

@kravets-levko Sure!


---------------------------------------------------------------------------
ServerOperationError

File /local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.10/site-packages/databricks/sql/client.py:768, in Cursor.execute(self, operation, parameters)
    767 self._close_and_clear_active_result_set()
--> 768 execute_response = self.thrift_backend.execute_command(
    769     operation=prepared_operation,
    770     session_handle=self.connection._session_handle,
    771     max_rows=self.arraysize,
    772     max_bytes=self.buffer_size_bytes,
    773     lz4_compression=self.connection.lz4_compression,
    774     cursor=self,
    775     use_cloud_fetch=self.connection.use_cloud_fetch,
    776     parameters=prepared_params,
    777 )
    778 self.active_result_set = ResultSet(
    779     self.connection,
    780     execute_response,
   (...)
    783     self.arraysize,
    784 )

File /local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.10/site-packages/databricks/sql/thrift_backend.py:869, in ThriftBackend.execute_command(self, operation, session_handle, max_rows, max_bytes, lz4_compression, cursor, use_cloud_fetch, parameters)
    868 resp = self.make_request(self._client.ExecuteStatement, req)
--> 869 return self._handle_execute_response(resp, cursor)

File /local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.10/site-packages/databricks/sql/thrift_backend.py:961, in ThriftBackend._handle_execute_response(self, resp, cursor)
    959 self._check_direct_results_for_error(resp.directResults)
--> 961 final_operation_state = self._wait_until_command_done(
    962     resp.operationHandle,
    963     resp.directResults and resp.directResults.operationStatus,
    964 )
    966 return self._results_message_to_execute_response(resp, final_operation_state)

File /local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.10/site-packages/databricks/sql/thrift_backend.py:807, in ThriftBackend._wait_until_command_done(self, op_handle, initial_operation_status_resp)
    806     operation_state = poll_resp.operationState
--> 807     self._check_command_not_in_error_or_closed_state(op_handle, poll_resp)
    808 return operation_state

File /local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.10/site-packages/databricks/sql/thrift_backend.py:607, in ThriftBackend._check_command_not_in_error_or_closed_state(self, op_handle, get_operations_resp)
    606     else:
--> 607         raise ServerOperationError(
    608             get_operations_resp.errorMessage,
    609             {
    610                 "operation-id": op_handle
    611                 and self.guid_to_hex_id(op_handle.operationId.guid),
    612                 "diagnostic-info": None,
    613             },
    614         )
    615 elif get_operations_resp.operationState == ttypes.TOperationState.CLOSED_STATE:

<class 'str'>: (<class 'TypeError'>, TypeError('__str__ returned non-string (type NoneType)'))```
kravets-levko commented 1 week ago

Thank you! Yeah, looks that it is initialized with the error message from server, but it actually may be empty. We need additional check there, as well as a reasonable fallback (probably)

chidifrank commented 1 week ago

Thanks for the effort! @kravets-levko