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 5101ea35..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__) @@ -935,7 +935,7 @@ def _build_person_data(request): if StarletteRequest: from rollbar.contrib.starlette.requests import hasuser else: - hasuser = lambda request: False + def hasuser(request): return True if hasuser(request) and hasattr(request, 'user'): user_prop = request.user diff --git a/rollbar/contrib/starlette/requests.py b/rollbar/contrib/starlette/requests.py index c4fe4db9..cd33981b 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( @@ -63,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 diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index 938c76d0..ed797883 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -327,6 +327,76 @@ 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'] + ) + if django.VERSION >= (1, 7): + 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'} + ) + + 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: diff --git a/setup.py b/setup.py index 71d019c8..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__)) @@ -78,8 +77,20 @@ "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, - ) +)