From c6d74e77411034312a2b92d626ff8434f1e3f5d9 Mon Sep 17 00:00:00 2001 From: Jordan Woods <13803242+jorwoods@users.noreply.github.com> Date: Sat, 28 Sep 2024 11:36:35 -0500 Subject: [PATCH 1/3] 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. --- tableauserverclient/__init__.py | 2 ++ tableauserverclient/server/__init__.py | 3 ++- .../server/endpoint/endpoint.py | 3 ++- .../server/endpoint/exceptions.py | 24 +++++++++++++++---- test/test_auth.py | 6 ++--- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/tableauserverclient/__init__.py b/tableauserverclient/__init__.py index bab2cf05f..1299c33bc 100644 --- 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/server/__init__.py b/tableauserverclient/server/__init__.py index f5cd1d236..87cc9460b 100644 --- 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 index bef96fdee..7ff71baaa 100644 --- 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 index 17d789d01..77332da3e 100644 --- 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/test_auth.py b/test/test_auth.py index eaf13481e..48100ad88 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -63,7 +63,7 @@ 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: @@ -71,7 +71,7 @@ def test_sign_in_invalid_token(self): 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: @@ -79,7 +79,7 @@ def test_sign_in_without_auth(self): 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: From 1ec1a5f6210b50b49e8c69de4c691b70e49d3147 Mon Sep 17 00:00:00 2001 From: Jordan Woods <13803242+jorwoods@users.noreply.github.com> Date: Sat, 28 Sep 2024 13:51:35 -0500 Subject: [PATCH 2/3] 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 --- tableauserverclient/models/server_info_item.py | 8 +++----- test/test_server_info.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tableauserverclient/models/server_info_item.py b/tableauserverclient/models/server_info_item.py index 5c3f6acc7..4b299b29d 100644 --- 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/test/test_server_info.py b/test/test_server_info.py index 1cf190ecd..fa1472c9a 100644 --- a/test/test_server_info.py +++ b/test/test_server_info.py @@ -4,6 +4,7 @@ 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") @@ -11,6 +12,7 @@ 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() From 6eab3c9dd6c16f1d1cf0c55c33b92eea15ebda19 Mon Sep 17 00:00:00 2001 From: Jordan Woods <13803242+jorwoods@users.noreply.github.com> Date: Sat, 28 Sep 2024 13:55:23 -0500 Subject: [PATCH 3/3] fix(test): add missing test asset --- test/assets/server_info_wrong_site.html | 56 +++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 test/assets/server_info_wrong_site.html diff --git a/test/assets/server_info_wrong_site.html b/test/assets/server_info_wrong_site.html new file mode 100644 index 000000000..e92daeb2d --- /dev/null +++ b/test/assets/server_info_wrong_site.html @@ -0,0 +1,56 @@ + + + + + + Example website + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ ABCDE
12345
23456
34567
45678
56789
+ + + \ No newline at end of file