Bug #1316655: Upgrade Falcon to 1.1

antenna/app.py

@@ -2,7 +2,6 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from functools import wraps
 import logging
 import os
 import logging.config
@@ -62,18 +61,6 @@ def setup_metrics(metrics_class, config):
     metrics.metrics_configure(metrics_class, config)
 
 
-def log_unhandled(fun):
-    @wraps(fun)
-    def _log_unhandled(*args, **kwargs):
-        try:
-            return fun(*args, **kwargs)
-        except Exception:
-            logger.exception('UNHANDLED EXCEPTION!')
-            raise
-
-    return _log_unhandled
-
-
 class AppConfig(RequiredConfigMixin, LogConfigMixin):
     """Application-level config
 
@@ -195,31 +182,35 @@ def log_config(self, logger):
                 res.log_config(logger)
 
 
-@log_unhandled
 def get_app(config=None):
     """Returns AntennaAPI instance"""
-    if config is None:
-        config = ConfigManager([
-            # Pull configuration from env file specified as ANTENNA_ENV
-            ConfigEnvFileEnv([os.environ.get('ANTENNA_ENV')]),
-            # Pull configuration from environment variables
-            ConfigOSEnv()
-        ])
-
-    app_config = AppConfig(config)
-    setup_logging(app_config('logging_level'))
-    setup_metrics(app_config('metrics_class'), config)
-
-    app_config.log_config(logger)
-
-    app = AntennaAPI(config)
-    app.add_error_handler(Exception, handler=app.unhandled_exception_handler)
-
-    app.add_route('homepage', '/', HomePageResource(config))
-    app.add_route('breakpad', '/submit', BreakpadSubmitterResource(config))
-    app.add_route('version', '/__version__', VersionResource(config, basedir=app_config('basedir')))
-    app.add_route('heartbeat', '/__heartbeat__', HeartbeatResource(config, app))
-    app.add_route('lbheartbeat', '/__lbheartbeat__', LBHeartbeatResource(config))
-
-    app.log_config(logger)
-    return app
+    try:
+        if config is None:
+            config = ConfigManager([
+                # Pull configuration from env file specified as ANTENNA_ENV
+                ConfigEnvFileEnv([os.environ.get('ANTENNA_ENV')]),
+                # Pull configuration from environment variables
+                ConfigOSEnv()
+            ])
+
+        app_config = AppConfig(config)
+        setup_logging(app_config('logging_level'))
+        setup_metrics(app_config('metrics_class'), config)
+
+        app_config.log_config(logger)
+
+        app = AntennaAPI(config)
+        app.add_error_handler(Exception, handler=app.unhandled_exception_handler)
+
+        app.add_route('homepage', '/', HomePageResource(config))
+        app.add_route('breakpad', '/submit', BreakpadSubmitterResource(config))
+        app.add_route('version', '/__version__', VersionResource(config, basedir=app_config('basedir')))
+        app.add_route('heartbeat', '/__heartbeat__', HeartbeatResource(config, app))
+        app.add_route('lbheartbeat', '/__lbheartbeat__', LBHeartbeatResource(config))
+
+        app.log_config(logger)
+        return app
+
+    except Exception:
+        logger.exception('Unhandled startup exception')
+        raise

antenna/breakpad_resource.py

@@ -124,10 +124,8 @@ def extract_payload(self, req):
             # have to do that here because nginx doesn't have a good way to do
             # that in nginx-land.
             gzip_header = 16 + zlib.MAX_WBITS
-            content_length = int(req.env.get('CONTENT_LENGTH', 0))
-            data = zlib.decompress(
-                req.stream.read(content_length), gzip_header
-            )
+            content_length = req.content_length or 0
+            data = zlib.decompress(req.stream.read(content_length), gzip_header)
 
             # Stomp on the content length to correct it because we've changed
             # the payload size by decompressing it. We save the original value
@@ -137,7 +135,7 @@ def extract_payload(self, req):
 
             data = io.BytesIO(data)
         else:
-            data = req.stream
+            data = io.BytesIO(req.stream.read(req.content_length or 0))
 
         fs = cgi.FieldStorage(fp=data, environ=req.env, keep_blank_values=1)
 
@@ -199,11 +197,7 @@ def get_throttle_result(self, raw_crash):
                 if not (0 <= percentage <= 100):
                     raise ValueError('Percentage is not a valid value: %r', result)
 
-                return (
-                    result,
-                    'ALREADY_THROTTLED',
-                    percentage
-                )
+                return result, 'ALREADY_THROTTLED', percentage
 
             except ValueError:
                 # If we've gotten a ValueError, it means one or both of the

requirements.txt

@@ -9,7 +9,9 @@ decorator==4.0.10 \
 everett==0.5 \
     --hash=sha256:5fe8201ee5c05f3cd8ebc8393a555150da5133b577ed63bb048d9bbc9321b1c5 \
     --hash=sha256:b427bd9ce32d7de503e896e1195350006a43812ce8eaad26990cd2d93a1b8020
-falcon==1.0.0 --hash=sha256:0145eb7fc001fb943b8b8f797832ce45302cf7c1edc769e28e5bf0963a406403
+falcon==1.1.0 \
+    --hash=sha256:5ed3d950c90543713de91088ccd2010e80522f2fcb95a8987fc7a56f2829f653 \
+    --hash=sha256:a68c5685459427cc2129c7b9e3aa19ed5adc73b6adff8ca69fe09f6666885ae7
 gevent==1.1.2 \
     --hash=sha256:ca934dc30f26e84c79e483db4de433702283ff02344d029635730ff9e7c1d416 \
     --hash=sha256:f42de31de5f29a11f5480f76c05f3afce80e030333295718efd64bfb823e4e82 \

tests/unittest/conftest.py

@@ -4,18 +4,14 @@
 
 import contextlib
 from pathlib import Path
-
 import sys
 from unittest import mock
 
 from everett.manager import ConfigManager
-import falcon
 from falcon.request import Request
 from falcon.testing.helpers import create_environ
-from falcon.testing.srmock import StartResponseMock
-from falcon.testing.test_case import Result
+from falcon.testing.client import TestClient
 import pytest
-import wsgiref.validate
 
 
 # Add repository root so we can import Antenna.
@@ -38,26 +34,10 @@ def pytest_runtest_setup():
     setup_metrics(metrics.LoggingMetrics, ConfigManager.from_dict({}))
 
 
-def build_app(config=None):
-    if config is None:
-        config = {}
-
-    # Falcon 1.0 has a global variable that denotes whether it should wrap the
-    # wsgi stream or not. Every time we rebuild the app, we should set this to
-    # False. If we set it to True, it wraps the stream in a
-    # falcon.request_helpers.Body which breaks FieldStorage parsing and then we
-    # end up with empty crash reports. If we don't set it at all, then tests fail
-    # until a test runs which causes it to flip to False first.
-    falcon.request._maybe_wrap_wsgi_stream = False
-
-    return get_app(ConfigManager.from_dict(config))
-
-
 @pytest.fixture
 def request_generator():
     """Returns a Falcon Request generator"""
-    def _request_generator(method, path, query_string=None, headers=None,
-                           body=None):
+    def _request_generator(method, path, query_string=None, headers=None, body=None):
         env = create_environ(
             method=method,
             path=path,
@@ -70,20 +50,8 @@ def _request_generator(method, path, query_string=None, headers=None,
     return _request_generator
 
 
-class Client:
-    """Test client to ease testing with the Antenna API
-
-    .. Note::
-
-       Falcon 1.0 has test infrastructure that's interesting, but is
-       unittest-based and does some weird things that I found difficult to work
-       around. A future version of Falcon will have better things so we can
-       probably rip all this out when we upgrade.
-
-    """
-    def __init__(self, app):
-        self.app = app
-
+class AntennaTestClient(TestClient):
+    """Test client to ease testing with Antenna API"""
     def rebuild_app(self, new_config=None):
         """Rebuilds the app
 
@@ -95,7 +63,7 @@ def rebuild_app(self, new_config=None):
         """
         if new_config is None:
             new_config = {}
-        self.app = build_app(new_config)
+        self.app = get_app(ConfigManager.from_dict(new_config))
 
     def join_app(self):
         """This goes through and calls join on all gevent pools in the app
@@ -116,34 +84,6 @@ def join_app(self):
         bsr = self.app.get_resource_by_name('breakpad')
         bsr.join_pool()
 
-    def get(self, path, headers=None, **kwargs):
-        """Performs a fake HTTP GET request"""
-        return self._request('GET', path=path, headers=headers, **kwargs)
-
-    def post(self, path, headers=None, body=None, **kwargs):
-        """Performs a fake HTTP POST request"""
-        return self._request('POST', path=path, headers=headers, body=body, **kwargs)
-
-    def _request(self, method, path, query_string=None, headers=None,
-                 body=None, **kwargs):
-        env = create_environ(
-            method=method,
-            path=path,
-            query_string=(query_string or ''),
-            headers=headers,
-            body=body,
-        )
-
-        resp = StartResponseMock()
-        # Wrap the app in a validator which will raise an assertion error if
-        # either side isn't speaking valid WSGI.
-        validator = wsgiref.validate.validator(self.app)
-        iterable = validator(env, resp)
-
-        result = Result(iterable, resp.status, resp.headers)
-
-        return result
-
 
 @pytest.fixture
 def client():
@@ -161,7 +101,7 @@ def test_foo(client, tmpdir):
             })
 
     """
-    return Client(build_app())
+    return AntennaTestClient(get_app(ConfigManager.from_dict({})))
 
 
 @pytest.yield_fixture

tests/unittest/test_app.py

@@ -5,9 +5,9 @@
 
 class TestBasic:
     def test_404(self, client):
-        result = client.get('/foo')
+        result = client.simulate_get('/foo')
         assert result.status_code == 404
 
     def test_home_page(self, client):
-        result = client.get('/')
+        result = client.simulate_get('/')
         assert result.status_code == 200

tests/unittest/test_breakpad_resource.py

@@ -22,7 +22,7 @@ def test_submit_crash_report_reply(self, client):
             'upload_file_minidump': ('fakecrash.dump', io.BytesIO(b'abcd1234'))
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data,
@@ -125,7 +125,7 @@ def test_existing_uuid(self, client):
             'upload_file_minidump': ('fakecrash.dump', io.BytesIO(b'abcd1234'))
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data
@@ -149,7 +149,7 @@ def test_legacy_processing(self, client, loggingmock):
         })
 
         with loggingmock(['antenna']) as lm:
-            result = client.post(
+            result = client.simulate_post(
                 '/submit',
                 headers=headers,
                 body=data
@@ -188,7 +188,7 @@ def test_legacy_processing_bad_values(self, legacy, percentage, client, loggingm
         })
 
         with loggingmock(['antenna']) as lm:
-            result = client.post(
+            result = client.simulate_post(
                 '/submit',
                 headers=headers,
                 body=data
@@ -215,7 +215,7 @@ def test_throttleable(self, client, loggingmock):
         })
 
         with loggingmock(['antenna']) as lm:
-            result = client.post(
+            result = client.simulate_post(
                 '/submit',
                 headers=headers,
                 body=data

tests/unittest/test_crashstorage.py

@@ -24,7 +24,7 @@ def test_flow(self, client):
             'upload_file_minidump': ('fakecrash.dump', io.BytesIO(b'abcd1234'))
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data

tests/unittest/test_fs_crashstorage.py

@@ -40,7 +40,7 @@ def test_storage_files(self, client, tmpdir):
             'CRASHSTORAGE_FS_ROOT': str(tmpdir.join('antenna_crashes')),
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data
@@ -116,7 +116,7 @@ def test_load_files(self, client, tmpdir):
             'CRASHSTORAGE_FS_ROOT': str(tmpdir.join('antenna_crashes')),
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data

tests/unittest/test_health_resource.py

@@ -11,7 +11,7 @@ def test_no_version(self, client, tmpdir):
             'BASEDIR': str(tmpdir)
         })
 
-        result = client.get('/__version__')
+        result = client.simulate_get('/__version__')
         assert result.content == b'{}'
 
     def test_version(self, client, tmpdir):
@@ -25,15 +25,15 @@ def test_version(self, client, tmpdir):
         version_path = tmpdir.join('/version.json')
         version_path.write('{"commit": "ou812"}')
 
-        result = client.get('/__version__')
+        result = client.simulate_get('/__version__')
         assert result.content == b'{"commit": "ou812"}'
 
     def test_lb_heartbeat(self, client):
-        resp = client.get('/__lbheartbeat__')
+        resp = client.simulate_get('/__lbheartbeat__')
         assert resp.status_code == 200
 
     def test_heartbeat(self, client):
-        resp = client.get('/__heartbeat__')
+        resp = client.simulate_get('/__heartbeat__')
         assert resp.status_code == 200
         # NOTE(willkg): This isn't mocked out, so it's entirely likely that
         # this expected result will change over time.

tests/unittest/test_s3_crashstorage.py

@@ -51,7 +51,7 @@ def test_crash_storage(self, client, s3mock):
             'CRASHSTORAGE_BUCKET_NAME': 'fakebucket',
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data
@@ -108,7 +108,7 @@ def test_region_and_bucket_with_periods(self, client, s3mock):
             'CRASHSTORAGE_BUCKET_NAME': 'fakebucket.with.periods',
         })
 
-        result = client.post(
+        result = client.simulate_post(
             '/submit',
             headers=headers,
             body=data
@@ -204,7 +204,7 @@ def test_retrying(self, client, s3mock, loggingmock):
         })
 
         with loggingmock(['antenna']) as lm:
-            result = client.post(
+            result = client.simulate_post(
                 '/submit',
                 headers=headers,
                 body=data