Working on feedback - StatbankAuthError now takes a param into init, less aggressive about emptying the password from netrc

Author: aecorn

src/statbank/api_exceptions.py

@@ -4,23 +4,32 @@
 
 
 class StatbankAuthError(requests.HTTPError):
-    """We pass up the error-response with this error up through try-excepts."""
+    """Raised when authentication with Statbank fails.
 
-    def __init__(self, other_error: str | Exception) -> None:
-        """Initializing the error with a possibility of wrapping around an existing error.
+    Extends ``requests.HTTPError`` with an optional ``response_content`` attribute
+    that can hold parsed JSON from the response.
+
+    Args:
+        *args: Positional arguments forwarded to ``requests.HTTPError``.
+        response_content: Optional JSON-decoded payload to attach.
+        **kwargs: Keyword arguments forwarded to ``requests.HTTPError``.
+    """
+
+    def __init__(
+        self,
+        *args: Any,
+        response_content: dict[str, Any] | None = None,
+        **kwargs: Any,
+    ) -> None:
+        """Initialize the Statbank authentication error.
 
         Args:
-            other_error: The error-string, or a different exception that we want to re-wrap in this exception.
+            *args: Positional arguments forwarded to ``requests.HTTPError``.
+            response_content: Optional JSON-decoded payload to attach.
+            **kwargs: Keyword arguments forwarded to ``requests.HTTPError``.
         """
-        error_text: str = str(other_error)
-        super().__init__(error_text)
-
-        self.response_content: dict[str, Any] | None = None
-        if hasattr(
-            other_error,
-            "response_content",
-        ):  # If we double-wrap the error, lets try to save this attribute
-            self.response_content = other_error.response_content
+        super().__init__(*args, **kwargs)
+        self.response_content: dict[str, Any] | None = response_content
 
 
 class StatbankApiError(Exception):

src/statbank/auth.py

@@ -212,28 +212,32 @@ def _react_to_httperror_should_retry(
         self,
         e: requests.HTTPError | StatbankAuthError,
     ) -> bool:
-        logger.error("We got an http-error, proceding to emptying the auth.")
-        self._cleanup_netrc()
-
-        default_err_msg = "Got an http-error, but it does not look like an account-lock or invalid password/username, is this something we should program for? - "
+        default_err_msg = """Got an http-error, but it does not look like an account-lock or invalid password/username,
+        is this something we should program for?
+        If you want to reset the auth manually call StatbankClient().reset_auth() or
+        StatbankAuth().reset_auth(). Alternatively edit the .netrc file directly.""".replace(
+            "\n",
+            "",
+        )
 
         if hasattr(e, "response_content") and e.response_content:
             if "ORA-28000" in e.response_content.get("ExceptionMessage", ""):
-                err_msg = f'Your account has been locked. Contact kundeservice@ssb.no to unlock. Errortext: {e.response_content.get("ExceptionMessage", "")}'
+                err_msg = f'Your account has been locked. Contact kundeservice@ssb.no to unlock. Resetting auth. Errortext: {e.response_content.get("ExceptionMessage", "")}'
                 logger.error(err_msg)
                 new_err = StatbankAuthError(err_msg)
+                self._cleanup_netrc()
                 raise new_err
             if "ORA-01017" in e.response_content.get("ExceptionMessage", ""):
                 logger.error(
-                    f"TYPE CAREFULLY - The username and password you used may have been wrong. Errortext: {e.response_content.get('ExceptionMessage', '')}",
+                    f"TYPE CAREFULLY - The username and password you used may have been wrong. Resetting Auth. Errortext: {e.response_content.get('ExceptionMessage', '')}",
                 )
-                self._auth = self._get_auth()
+                self.reset_auth()
                 return True
             logger.error(
                 f"{default_err_msg}{e.response_content.get('Exception_message', '')} - {e} ",
             )
         else:
-            logger.error(f"{default_err_msg}{e}")
+            logger.error(f"{default_err_msg}{e}.")
 
         raise e
 
@@ -257,7 +261,7 @@ def _encrypt_password(self: Self, password: str) -> str:
 
         response.raise_for_status()
 
-        data = cast(dict[Literal["message"], str], response.json())
+        data = cast('dict[Literal["message"], str]', response.json())
 
         return data["message"]
 

src/statbank/transfer.py

@@ -372,9 +372,7 @@ def _make_transfer_request(
         try:
             result.raise_for_status()
         except r.HTTPError as e:
-            e = StatbankAuthError(e)  # Wrap so attribute is available
-            e.response_content = result.json()
-            raise
+            raise StatbankAuthError(response_content=result.json()) from e
         return result
 
     def _cleanup_response(self) -> None:

src/statbank/uttrekk.py

@@ -602,9 +602,7 @@ def _make_request(self, url: furl, params: dict[str, str | int]) -> r.Response:
         try:
             response.raise_for_status()
         except r.HTTPError as e:
-            e = StatbankAuthError(e)  # Wrap so attribute is available to assign to
-            e.response_content = response.json()
-            raise
+            raise StatbankAuthError(response_content=response.json()) from e
         return response
 
     @classmethod

tests/test_api_exceptions.py

@@ -1,9 +1,11 @@
 from statbank.api_exceptions import StatbankAuthError
 
 
-def test_double_wrap_is_fine():
-    err1 = StatbankAuthError("Try adding the argument here")
-    err1.response_content = {"ExceptionMessage": "You are using testcode bro"}
-    err2 = StatbankAuthError(err1)
+def test_statbankautherror_param():
+    err1 = StatbankAuthError(
+        "Try adding the argument here",
+        response_content={"ExceptionMessage": "You are using testcode bro"},
+    )
 
-    assert "ExceptionMessage" in err2.response_content
+    assert "ExceptionMessage" in err1.response_content
+    assert len(err1.response_content.get("ExceptionMessage", ""))

tests/test_auth.py

@@ -198,9 +198,10 @@ def test_react_to_httperror_oracle_28000_raises(
     cleanup_mock = mock.Mock()
     monkeypatch.setattr(sa, "_cleanup_netrc", cleanup_mock)
 
-    err = StatbankAuthError("wrapped")
-    err.response_content = {"ExceptionMessage": "ORA-28000: account locked"}
-
+    err = StatbankAuthError(
+        "wrapped",
+        response_content={"ExceptionMessage": "ORA-28000: account locked"},
+    )
     with pytest.raises(StatbankAuthError) as exc:
         sa._react_to_httperror_should_retry(err)  # noqa: SLF001
 
@@ -224,8 +225,10 @@ def test_react_to_httperror_oracle_01017_returns_true_and_refreshes(
     get_auth_mock = mock.Mock(return_value=new_auth)
     monkeypatch.setattr(sa, "_get_auth", get_auth_mock)
 
-    err = StatbankAuthError("wrapped")
-    err.response_content = {"ExceptionMessage": "ORA-01017: invalid username/password"}
+    err = StatbankAuthError(
+        "wrapped",
+        response_content={"ExceptionMessage": "ORA-01017: invalid username/password"},
+    )
 
     should_retry = sa._react_to_httperror_should_retry(err)  # noqa: SLF001
 
@@ -250,7 +253,8 @@ def test_react_to_httperror_other_raises_original(
     with pytest.raises(StatbankAuthError) as exc:
         sa._react_to_httperror_should_retry(err)  # noqa: SLF001
 
-    cleanup_mock.assert_called_once()
+    # On a general error we dont want the cleanup to have been called, because we do not recognize the error
+    assert not cleanup_mock.called
     # The method re-raises the same exception object when unhandled
     assert exc.value is err