feat(exceptions): separate failed signin error (#1478)

* feat(exceptions): separate failed signin error Closes #1472 This makes sign in failures their own class of exceptions, while still inheriting from NotSignedInException to not break backwards compatability for any existing client code. This should allow users to get out more specific exceptions more easily on what failed with their authentication request. * fix(error): raise exception when ServerInfo.get fails If ServerInfoItem.from_response gets invalid XML, raise the error immediately instead of suppressing the error and setting an invalid version number * fix(test): add missing test asset --------- Co-authored-by: Jordan Woods <[email protected]>
tableau · Oct 10, 2024 · b49eac5 · b49eac5
1 parent e1b8281
commit b49eac5
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 14 deletions.
diff --git a/tableauserverclient/__init__.py b/tableauserverclient/__init__.py
@@ -56,6 +56,7 @@
     PDFRequestOptions,
     RequestOptions,
     MissingRequiredFieldError,
+    FailedSignInError,
     NotSignedInError,
     ServerResponseError,
     Filter,
@@ -79,6 +80,7 @@
     "DatabaseItem",
     "DataFreshnessPolicyItem",
     "DatasourceItem",
+    "FailedSignInError",
     "FavoriteItem",
     "FlowItem",
     "FlowRunItem",

diff --git a/tableauserverclient/models/server_info_item.py b/tableauserverclient/models/server_info_item.py
@@ -40,13 +40,11 @@ def from_response(cls, resp, ns):
         try:
             parsed_response = fromstring(resp)
         except xml.etree.ElementTree.ParseError as error:
-            logger.info(f"Unexpected response for ServerInfo: {resp}")
-            logger.info(error)
+            logger.exception(f"Unexpected response for ServerInfo: {resp}")
             return cls("Unknown", "Unknown", "Unknown")
         except Exception as error:
-            logger.info(f"Unexpected response for ServerInfo: {resp}")
-            logger.info(error)
-            return cls("Unknown", "Unknown", "Unknown")
+            logger.exception(f"Unexpected response for ServerInfo: {resp}")
+            raise error
 
         product_version_tag = parsed_response.find(".//t:productVersion", namespaces=ns)
         rest_api_version_tag = parsed_response.find(".//t:restApiVersion", namespaces=ns)

diff --git a/tableauserverclient/server/__init__.py b/tableauserverclient/server/__init__.py
@@ -11,7 +11,7 @@
 from tableauserverclient.server.sort import Sort
 from tableauserverclient.server.server import Server
 from tableauserverclient.server.pager import Pager
-from tableauserverclient.server.endpoint.exceptions import NotSignedInError
+from tableauserverclient.server.endpoint.exceptions import FailedSignInError, NotSignedInError
 
 from tableauserverclient.server.endpoint import (
     Auth,
@@ -57,6 +57,7 @@
     "Sort",
     "Server",
     "Pager",
+    "FailedSignInError",
     "NotSignedInError",
     "Auth",
     "CustomViews",

diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py
@@ -19,6 +19,7 @@
 from tableauserverclient.server.request_options import RequestOptions
 
 from tableauserverclient.server.endpoint.exceptions import (
+    FailedSignInError,
     ServerResponseError,
     InternalServerError,
     NonXMLResponseError,
@@ -160,7 +161,7 @@ def _check_status(self, server_response: "Response", url: Optional[str] = None):
             try:
                 if server_response.status_code == 401:
                     # TODO: catch this in server.py and attempt to sign in again, in case it's a session expiry
-                    raise NotSignedInError(server_response.content, url)
+                    raise FailedSignInError.from_response(server_response.content, self.parent_srv.namespace, url)
 
                 raise ServerResponseError.from_response(server_response.content, self.parent_srv.namespace, url)
             except ParseError:

diff --git a/tableauserverclient/server/endpoint/exceptions.py b/tableauserverclient/server/endpoint/exceptions.py
@@ -1,13 +1,20 @@
 from defusedxml.ElementTree import fromstring
-from typing import Optional
+from typing import Mapping, Optional, TypeVar
+
+
+def split_pascal_case(s: str) -> str:
+    return "".join([f" {c}" if c.isupper() else c for c in s]).strip()
 
 
 class TableauError(Exception):
     pass
 
 
-class ServerResponseError(TableauError):
-    def __init__(self, code, summary, detail, url=None):
+T = TypeVar("T")
+
+
+class XMLError(TableauError):
+    def __init__(self, code: str, summary: str, detail: str, url: Optional[str] = None) -> None:
         self.code = code
         self.summary = summary
         self.detail = detail
@@ -18,7 +25,7 @@ def __str__(self):
         return f"\n\n\t{self.code}: {self.summary}\n\t\t{self.detail}"
 
     @classmethod
-    def from_response(cls, resp, ns, url=None):
+    def from_response(cls, resp, ns, url):
         # Check elements exist before .text
         parsed_response = fromstring(resp)
         try:
@@ -33,6 +40,10 @@ def from_response(cls, resp, ns, url=None):
         return error_response
 
 
+class ServerResponseError(XMLError):
+    pass
+
+
 class InternalServerError(TableauError):
     def __init__(self, server_response, request_url: Optional[str] = None):
         self.code = server_response.status_code
@@ -51,6 +62,11 @@ class NotSignedInError(TableauError):
     pass
 
 
+class FailedSignInError(XMLError, NotSignedInError):
+    def __str__(self):
+        return f"{split_pascal_case(self.__class__.__name__)}: {super().__str__()}"
+
+
 class ItemTypeNotAllowed(TableauError):
     pass
 

diff --git a/test/assets/server_info_wrong_site.html b/test/assets/server_info_wrong_site.html
@@ -0,0 +1,56 @@
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+    <title>Example website</title>
+</head>
+
+<body>
+    <table>
+        <th>
+            <td>A</td>
+            <td>B</td>
+            <td>C</td>
+            <td>D</td>
+            <td>E</td>
+        </th>
+        <tr>
+            <td>1</td>
+            <td>2</td>
+            <td>3</td>
+            <td>4</td>
+            <td>5</td>
+        </tr>
+        <tr>
+            <td>2</td>
+            <td>3</td>
+            <td>4</td>
+            <td>5</td>
+            <td>6</td>
+        </tr>
+        <tr>
+            <td>3</td>
+            <td>4</td>
+            <td>5</td>
+            <td>6</td>
+            <td>7</td>
+        </tr>
+        <tr>
+            <td>4</td>
+            <td>5</td>
+            <td>6</td>
+            <td>7</td>
+            <td>8</td>
+        </tr>
+        <tr>
+            <td>5</td>
+            <td>6</td>
+            <td>7</td>
+            <td>8</td>
+            <td>9</td>
+        </tr>
+    </table>
+</body>
+
+</html>
diff --git a/test/test_auth.py b/test/test_auth.py
@@ -63,23 +63,23 @@ def test_sign_in_error(self):
         with requests_mock.mock() as m:
             m.post(self.baseurl + "/signin", text=response_xml, status_code=401)
             tableau_auth = TSC.TableauAuth("testuser", "wrongpassword")
-            self.assertRaises(TSC.NotSignedInError, self.server.auth.sign_in, tableau_auth)
+            self.assertRaises(TSC.FailedSignInError, self.server.auth.sign_in, tableau_auth)
 
     def test_sign_in_invalid_token(self):
         with open(SIGN_IN_ERROR_XML, "rb") as f:
             response_xml = f.read().decode("utf-8")
         with requests_mock.mock() as m:
             m.post(self.baseurl + "/signin", text=response_xml, status_code=401)
             tableau_auth = TSC.PersonalAccessTokenAuth(token_name="mytoken", personal_access_token="invalid")
-            self.assertRaises(TSC.NotSignedInError, self.server.auth.sign_in, tableau_auth)
+            self.assertRaises(TSC.FailedSignInError, self.server.auth.sign_in, tableau_auth)
 
     def test_sign_in_without_auth(self):
         with open(SIGN_IN_ERROR_XML, "rb") as f:
             response_xml = f.read().decode("utf-8")
         with requests_mock.mock() as m:
             m.post(self.baseurl + "/signin", text=response_xml, status_code=401)
             tableau_auth = TSC.TableauAuth("", "")
-            self.assertRaises(TSC.NotSignedInError, self.server.auth.sign_in, tableau_auth)
+            self.assertRaises(TSC.FailedSignInError, self.server.auth.sign_in, tableau_auth)
 
     def test_sign_out(self):
         with open(SIGN_IN_XML, "rb") as f:

diff --git a/test/test_server_info.py b/test/test_server_info.py
@@ -4,13 +4,15 @@
 import requests_mock
 
 import tableauserverclient as TSC
+from tableauserverclient.server.endpoint.exceptions import NonXMLResponseError
 
 TEST_ASSET_DIR = os.path.join(os.path.dirname(__file__), "assets")
 
 SERVER_INFO_GET_XML = os.path.join(TEST_ASSET_DIR, "server_info_get.xml")
 SERVER_INFO_25_XML = os.path.join(TEST_ASSET_DIR, "server_info_25.xml")
 SERVER_INFO_404 = os.path.join(TEST_ASSET_DIR, "server_info_404.xml")
 SERVER_INFO_AUTH_INFO_XML = os.path.join(TEST_ASSET_DIR, "server_info_auth_info.xml")
+SERVER_INFO_WRONG_SITE = os.path.join(TEST_ASSET_DIR, "server_info_wrong_site.html")
 
 
 class ServerInfoTests(unittest.TestCase):
@@ -63,3 +65,11 @@ def test_server_use_server_version_flag(self):
             m.get("http://test/api/2.4/serverInfo", text=si_response_xml)
             server = TSC.Server("http://test", use_server_version=True)
             self.assertEqual(server.version, "2.5")
+
+    def test_server_wrong_site(self):
+        with open(SERVER_INFO_WRONG_SITE, "rb") as f:
+            response = f.read().decode("utf-8")
+        with requests_mock.mock() as m:
+            m.get(self.server.server_info.baseurl, text=response, status_code=404)
+            with self.assertRaises(NonXMLResponseError):
+                self.server.server_info.get()