Skip to content

Commit

Permalink
Add aiohttp support for expected status codes
Browse files Browse the repository at this point in the history
  • Loading branch information
justin-richert committed Jan 4, 2023
1 parent fd0fd01 commit 072d5a3
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 9 deletions.
6 changes: 5 additions & 1 deletion newrelic/api/time_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, "_is_expected"):
is_expected = transaction._is_expected(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
Expand Down Expand Up @@ -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

Expand Down
18 changes: 17 additions & 1 deletion newrelic/hooks/framework_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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._is_expected = is_expected(transaction)

import aiohttp

transaction.add_framework_info(name="aiohttp", version=aiohttp.__version__)
Expand Down
5 changes: 5 additions & 0 deletions tests/framework_aiohttp/_target_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion tests/framework_aiohttp/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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):
Expand All @@ -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=[
Expand Down Expand Up @@ -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())

Expand Down
25 changes: 25 additions & 0 deletions tests/testing_support/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,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"]
Expand Down
18 changes: 12 additions & 6 deletions tests/testing_support/validators/validate_transaction_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
# 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,
)
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 []
Expand Down Expand Up @@ -71,6 +72,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

0 comments on commit 072d5a3

Please sign in to comment.