Add aiohttp support for expected status codes (#735)

* Add aiohttp support for expected status codes

* Adjust naming convention

* Fix expected tests for new validator behavior

---------

Co-authored-by: Timothy Pansino <11214426+TimPansino@users.noreply.github.com>
Co-authored-by: Tim Pansino <timpansino@gmail.com>

newrelic/api/time_trace.py

@@ -327,6 +327,10 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
         if is_expected is None and callable(expected):
             is_expected = expected(exc, value, tb)
 
+        # Callable on transaction
+        if is_expected is None and hasattr(transaction, "_expect_errors"):
+            is_expected = transaction._expect_errors(exc, value, tb)
+
         # List of class names
         if is_expected is None and expected is not None and not callable(expected):
             # Do not set is_expected to False
@@ -631,7 +635,7 @@ def get_service_linking_metadata(application=None, settings=None):
         if application is None:
             from newrelic.api.application import application_instance
             application = application_instance(activate=False)
-        
+
         if application is not None:
             settings = application.settings
 

newrelic/hooks/framework_aiohttp.py

@@ -26,7 +26,7 @@
     function_wrapper,
     wrap_function_wrapper,
 )
-from newrelic.core.config import should_ignore_error
+from newrelic.core.config import is_expected_error, should_ignore_error
 
 SUPPORTED_METHODS = ("connect", "head", "get", "delete", "options", "patch", "post", "put", "trace")
 
@@ -61,6 +61,19 @@ def _should_ignore(exc, value, tb):
     return _should_ignore
 
 
+def is_expected(transaction):
+    settings = transaction.settings
+
+    def _is_expected(exc, value, tb):
+        from aiohttp import web
+
+        if isinstance(value, web.HTTPException):
+            status_code = value.status_code
+            return is_expected_error((exc, value, tb), status_code, settings=settings)
+
+    return _is_expected
+
+
 def _nr_process_response_proxy(response, transaction):
     nr_headers = transaction.process_response(response.status, response.headers)
 
@@ -338,6 +351,9 @@ async def _coro(*_args, **_kwargs):
         # Patch in should_ignore to all notice_error calls
         transaction._ignore_errors = should_ignore(transaction)
 
+        # Patch in is_expected to all notice_error calls
+        transaction._expect_errors = is_expected(transaction)
+
         import aiohttp
 
         transaction.add_framework_info(name="aiohttp", version=aiohttp.__version__)

tests/agent_features/test_ignore_expected_errors.py

@@ -94,8 +94,9 @@ def test_classes_error_event_inside_transaction(settings, expected, ignore):
 
     error_count = 1 if not ignore else 0
     errors = _test_runtime_error if not ignore else []
+    expected_errors = _runtime_error_name if expected and not ignore else None
 
-    @validate_transaction_errors(errors=errors)
+    @validate_transaction_errors(errors=errors, expected_errors=expected_errors)
     @validate_error_event_sample_data(
         required_attrs=attributes,
         required_user_attrs=False,
@@ -268,8 +269,9 @@ def test_status_codes_inside_transaction(settings, expected, ignore, status_code
 
     error_count = 1 if not ignore else 0
     errors = _test_teapot_error if not ignore else []
+    expected_errors = _teapot_error_name if expected and not ignore else None
 
-    @validate_transaction_errors(errors=errors)
+    @validate_transaction_errors(errors=errors, expected_errors=expected_errors)
     @validate_error_event_sample_data(
         required_attrs=attributes,
         required_user_attrs=False,
@@ -359,8 +361,9 @@ def test_mixed_ignore_expected_settings_inside_transaction(
 
     error_count = 1 if not ignore else 0
     errors = _test_runtime_error if not ignore else []
+    expected_errors = _runtime_error_name if expected and not ignore else None
 
-    @validate_transaction_errors(errors=errors)
+    @validate_transaction_errors(errors=errors, expected_errors=expected_errors)
     @validate_error_event_sample_data(
         required_attrs=attributes,
         required_user_attrs=False,
@@ -428,8 +431,9 @@ def test_overrides_inside_transaction(override, result, parameter):
 
     error_count = 1 if not ignore else 0
     errors = _test_runtime_error if not ignore else []
+    expected_errors = _runtime_error_name if expected and not ignore else None
 
-    @validate_transaction_errors(errors=errors)
+    @validate_transaction_errors(errors=errors, expected_errors=expected_errors)
     @validate_error_event_sample_data(
         required_attrs=attributes,
         required_user_attrs=False,

tests/framework_aiohttp/_target_application.py

@@ -40,6 +40,10 @@ async def non_500_error(request):
     raise web.HTTPGone()
 
 
+async def raise_403(request):
+    raise web.HTTPForbidden()
+
+
 async def raise_404(request):
     raise web.HTTPNotFound()
 
@@ -167,6 +171,7 @@ def make_app(middlewares=None):
     app.router.add_route("*", "/error", error)
     app.router.add_route("*", "/known_error", KnownErrorView)
     app.router.add_route("*", "/non_500_error", non_500_error)
+    app.router.add_route("*", "/raise_403", raise_403)
     app.router.add_route("*", "/raise_404", raise_404)
     app.router.add_route("*", "/hang", hang)
     app.router.add_route("*", "/background", background)

tests/framework_aiohttp/test_server.py

@@ -19,6 +19,7 @@
 from testing_support.fixtures import (
     count_transactions,
     override_application_settings,
+    override_expected_status_codes,
     override_generic_settings,
     override_ignore_status_codes,
 )
@@ -64,6 +65,7 @@
         ("/error?hello=world", "_target_application:error", "builtins:ValueError", 500),
         ("/non_500_error?hello=world", "_target_application:non_500_error", "aiohttp.web_exceptions:HTTPGone", 410),
         ("/raise_404?hello=world", "_target_application:raise_404", None, 404),
+        ("/raise_403?hello=world", "_target_application:raise_403", "aiohttp.web_exceptions:HTTPForbidden", 403),
     ],
 )
 def test_error_exception(method, uri, metric_name, error, status, nr_enabled, aiohttp_app):
@@ -79,7 +81,9 @@ async def fetch():
         if error:
             errors.append(error)
 
-        @validate_transaction_errors(errors=errors)
+        @validate_transaction_errors(
+            errors=errors, expected_errors=["aiohttp.web_exceptions:HTTPForbidden"]
+        )
         @validate_transaction_metrics(
             metric_name,
             scoped_metrics=[
@@ -111,6 +115,7 @@ async def fetch():
         )
         @validate_code_level_metrics(*metric_name.split(":"))
         @override_ignore_status_codes([404])
+        @override_expected_status_codes([403])
         def _test():
             aiohttp_app.loop.run_until_complete(fetch())
 

tests/testing_support/fixtures.py

@@ -1468,6 +1468,31 @@ def _override_ignore_status_codes(wrapped, instance, args, kwargs):
     return _override_ignore_status_codes
 
 
+def override_expected_status_codes(status_codes):
+    @function_wrapper
+    def _override_expected_status_codes(wrapped, instance, args, kwargs):
+        # Updates can be made to expected status codes in server
+        # side configs. Changes will be applied to application
+        # settings so we first check there and if they don't
+        # exist, we default to global settings
+
+        application = application_instance()
+        settings = application and application.settings
+
+        if not settings:
+            settings = global_settings()
+
+        original = settings.error_collector.expected_status_codes
+
+        try:
+            settings.error_collector.expected_status_codes = status_codes
+            return wrapped(*args, **kwargs)
+        finally:
+            settings.error_collector.expected_status_codes = original
+
+    return _override_expected_status_codes
+
+
 def code_coverage_fixture(source=None):
     if source is None:
         source = ["newrelic"]

tests/testing_support/validators/validate_transaction_errors.py

@@ -12,18 +12,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import copy
-
 from newrelic.common.object_wrapper import (
     function_wrapper,
     transient_function_wrapper,
 )
 from testing_support.fixtures import catch_background_exceptions
 
-def validate_transaction_errors(errors=None, required_params=None, forgone_params=None):
+
+def validate_transaction_errors(
+    errors=None, required_params=None, forgone_params=None, expected_errors=None
+):
     errors = errors or []
     required_params = required_params or []
     forgone_params = forgone_params or []
+    expected_errors = expected_errors or []
     captured_errors = []
 
     @transient_function_wrapper("newrelic.core.stats_engine", "StatsEngine.record_transaction")
@@ -71,6 +73,11 @@ def _validate_transaction_errors(wrapped, instance, args, kwargs):
             for name, value in forgone_params:
                 assert name not in e.custom_params, "name=%r, params=%r" % (name, e.custom_params)
 
+            if e.type in expected_errors:
+                assert e.expected is True
+            else:
+                assert e.expected is False
+
         return output
 
     return _validate_transaction_errors