From d43c2e3188e6d9379435e536e672b204d1ef1f66 Mon Sep 17 00:00:00 2001 From: Mukund Ananthu Date: Tue, 5 Mar 2024 15:32:22 +0000 Subject: [PATCH 1/2] fix: Catch and surface BaseException() Currently, the behavior of the library is to catch Exception() when encountered in the user provided callback, surface it to the calling code and shut down the client. However, for BaseException() encountered in the user provided callback, the BaseException() is not surfaced to the calling code and the client is not shut down. Make the behavior of the client when BaseException() is encountered consistent with the behavior of the client when Exception() is encountered. --- .../subscriber/_protocol/streaming_pull_manager.py | 4 ++-- .../subscriber/test_streaming_pull_manager.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py b/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py index 2f5a31e49..f07db8546 100644 --- a/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py +++ b/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py @@ -112,7 +112,7 @@ def _wrap_as_exception(maybe_exception: Any) -> BaseException: def _wrap_callback_errors( callback: Callable[["google.cloud.pubsub_v1.subscriber.message.Message"], Any], - on_callback_error: Callable[[Exception], Any], + on_callback_error: Callable[[BaseException], Any], message: "google.cloud.pubsub_v1.subscriber.message.Message", ): """Wraps a user callback so that if an exception occurs the message is @@ -124,7 +124,7 @@ def _wrap_callback_errors( """ try: callback(message) - except Exception as exc: + except BaseException as exc: # Note: the likelihood of this failing is extremely low. This just adds # a message to a queue, so if this doesn't work the world is in an # unrecoverable state and this thread should just bail. diff --git a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py index 1f781b722..9e46f3d79 100644 --- a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py +++ b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py @@ -77,8 +77,14 @@ def test__wrap_callback_errors_no_error(): on_callback_error.assert_not_called() -def test__wrap_callback_errors_error(): - callback_error = ValueError("meep") +@pytest.mark.parametrize( + "callback_error", + [ + (ValueError("ValueError")), + (BaseException("BaseException")), + ], +) +def test__wrap_callback_errors_error(callback_error): msg = mock.create_autospec(message.Message, instance=True) callback = mock.Mock(side_effect=callback_error) From d9429e7b5ebb597fe5a136fde58c4710ab065531 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 5 Mar 2024 15:57:21 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py index 9e46f3d79..278f3e88e 100644 --- a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py +++ b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py @@ -85,7 +85,6 @@ def test__wrap_callback_errors_no_error(): ], ) def test__wrap_callback_errors_error(callback_error): - msg = mock.create_autospec(message.Message, instance=True) callback = mock.Mock(side_effect=callback_error) on_callback_error = mock.Mock()