Repository: scoutapp/scout_apm_python
Pull request #706 (2021-11-17 23:31:35)

Use urllib3 to post error messages to Scout Error API. 
Removes requests from packages dependencies


Requires integration testing to confirm error logging still works properly.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4777315..ac6097c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,9 @@
 - When Celery `task_failure` signal has a string type for the `traceback`
   parameter, use the `einfo.tb` to fetch the traceback.
   ([Issue 708](https://github.com/scoutapp/scout_apm_python/issues/708))
+- Use urllib3 to post errors to Scout Errors API. Removes dependency on
+  requests library.
+- Switch to a dict for the `request_params` away from a list of tuples.
 
 ## [2.23.4] 2021-11-12
 
diff --git a/setup.py b/setup.py
index 3207e1b..8fd1df3 100644
--- a/setup.py
+++ b/setup.py
@@ -68,7 +68,6 @@ setup(
         'urllib3[secure] < 1.25 ; python_version < "3.5"',
         'urllib3[secure] < 2 ; python_version >= "3.5"',
         "wrapt>=1.10,<2.0",
-        "requests",
     ],
     keywords=["apm", "performance monitoring", "development"],
     classifiers=[
diff --git a/src/scout_apm/compat.py b/src/scout_apm/compat.py
index 4acc0d6..319aff7 100644
--- a/src/scout_apm/compat.py
+++ b/src/scout_apm/compat.py
@@ -79,11 +79,13 @@ def text(value, encoding="utf-8", errors="strict"):
 
 
 if sys.version_info >= (3, 0):
-    from urllib.parse import parse_qsl, urlencode
+    from html import escape
+    from urllib.parse import parse_qsl, urlencode, urljoin
 else:
+    from cgi import escape
     from urllib import urlencode
 
-    from urlparse import parse_qsl
+    from urlparse import parse_qsl, urljoin
 
 
 if sys.version_info >= (3, 0):
@@ -160,7 +162,7 @@ else:
     def gzip_compress(data):
         """Reimplementation gzip.compress for python 2.7"""
         buf = io.BytesIO()
-        with gzip.GzipFile(fileobj=buf, mode="wb") as f:
+        with gzip.GzipFile(fileobj=buf, mode="w") as f:
             f.write(data)
         return buf.getvalue()
 
@@ -168,6 +170,7 @@ else:
 __all__ = [
     "ContextDecorator",
     "datetime_to_timestamp",
+    "escape",
     "gzip_compress",
     "kwargs_only",
     "parse_qsl",
@@ -176,4 +179,5 @@ __all__ = [
     "text",
     "text_type",
     "urlencode",
+    "urljoin",
 ]
diff --git a/src/scout_apm/core/error_service.py b/src/scout_apm/core/error_service.py
index a469ea1..9ac210f 100644
--- a/src/scout_apm/core/error_service.py
+++ b/src/scout_apm/core/error_service.py
@@ -7,19 +7,14 @@ import os
 import threading
 import time
 
-try:
-    from html import escape
-except ImportError:
-    from cgi import escape
-
-try:
-    from urllib.parse import urljoin
-except ImportError:
-    from urlparse import urljoin
-
-import requests
-
-from scout_apm.compat import gzip_compress, queue
+from scout_apm.compat import (
+    escape,
+    gzip_compress,
+    queue,
+    urlencode,
+    urljoin,
+    urllib3_cert_pool_manager,
+)
 from scout_apm.core.config import scout_config
 from scout_apm.core.threading import SingletonThread
 
@@ -75,6 +70,7 @@ class ErrorServiceThread(SingletonThread):
 
     def run(self):
         batch_size = scout_config.value("errors_batch_size") or 1
+        http = urllib3_cert_pool_manager()
         try:
             while True:
                 errors = []
@@ -87,7 +83,7 @@ class ErrorServiceThread(SingletonThread):
                 except queue.Empty:
                     pass
 
-                if errors and self._send(errors):
+                if errors and self._send(http, errors):
                     for _ in range(len(errors)):
                         self._queue.task_done()
 
@@ -99,9 +95,10 @@ class ErrorServiceThread(SingletonThread):
         except Exception as exc:
             logger.debug("ErrorServiceThread exception: %r", exc, exc_info=exc)
         finally:
+            http.clear()
             logger.debug("ErrorServiceThread stopped.")
 
-    def _send(self, errors):
+    def _send(self, http, errors):
         try:
             data = json.dumps(
                 {
@@ -110,7 +107,7 @@ class ErrorServiceThread(SingletonThread):
                     "root": scout_config.value("application_root"),
                     "problems": errors,
                 }
-            )
+            ).encode("utf-8")
         except (ValueError, TypeError) as exc:
             logger.debug(
                 "Exception when serializing error message: %r", exc, exc_info=exc
@@ -122,21 +119,41 @@ class ErrorServiceThread(SingletonThread):
             "name": escape(scout_config.value("name"), quote=False),
         }
         headers = {
-            "Agent-Hostname": scout_config.value("hostname"),
+            "Agent-Hostname": scout_config.value("hostname") or "",
             "Content-Encoding": "gzip",
             "Content-Type": "application/json",
             "X-Error-Count": "{}".format(len(errors)),
         }
 
+        encoded_args = urlencode(params)
+        full_url = urljoin(
+            scout_config.value("errors_host"), "apps/error.scout"
+        ) + "?{}".format(encoded_args)
+
         try:
-            response = requests.post(
-                urljoin(scout_config.value("errors_host"), "apps/error.scout"),
-                params=params,
-                data=gzip_compress(data.encode("utf-8")),
-                headers=headers,
+            # urllib3 requires all parameters to be the same type for
+            # python 2.7.
+            # Since gzip can only return a str, convert all unicode instances
+            # to str.
+            response = http.request(
+                str("POST"),
+                str(full_url),
+                body=gzip_compress(data),
+                headers={str(key): str(value) for key, value in headers.items()},
             )
-            response.raise_for_status()
-        except requests.HTTPError as exc:
+            if response.status >= 400:
+                logger.debug(
+                    (
+                        "ErrorServiceThread %r response error on _send:"
+                        + " %r on PID: %s on thread: %s"
+                    ),
+                    response.status,
+                    response.data,
+                    os.getpid(),
+                    threading.current_thread(),
+                )
+                return False
+        except Exception as exc:
             logger.debug(
                 (
                     "ErrorServiceThread exception on _send:"
@@ -148,5 +165,4 @@ class ErrorServiceThread(SingletonThread):
                 exc_info=exc,
             )
             return False
-
         return True
diff --git a/src/scout_apm/django/apps.py b/src/scout_apm/django/apps.py
index 9525a1b..2036da2 100644
--- a/src/scout_apm/django/apps.py
+++ b/src/scout_apm/django/apps.py
@@ -63,7 +63,7 @@ class ScoutApmDjangoConfig(AppConfig):
             sys.exc_info(),
             request_components=get_request_components(request),
             request_path=request.path,
-            request_params=[(k, v) for k, vs in request.GET.lists() for v in vs],
+            request_params=dict(request.GET.lists()),
             session=dict(request.session.items())
             if hasattr(request, "session")
             else None,
diff --git a/src/scout_apm/flask/__init__.py b/src/scout_apm/flask/__init__.py
index 2007630..dfbb599 100644
--- a/src/scout_apm/flask/__init__.py
+++ b/src/scout_apm/flask/__init__.py
@@ -66,9 +66,7 @@ class ScoutApm(object):
                         sys.exc_info(),
                         request_components=get_request_components(self.app, request),
                         request_path=request.path,
-                        request_params=[
-                            (k, v) for k, vs in request.args.lists() for v in vs
-                        ],
+                        request_params=dict(request.args.lists()),
                         session=dict(session.items()),
                         environment=self.app.config,
                     )
diff --git a/tox.ini b/tox.ini
index c116f77..0958a60 100644
--- a/tox.ini
+++ b/tox.ini
@@ -56,7 +56,7 @@ deps =
     huey
     hug>=2.5.1 ; python_version >= "3.5"
     httpretty<1 ; python_version < "3.5"
-    httpretty!=1.1.1 ; python_version >= "3.5"
+    httpretty ; python_version >= "3.5"
     jinja2
     ; nameko tests temporarily disabled
     ; Due to bad pinning of kombu only a past nameko version can be installed
@@ -73,7 +73,6 @@ deps =
     pyyaml ; python_version != "3.4"
     pyyaml < 5.3 ; python_version == "3.4"
     redis
-    requests
     rq
     six
     sqlalchemy ; python_version != "3.4"


Test patch:
diff --git a/tests/compat.py b/tests/compat.py
index fd9d12e..a53b115 100644
--- a/tests/compat.py
+++ b/tests/compat.py
@@ -64,11 +64,11 @@ if sys.version_info >= (3, 2):
         return gzip.decompress(data)
 
 else:
-    import io
+    import StringIO
 
     def gzip_decompress(data):
         """Reimplementation gzip.compress for python 2.7"""
-        with gzip.GzipFile(fileobj=io.BytesIO(data), mode="rb") as f:
+        with gzip.GzipFile(fileobj=StringIO.StringIO(data), mode="rb") as f:
             return f.read()
 
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 216352a..06c2c4c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -245,10 +245,14 @@ def tracked_requests():
         TrackedRequest.finish = orig
 
 
-@pytest.fixture
-def error_monitor_errors():
+@pytest.fixture(autouse=True, params=[False])
+def error_monitor_errors(request):
     """
     Mock calls made to the error service thread.
+
+    Will prevent messages being sent to the ErrorServiceThread unless
+    the fixture is overridden with ``@pytest.fixture(params=[True])``
+
     Returns a list of error dicts
     """
     errors = []
@@ -256,7 +260,9 @@ def error_monitor_errors():
     @wrapt.decorator
     def capture_error(wrapped, instance, args, kwargs):
         errors.append(kwargs["error"])
-        return wrapped(*args, **kwargs)
+        send = request.param
+        if send:
+            return wrapped(*args, **kwargs)
 
     orig = ErrorServiceThread.send
     ErrorServiceThread.send = capture_error(orig)
diff --git a/tests/integration/core/test_error_service.py b/tests/integration/core/test_error_service.py
index 779b54c..0f51958 100644
--- a/tests/integration/core/test_error_service.py
+++ b/tests/integration/core/test_error_service.py
@@ -1,6 +1,7 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import json
 import logging
 import os
 from datetime import datetime
@@ -13,7 +14,7 @@ from scout_apm.core.config import scout_config
 from scout_apm.core.error_service import ErrorServiceThread
 from tests.compat import gzip_decompress
 
-WORKING_DIRECTORY = os.getcwd().encode("utf-8")
+WORKING_DIRECTORY = os.getcwd()
 
 
 @pytest.fixture
@@ -23,8 +24,17 @@ def error_service_thread():
     # ensure_stopped() already called by global stop_and_empty_core_agent_socket
 
 
+@pytest.fixture(params=[True])
+def error_monitor_errors(error_monitor_errors):
+    """
+    Override error_monitor_errors to capture the messages sent to
+    ErrorServiceThread
+    """
+    return error_monitor_errors
+
+
 @pytest.mark.parametrize(
-    "config, decompressed_body, expected_headers, expected_uri",
+    "config, decoded_body, expected_headers, expected_uri",
     [
         (
             {
@@ -35,69 +45,78 @@ def error_service_thread():
                 "application_root": "/tmp/",
                 "errors_host": "https://testserver",
             },
-            b'{"notifier": "scout_apm_python", "environment": "scout-test", '
-            b'"root": "/tmp/", "problems": [{"foo": "bar"}]}',
+            {
+                "notifier": "scout_apm_python",
+                "environment": "scout-test",
+                "root": "/tmp/",
+                "problems": [{"foo": "BØØM!"}],
+            },
             {"Agent-Hostname": "example.com", "X-Error-Count": "1"},
             "https://testserver/apps/error.scout?key=scout-app-key"
             "&name=scout+test+app",
         ),
         (
             {},
-            b'{"notifier": "scout_apm_python", "environment": null, '
-            b'"root": "' + WORKING_DIRECTORY + b'", "problems": [{"foo": "bar"}]}',
+            {
+                "notifier": "scout_apm_python",
+                "environment": None,
+                "root": WORKING_DIRECTORY,
+                "problems": [{"foo": "BØØM!"}],
+            },
             {"Agent-Hostname": None, "X-Error-Count": "1"},
-            "https://errors.scoutapm.com/apps/error.scout" "?name=Python+App",
+            "https://errors.scoutapm.com/apps/error.scout?name=Python+App",
         ),
     ],
 )
 def test_send(
-    config, decompressed_body, expected_headers, expected_uri, error_service_thread
+    config, decoded_body, expected_headers, expected_uri, error_service_thread
 ):
     scout_config.set(**config)
 
-    def request_callback(request, uri, response_headers):
-        assert uri == expected_uri
-        for key, value in expected_headers.items():
-            assert request.headers.get(key) == value
-        assert gzip_decompress(request.body) == decompressed_body
-        return [200, response_headers, "Hello world!"]
-
     try:
         with httpretty.enabled(allow_net_connect=False):
             httpretty.register_uri(
                 httpretty.POST,
-                "https://errors.scoutapm.com/apps/error.scout",
-                body=request_callback,
+                expected_uri,
+                body="Hello World!",
             )
-            ErrorServiceThread.send({"foo": "bar"})
+            ErrorServiceThread.send(error={"foo": "BØØM!"})
             ErrorServiceThread.wait_until_drained()
+
+            request = httpretty.last_request()
+            assert (
+                json.loads(gzip_decompress(request.body).decode("utf-8"))
+                == decoded_body
+            )
+            assert request.headers.get("X-Error-Count") == "1"
     finally:
         scout_config.reset_all()
 
 
 def test_send_batch(error_service_thread):
-    def request_callback(request, uri, response_headers):
-        decompressed_body = (
-            b'{"notifier": "scout_apm_python", "environment": null, '
-            b'"root": "'
-            + WORKING_DIRECTORY
-            + b'", "problems": [{"foo": 0}, {"foo": 1}, '
-            b'{"foo": 2}, {"foo": 3}, {"foo": 4}]}'
-        )
-        assert gzip_decompress(request.body) == decompressed_body
-        assert request.headers.get("X-Error-Count") == "5"
-        return [200, response_headers, "Hello world!"]
-
+    decompressed_body = {
+        "notifier": "scout_apm_python",
+        "root": WORKING_DIRECTORY,
+        "environment": None,
+        "problems": [{"foo": 0}, {"foo": 1}, {"foo": 2}, {"foo": 3}, {"foo": 4}],
+    }
     try:
         with httpretty.enabled(allow_net_connect=False):
             httpretty.register_uri(
                 httpretty.POST,
                 "https://errors.scoutapm.com/apps/error.scout",
-                body=request_callback,
+                body="Hello world!",
             )
             for i in range(5):
-                ErrorServiceThread.send({"foo": i})
+                ErrorServiceThread.send(error={"foo": i})
             ErrorServiceThread.wait_until_drained()
+
+            request = httpretty.last_request()
+            assert (
+                json.loads(gzip_decompress(request.body).decode("utf-8"))
+                == decompressed_body
+            )
+            assert request.headers.get("X-Error-Count") == "5"
     finally:
         scout_config.reset_all()
 
@@ -111,20 +130,20 @@ def test_send_api_error(error_service_thread, caplog):
                 body="Unexpected Error",
                 status=500,
             )
-            ErrorServiceThread.send({"foo": "bar"})
+            ErrorServiceThread.send(error={"foo": "BØØM!"})
             ErrorServiceThread.wait_until_drained()
     finally:
         scout_config.reset_all()
     assert caplog.record_tuples[-1][0] == "scout_apm.core.error_service"
     assert caplog.record_tuples[-1][1] == logging.DEBUG
     assert caplog.record_tuples[-1][2].startswith(
-        "ErrorServiceThread exception on _send:"
+        "ErrorServiceThread 500 response error on _send:"
     )
 
 
 def test_send_unserializable_data(error_service_thread, caplog):
     with httpretty.enabled(allow_net_connect=False):
-        ErrorServiceThread.send({"value": datetime.now()})
+        ErrorServiceThread.send(error={"value": datetime.now()})
         ErrorServiceThread.wait_until_drained()
 
     if ErrorServiceThread._queue.empty() and not caplog.record_tuples:
diff --git a/tests/integration/test_django.py b/tests/integration/test_django.py
index 644e866..7acdd27 100644
--- a/tests/integration/test_django.py
+++ b/tests/integration/test_django.py
@@ -284,7 +284,7 @@ def test_server_error_error_monitor_with_params(tracked_requests, error_monitor_
 
     assert len(error_monitor_errors) == 1
     error = error_monitor_errors[0]
-    assert error["request_params"] == [("spam[]", "eggs"), ("spam[]", "false")]
+    assert error["request_params"] == {"spam[]": ["eggs", "false"]}
 
 
 def test_return_error(tracked_requests):
diff --git a/tests/integration/test_flask.py b/tests/integration/test_flask.py
index f8f8a67..666f6e3 100644
--- a/tests/integration/test_flask.py
+++ b/tests/integration/test_flask.py
@@ -285,7 +285,7 @@ def test_server_error_error_monitor_with_params(tracked_requests, error_monitor_
 
     assert len(error_monitor_errors) == 1
     error = error_monitor_errors[0]
-    assert error["request_params"] == [("spam[]", "eggs"), ("spam[]", "false")]
+    assert error["request_params"] == {"spam[]": ["eggs", "false"]}
 
 
 def test_return_error_error_monitor(tracked_requests, error_monitor_errors):
diff --git a/tests/unit/core/test_error.py b/tests/unit/core/test_error.py
index b02b5c6..7f1d32c 100644
--- a/tests/unit/core/test_error.py
+++ b/tests/unit/core/test_error.py
@@ -105,7 +105,7 @@ def test_monitor_ignore_exceptions(error_monitor_errors):
         ),
         (
             "/test/",
-            [("foo", "bar")],
+            {"foo": ["bar"]},
             {"spam": "eggs"},
             {"PASSWORD": "hunter2"},
             RequestComponents("sample.app", "DataView", "detail"),
@@ -116,7 +116,7 @@ def test_monitor_ignore_exceptions(error_monitor_errors):
                 "message": "division by zero",
                 "request_id": "sample_id",
                 "request_uri": "/test/",
-                "request_params": [("foo", "bar")],
+                "request_params": {"foo": ["bar"]},
                 "request_session": {"spam": "eggs"},
                 "environment": {"PASSWORD": "[FILTERED]"},
                 "request_components": {
@@ -131,7 +131,7 @@ def test_monitor_ignore_exceptions(error_monitor_errors):
         ),
         (
             "/test/",
-            [("foo", "bar")],
+            {"foo": ["bar"]},
             {"spam": "eggs"},
             {"PASSWORD": "hunter2"},
             RequestComponents("sample.app", "DataView", "detail"),
@@ -142,7 +142,7 @@ def test_monitor_ignore_exceptions(error_monitor_errors):
                 "message": "division by zero",
                 "request_id": "sample_id",
                 "request_uri": "/test/",
-                "request_params": [("foo", "bar")],
+                "request_params": {"foo": ["bar"]},
                 "request_session": {"spam": "eggs"},
                 "environment": {"PASSWORD": "[FILTERED]"},
                 "request_components": {