From 7913567fbae35092aa20fa287685fb52bd104368 Mon Sep 17 00:00:00 2001 From: Terence Honles Date: Thu, 1 Jul 2021 14:34:53 -0700 Subject: [PATCH 01/10] fix #373 breaking collecting users information on *not* starlette The change fixes the check to match the previous code: `if not StarletteRequest and hasattr(request, 'user'):` --- rollbar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 5101ea35..19d035f3 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -935,7 +935,7 @@ def _build_person_data(request): if StarletteRequest: from rollbar.contrib.starlette.requests import hasuser else: - hasuser = lambda request: False + hasuser = lambda request: True if hasuser(request) and hasattr(request, 'user'): user_prop = request.user From 1575465afb9f4c75e0fa9725e925341ce3c8e71b Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 19:47:31 +0200 Subject: [PATCH 02/10] Test person data building for Django --- rollbar/test/test_rollbar.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index 938c76d0..1d355d72 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -327,6 +327,33 @@ def test_fastapi_request_data_empty_values(self): self.assertEqual(data['user_ip'], scope['client'][0]) self.assertEqual(data['method'], scope['method']) + def test_django_build_person_data(self): + try: + import django + from django.conf import settings + except ImportError: + self.skipTest('Requires Django to be installed') + else: + settings.configure( + INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'] + ) + django.setup() + + from django.contrib.auth.models import User + from django.http.request import HttpRequest + + request = HttpRequest() + request.user = User() + request.user.id = 123 + request.user.username = 'admin' + request.user.email = 'admin@example.org' + + data = rollbar._build_person_data(request) + + self.assertDictEqual( + data, {'id': '123', 'username': 'admin', 'email': 'admin@example.org'} + ) + @unittest.skipUnless(sys.version_info >= (3, 6), 'Python3.6+ required') def test_get_request_starlette_middleware(self): try: From fd2888432e70f3216a96942f2a2a08068a7e263b Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 19:52:25 +0200 Subject: [PATCH 03/10] Test person data building for Starlette --- rollbar/test/test_rollbar.py | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index 1d355d72..e0882bc9 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -354,6 +354,48 @@ def test_django_build_person_data(self): data, {'id': '123', 'username': 'admin', 'email': 'admin@example.org'} ) + def test_starlette_build_person_data_if_user_authenticated(self): + try: + from starlette.authentication import SimpleUser + from starlette.requests import Request + except ImportError: + self.skipTest('Requires Starlette to be installed') + + # Implement interface with the id attribute + class User(SimpleUser): + counter = 0 + + def __init__(self, username, email): + super().__init__(username) + self.email = email + + User.counter += 1 + self.id = User.counter + + scope = {'type': 'http'} + request = Request(scope) + # Make the user authenticated + request.scope['user'] = User('admin', 'admin@example.org') + + data = rollbar._build_person_data(request) + + self.assertDictEqual( + data, {'id': '1', 'username': 'admin', 'email': 'admin@example.org'} + ) + + def test_starlette_failsafe_build_person_data_if_user_not_authenticated(self): + try: + from starlette.requests import Request + except ImportError: + self.skipTest('Requires Starlette to be installed') + + scope = {'type': 'http'} + request = Request(scope) + + data = rollbar._build_person_data(request) + + self.assertIsNone(data) + @unittest.skipUnless(sys.version_info >= (3, 6), 'Python3.6+ required') def test_get_request_starlette_middleware(self): try: From 119dc59892307c7b4e1dbe13b29693d95bf61a02 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 20:01:18 +0200 Subject: [PATCH 04/10] Enrich the traceback with the defined function --- rollbar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 19d035f3..215d0dbc 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -935,7 +935,7 @@ def _build_person_data(request): if StarletteRequest: from rollbar.contrib.starlette.requests import hasuser else: - hasuser = lambda request: True + def hasuser(request): return True if hasuser(request) and hasattr(request, 'user'): user_prop = request.user From 403c54effa8d1fc7792f4a63731643d155634493 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 23:52:32 +0200 Subject: [PATCH 05/10] Explicitly define the maximum version required The currently used version of `setuptools` has a bug, so the version requirements are not properly respected. In the current version, `requests>= 0.12.1` always installs the latest version of the package. --- setup.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 71d019c8..1fe70750 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,19 @@ "Topic :: System :: Monitoring", ], install_requires=[ - 'requests>=0.12.1', + # The currently used version of `setuptools` has a bug, + # so the version requirements are not properly respected. + # + # In the current version, `requests>= 0.12.1` + # always installs the latest version of the package. + 'requests>=0.12.1; python_version == "2.7"', + 'requests>=0.12.1; python_version >= "3.6"', + 'requests<2.26,>=0.12.1; python_version == "3.5"', + 'requests<2.22,>=0.12.1; python_version == "3.4"', + 'requests<2.19,>=0.12.1; python_version == "3.3"', + 'requests<1.2,>=0.12.1; python_version == "3.2"', + 'requests<1.2,>=0.12.1; python_version == "3.1"', + 'requests<1.2,>=0.12.1; python_version == "3.0"', 'six>=1.9.0' ], tests_require=tests_require, From ef5b42b7549dfb18ed801f8dec86bf691e7cf7d7 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 23:53:28 +0200 Subject: [PATCH 06/10] Remove redundant import and whitespaces --- setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 1fe70750..db85c93d 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,5 @@ import re import os.path -import sys from setuptools import setup, find_packages HERE = os.path.abspath(os.path.dirname(__file__)) @@ -94,4 +93,4 @@ 'six>=1.9.0' ], tests_require=tests_require, - ) +) From c4f7e3e47f87dc503b40bd493d39ed37138acb38 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sun, 19 Sep 2021 01:39:02 +0200 Subject: [PATCH 07/10] Call `django.setup()` for Django 1.7+ only --- rollbar/test/test_rollbar.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index e0882bc9..ed797883 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -337,7 +337,8 @@ def test_django_build_person_data(self): settings.configure( INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'] ) - django.setup() + if django.VERSION >= (1, 7): + django.setup() from django.contrib.auth.models import User from django.http.request import HttpRequest From ff30ac10319cc13a295d9e94a01da2b6be10f80d Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sun, 19 Sep 2021 17:22:12 +0200 Subject: [PATCH 08/10] Do not log error if request is missing in the context Return None object instead --- rollbar/contrib/starlette/requests.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/rollbar/contrib/starlette/requests.py b/rollbar/contrib/starlette/requests.py index c4fe4db9..e8cfb694 100644 --- a/rollbar/contrib/starlette/requests.py +++ b/rollbar/contrib/starlette/requests.py @@ -46,13 +46,7 @@ def get_current_request() -> Optional[Request]: ) return None - request = _current_request.get() - - if request is None: - log.error('Request is not available in the present context.') - return None - - return request + return _current_request.get() def store_current_request( From 427cab6c326aac9552c938641b18011b882b1045 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sun, 19 Sep 2021 17:22:54 +0200 Subject: [PATCH 09/10] Limit the creation of the request object to an HTTP scope only --- rollbar/contrib/starlette/requests.py | 4 +++- rollbar/test/starlette_tests/test_requests.py | 12 +++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/rollbar/contrib/starlette/requests.py b/rollbar/contrib/starlette/requests.py index e8cfb694..cd33981b 100644 --- a/rollbar/contrib/starlette/requests.py +++ b/rollbar/contrib/starlette/requests.py @@ -57,8 +57,10 @@ def store_current_request( if receive is None: request = request_or_scope - else: + elif request_or_scope['type'] == 'http': request = Request(request_or_scope, receive) + else: + request = None _current_request.set(request) return request diff --git a/rollbar/test/starlette_tests/test_requests.py b/rollbar/test/starlette_tests/test_requests.py index a88b2648..34aabebe 100644 --- a/rollbar/test/starlette_tests/test_requests.py +++ b/rollbar/test/starlette_tests/test_requests.py @@ -51,7 +51,7 @@ def test_should_accept_request_param(self): self.assertEqual(request, stored_request) - def test_should_accept_scope_and_receive_params(self): + def test_should_accept_scope_param_if_http_type(self): from starlette.requests import Request from rollbar.contrib.starlette.requests import store_current_request from rollbar.lib._async import async_receive @@ -83,6 +83,16 @@ def test_should_accept_scope_and_receive_params(self): self.assertEqual(request, expected_request) + def test_should_not_accept_scope_param_if_not_http_type(self): + from rollbar.contrib.starlette.requests import store_current_request + + scope = {'asgi': {'spec_version': '2.0', 'version': '3.0'}, 'type': 'lifespan'} + receive = {} + + request = store_current_request(scope, receive) + + self.assertIsNone(request) + def test_hasuser(self): from starlette.requests import Request from rollbar.contrib.starlette.requests import hasuser From ff8b2c4959558958b17e12844804c957f03b810c Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sun, 19 Sep 2021 19:02:04 +0200 Subject: [PATCH 10/10] Release v0.16.2 --- CHANGELOG.md | 6 ++++++ rollbar/__init__.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db85195e..5f61e720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ The change log is also available on the [GitHub Releases Page](https://github.com/rollbar/pyrollbar/releases). +**0.16.2** + +- Fix building person data in Django. See [#385](https://github.com/rollbar/pyrollbar/pull/385) +- Fix circular error logging for non-HTTP events in Starlette. See [#390](https://github.com/rollbar/pyrollbar/pull/390) +- Fix Python 3.4 builds. See [#389](https://github.com/rollbar/pyrollbar/pull/389) + **0.16.1** - Fix PyPI artifacts diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 215d0dbc..27a703d0 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -23,7 +23,7 @@ from rollbar.lib import events, filters, dict_merge, parse_qs, text, transport, urljoin, iteritems, defaultJSONEncode -__version__ = '0.16.1' +__version__ = '0.16.2' __log_name__ = 'rollbar' log = logging.getLogger(__log_name__)