From dc48b1d6f36257a9b7d33a7403de1cefa9486aab Mon Sep 17 00:00:00 2001
From: serramatutu <lucas.paes@dbtlabs.com>
Date: Tue, 8 Oct 2024 16:05:00 +0200
Subject: [PATCH] refactor: fix error messages

This commit adds explicit logging of `self.args` in `SemanticLayerError`
so that users get a proper display of the underlying error when they
`str(e)` or `repr(e)`.

It also adds `raise from` in `_handle_error` to add more context to the
exceptions raised via ADBC.
---
 .../unreleased/Under the Hood-20241008-160709.yaml   |  3 +++
 dbtsl/api/adbc/client/base.py                        |  6 +++---
 dbtsl/error.py                                       |  6 +++++-
 tests/test_error.py                                  | 12 ++++++++----
 4 files changed, 19 insertions(+), 8 deletions(-)
 create mode 100644 .changes/unreleased/Under the Hood-20241008-160709.yaml

diff --git a/.changes/unreleased/Under the Hood-20241008-160709.yaml b/.changes/unreleased/Under the Hood-20241008-160709.yaml
new file mode 100644
index 0000000..4613119
--- /dev/null
+++ b/.changes/unreleased/Under the Hood-20241008-160709.yaml	
@@ -0,0 +1,3 @@
+kind: Under the Hood
+body: Improve error display
+time: 2024-10-08T16:07:09.766189+02:00
diff --git a/dbtsl/api/adbc/client/base.py b/dbtsl/api/adbc/client/base.py
index 4961756..b75069a 100644
--- a/dbtsl/api/adbc/client/base.py
+++ b/dbtsl/api/adbc/client/base.py
@@ -52,15 +52,15 @@ def _get_connection_context_manager(self) -> AbstractContextManager[Connection]:
     def _handle_error(self, err: Exception) -> None:
         if isinstance(err, ProgrammingError):
             if err.status_code in (AdbcStatusCode.UNAUTHENTICATED, AdbcStatusCode.UNAUTHORIZED):
-                raise AuthError(err.args)
+                raise AuthError(err.args) from err
 
             if err.status_code == AdbcStatusCode.INVALID_ARGUMENT:
-                raise QueryFailedError(err.args)
+                raise QueryFailedError(err.args) from err
 
             # TODO: timeouts are not implemented for ADBC
             # See: https://arrow.apache.org/adbc/current/driver/flight_sql.html#timeouts
             if err.status_code == AdbcStatusCode.TIMEOUT:
-                raise TimeoutError()
+                raise TimeoutError() from err
 
         raise err
 
diff --git a/dbtsl/error.py b/dbtsl/error.py
index 864ed51..f26aeef 100644
--- a/dbtsl/error.py
+++ b/dbtsl/error.py
@@ -1,9 +1,13 @@
+import json
+
+
 class SemanticLayerError(RuntimeError):
     """Any errors related to the Semantic Layer inherit from this."""
 
     def __str__(self) -> str:
         """RuntimeError doesn't stringify itself by default, so we need to manually add this."""
-        return self.__class__.__name__
+        args_str = "" if len(self.args) == 0 else ", ".join(json.dumps(a) for a in self.args)
+        return f"{self.__class__.__name__}({args_str})"
 
     def __repr__(self) -> str:
         """RuntimeError doesn't stringify itself by default, so we need to manually add this."""
diff --git a/tests/test_error.py b/tests/test_error.py
index 450130b..c72bd43 100644
--- a/tests/test_error.py
+++ b/tests/test_error.py
@@ -1,12 +1,16 @@
 from dbtsl.error import SemanticLayerError, TimeoutError
 
 
-def test_error_str() -> None:
-    assert str(SemanticLayerError()) == "SemanticLayerError"
+def test_error_str_calls_repr() -> None:
+    assert str(SemanticLayerError()) == repr(SemanticLayerError())
 
 
-def test_error_repr() -> None:
-    assert repr(SemanticLayerError()) == "SemanticLayerError"
+def test_error_repr_no_args() -> None:
+    assert repr(SemanticLayerError()) == "SemanticLayerError()"
+
+
+def test_error_repr_with_args() -> None:
+    assert repr(SemanticLayerError(1, 2, "a")) == 'SemanticLayerError(1, 2, "a")'
 
 
 def test_timeout_error_str() -> None: