From 1ec5c3a666a98c9d4abdaf68e3f3cf37efb16e31 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 14 Sep 2026 13:46:49 -0500 Subject: [PATCH 1/2] Update to pass under the latest flake8-bugbear Primarily, flake8-bugbear flags several of our exceptions as being unsafe to stringify, copy, or pickle because they don't pass args to `super()` in the mandated way. (This is the new B042 rule.) In order to pass, most of these cases are simple to update to define and test explicit `str()` and `copy.copy()` calls. Copy is a simpler operation than pickling and exercises the same reducer. For the target error types, defining custom `__reduce__` methods is sufficient. For `GlobusAPIError`, because it's so key to SDK interfaces, no action is taken for now, other than the addition of a TODO comment. New unit tests exercise str and copy.copy on all of the updated errors. --- .pre-commit-config.yaml | 2 +- src/globus_sdk/exc/api.py | 8 +- src/globus_sdk/exc/convert.py | 11 +- src/globus_sdk/gare/_variants.py | 8 +- src/globus_sdk/scopes/consents/_errors.py | 14 +++ .../validating_token_storage/errors.py | 38 ++++++- tests/unit/errors/test_error_copy.py | 104 ++++++++++++++++++ tests/unit/errors/test_error_stringify.py | 48 ++++++++ tests/unit/responses/test_response.py | 4 +- 9 files changed, 223 insertions(+), 14 deletions(-) create mode 100644 tests/unit/errors/test_error_copy.py create mode 100644 tests/unit/errors/test_error_stringify.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 01312333c..41a5f1f90 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,7 @@ repos: - id: flake8 name: "Lint python files" additional_dependencies: - - 'flake8-bugbear==24.12.12' + - 'flake8-bugbear==26.9.9' - 'flake8-comprehensions==3.16.0' - 'flake8-typing-as-t==1.0.0' - repo: https://github.com/PyCQA/isort diff --git a/src/globus_sdk/exc/api.py b/src/globus_sdk/exc/api.py index 15932fa35..38529f0d9 100644 --- a/src/globus_sdk/exc/api.py +++ b/src/globus_sdk/exc/api.py @@ -42,7 +42,13 @@ class GlobusAPIError(GlobusError): MESSAGE_FIELDS = ["message", "detail", "title"] RECOGNIZED_AUTHZ_SCHEMES = ["bearer", "basic", "globus-goauthtoken"] - def __init__(self, r: requests.Response, *args: t.Any, **kwargs: t.Any) -> None: + # TODO: re-evaluate how exception args are handled by this class. + # For now, ignore flake8-bugbear's B042 rule about exception inheritance. + # Fixing it would likely require somehow adjusting the interface for a + # GlobusAPIError, possibly in a breaking way. + def __init__( # noqa: B042 + self, r: requests.Response, *args: t.Any, **kwargs: t.Any + ) -> None: # defer this import to avoid circularity between 'exc' and 'transport' from globus_sdk.transport import RequestsTransport diff --git a/src/globus_sdk/exc/convert.py b/src/globus_sdk/exc/convert.py index dc5ee8ca1..1c560bb4b 100644 --- a/src/globus_sdk/exc/convert.py +++ b/src/globus_sdk/exc/convert.py @@ -1,7 +1,5 @@ from __future__ import annotations -import typing as t - import requests from .base import GlobusError @@ -18,10 +16,17 @@ class NetworkError(GlobusError): to explain potentially confusing or inconsistent exceptions passed to us """ - def __init__(self, msg: str, exc: Exception, *args: t.Any, **kwargs: t.Any) -> None: + def __init__(self, msg: str, exc: Exception) -> None: super().__init__(msg) + self.message = msg self.underlying_exception = exc + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, Exception]]: + return (NetworkError, (self.message, self.underlying_exception)) + class GlobusTimeoutError(NetworkError): """The REST request timed out.""" diff --git a/src/globus_sdk/gare/_variants.py b/src/globus_sdk/gare/_variants.py index 3d4fc9d92..e57399a4f 100644 --- a/src/globus_sdk/gare/_variants.py +++ b/src/globus_sdk/gare/_variants.py @@ -28,7 +28,7 @@ class LegacyDependentConsentRequiredAuthError(Serializable): The dependent_consent_required error format emitted by the Globus Auth service. """ - def __init__( + def __init__( # noqa: B042 self, *, error: t.Literal["dependent_consent_required"], @@ -68,7 +68,7 @@ class LegacyConsentRequiredTransferError(Serializable): The ConsentRequired error format emitted by the Globus Transfer service. """ - def __init__( + def __init__( # noqa: B042 self, *, code: t.Literal["ConsentRequired"], @@ -99,7 +99,7 @@ class LegacyConsentRequiredAPError(Serializable): Action Providers. """ - def __init__( + def __init__( # noqa: B042 self, *, code: t.Literal["ConsentRequired"], @@ -195,7 +195,7 @@ class LegacyAuthorizationParametersError(Serializable): DEFAULT_CODE = "AuthorizationRequired" - def __init__( + def __init__( # noqa: B042 self, *, authorization_parameters: dict[str, t.Any] | LegacyAuthorizationParameters, diff --git a/src/globus_sdk/scopes/consents/_errors.py b/src/globus_sdk/scopes/consents/_errors.py index 6e3f246d5..dc29fc826 100644 --- a/src/globus_sdk/scopes/consents/_errors.py +++ b/src/globus_sdk/scopes/consents/_errors.py @@ -11,12 +11,26 @@ class ConsentParseError(Exception): def __init__(self, message: str, raw_consent: dict[str, t.Any]) -> None: super().__init__(message) + self.message = message self.raw_consent = raw_consent + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, dict[str, t.Any]]]: + return (ConsentParseError, (self.message, self.raw_consent)) + class ConsentTreeConstructionError(Exception): """An error raised if consent tree construction fails.""" def __init__(self, message: str, consents: list[Consent]) -> None: super().__init__(message) + self.message = message self.consents = consents + + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, list[Consent]]]: + return (ConsentTreeConstructionError, (self.message, self.consents)) diff --git a/src/globus_sdk/token_storage/validating_token_storage/errors.py b/src/globus_sdk/token_storage/validating_token_storage/errors.py index 27e559da3..b2153b6cb 100644 --- a/src/globus_sdk/token_storage/validating_token_storage/errors.py +++ b/src/globus_sdk/token_storage/validating_token_storage/errors.py @@ -1,7 +1,7 @@ from __future__ import annotations +import datetime import uuid -from datetime import datetime from globus_sdk import GlobusError, Scope @@ -28,25 +28,50 @@ def __init__( self, message: str, stored_id: uuid.UUID | str, new_id: uuid.UUID | str ) -> None: super().__init__(message) + self.message = message self.stored_id = stored_id self.new_id = new_id + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, uuid.UUID | str, uuid.UUID | str]]: + return (IdentityMismatchError, (self.message, self.stored_id, self.new_id)) + class MissingTokenError(TokenValidationError, LookupError): """No token stored for a given resource server.""" def __init__(self, message: str, resource_server: str) -> None: super().__init__(message) + self.message = message self.resource_server = resource_server + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, str]]: + return (MissingTokenError, (self.message, self.resource_server)) + class ExpiredTokenError(TokenValidationError, ValueError): """The token stored for a given resource server has expired.""" def __init__(self, expires_at_seconds: int) -> None: - expiration = datetime.fromtimestamp(expires_at_seconds) - super().__init__(f"Token expired at {expiration.isoformat()}") + expiration = datetime.datetime.fromtimestamp(expires_at_seconds) + message = f"Token expired at {expiration.isoformat()}" + + super().__init__(message) + + self.message = message self.expiration = expiration + self._expires_at_seconds = expires_at_seconds + + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[int]]: + return (ExpiredTokenError, (self._expires_at_seconds,)) class UnmetScopeRequirementsError(TokenValidationError, ValueError): @@ -56,6 +81,13 @@ def __init__( self, message: str, scope_requirements: dict[str, list[Scope]] ) -> None: super().__init__(message) + self.message = message # The full set of scope requirements which were evaluated. # Notably this is not exclusively the unmet scope requirements. self.scope_requirements = scope_requirements + + def __str__(self) -> str: + return self.message + + def __reduce__(self) -> tuple[type, tuple[str, dict[str, list[Scope]]]]: + return (UnmetScopeRequirementsError, (self.message, self.scope_requirements)) diff --git a/tests/unit/errors/test_error_copy.py b/tests/unit/errors/test_error_copy.py new file mode 100644 index 000000000..8bb14fc5c --- /dev/null +++ b/tests/unit/errors/test_error_copy.py @@ -0,0 +1,104 @@ +""" +Several errors define `__reduce__` to ensure that they are pickleable. + +These tests ensure that we can copy without errors and that the results compare equal +under some definition of "equal" (which may be specific top the error type). +""" + +import copy + +from globus_sdk.exc import NetworkError +from globus_sdk.scopes.consents import ConsentParseError, ConsentTreeConstructionError +from globus_sdk.token_storage.validating_token_storage import ( + ExpiredTokenError, + IdentityMismatchError, + MissingTokenError, + UnmetScopeRequirementsError, +) + + +def test_copy_of_network_error(): + err = NetworkError("bad cxn", Exception("kaboom")) + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert str(err_copy.underlying_exception) == "kaboom" + + +def test_copy_of_identity_mismatch_error(): + err = IdentityMismatchError("they didn't match", "a", "b") + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.stored_id, + err.new_id, + ) == ( + err_copy.message, + err_copy.stored_id, + err_copy.new_id, + ) + + +def test_copy_of_missing_token_error(): + err = MissingTokenError("it gone", "my_rs") + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.resource_server, + ) == ( + err_copy.message, + err_copy.resource_server, + ) + + +def test_copy_of_expired_token_error(): + err = ExpiredTokenError(101) + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.expiration, + ) == ( + err_copy.message, + err_copy.expiration, + ) + + +def test_copy_of_scope_requirements_error(): + err = UnmetScopeRequirementsError("needed a token for scope", {"my_rs": []}) + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.scope_requirements, + ) == ( + err_copy.message, + err_copy.scope_requirements, + ) + + +def test_copy_of_consent_parse_error(): + err = ConsentParseError("it didn't parse", {}) + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.raw_consent, + ) == ( + err_copy.message, + err_copy.raw_consent, + ) + + +def test_copy_of_consent_tree_construction_error(): + err = ConsentTreeConstructionError("empty", []) + err_copy = copy.copy(err) + assert id(err) != id(err_copy) + assert ( + err.message, + err.consents, + ) == ( + err_copy.message, + err_copy.consents, + ) diff --git a/tests/unit/errors/test_error_stringify.py b/tests/unit/errors/test_error_stringify.py new file mode 100644 index 000000000..c2b33df75 --- /dev/null +++ b/tests/unit/errors/test_error_stringify.py @@ -0,0 +1,48 @@ +""" +Several errors define `__str__` to ensure that args are formatted +(or dropped) as desired. These tests ensure that we get the right strings back. +""" + +from globus_sdk.exc import NetworkError +from globus_sdk.scopes.consents import ConsentParseError, ConsentTreeConstructionError +from globus_sdk.token_storage.validating_token_storage import ( + ExpiredTokenError, + IdentityMismatchError, + MissingTokenError, + UnmetScopeRequirementsError, +) + + +def test_str_of_network_error(): + err = NetworkError("bad cxn", Exception("kaboom")) + assert str(err) == "bad cxn" + + +def test_str_of_identity_mismatch_error(): + err = IdentityMismatchError("they didn't match", "a", "b") + assert str(err) == "they didn't match" + + +def test_str_of_missing_token_error(): + err = MissingTokenError("it gone", "my_rs") + assert str(err) == "it gone" + + +def test_str_of_expired_token_error(): + err = ExpiredTokenError(101) + assert str(err).startswith("Token expired at ") + + +def test_str_of_scope_requirements_error(): + err = UnmetScopeRequirementsError("needed a token for scope", {"my_rs": []}) + assert str(err) == "needed a token for scope" + + +def test_str_of_consent_parse_error(): + err = ConsentParseError("it didn't parse", {}) + assert str(err) == "it didn't parse" + + +def test_str_of_consent_tree_construction_error(): + err = ConsentTreeConstructionError("empty", []) + assert str(err) == "empty" diff --git a/tests/unit/responses/test_response.py b/tests/unit/responses/test_response.py index 41de7e6ee..1586d1505 100644 --- a/tests/unit/responses/test_response.py +++ b/tests/unit/responses/test_response.py @@ -196,14 +196,14 @@ def test_len_array_bad_data(dict_response, json_response_factory): "Cannot take len() on ArrayResponse data when type is 'NoneType'" ), ): - len(null_array) + len(null_array) # noqa: B018 dict_array = ArrayResponse(dict_response.r) with pytest.raises( TypeError, match=re.escape("Cannot take len() on ArrayResponse data when type is 'dict'"), ): - len(dict_array) + len(dict_array) # noqa: B018 def test_iter_array_bad_data(dict_response, json_response_factory): From b2d36f7c4e7c4263fca55b128c171822d62d0a04 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 15 Sep 2026 10:56:04 -0500 Subject: [PATCH 2/2] Update tests/unit/errors/test_error_copy.py Co-authored-by: Kurt McKee --- tests/unit/errors/test_error_copy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/errors/test_error_copy.py b/tests/unit/errors/test_error_copy.py index 8bb14fc5c..0c013e831 100644 --- a/tests/unit/errors/test_error_copy.py +++ b/tests/unit/errors/test_error_copy.py @@ -2,7 +2,7 @@ Several errors define `__reduce__` to ensure that they are pickleable. These tests ensure that we can copy without errors and that the results compare equal -under some definition of "equal" (which may be specific top the error type). +under some definition of "equal" (which may be specific to the error type). """ import copy