diff --git a/newrelic/api/time_trace.py b/newrelic/api/time_trace.py index fd0f62fdef..800c6f01b7 100644 --- a/newrelic/api/time_trace.py +++ b/newrelic/api/time_trace.py @@ -362,15 +362,19 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c def notice_error(self, error=None, attributes=None, expected=None, ignore=None, status_code=None): attributes = attributes if attributes is not None else {} - # If no exception details provided, use current exception. + # If an exception instance is passed, attempt to unpack it into an exception tuple with traceback + if isinstance(error, BaseException): + error = (type(error), error, getattr(error, "__traceback__", None)) - # Pull from sys.exc_info if no exception is passed - if not error or None in error: + # Use current exception from sys.exc_info() if no exception was passed, + # or if the exception tuple is missing components like the traceback + if not error or (isinstance(error, (tuple, list)) and None in error): error = sys.exc_info() - # If no exception to report, exit - if not error or None in error: - return + # Error should be a tuple or list of 3 elements by this point. + # If it's falsey or missing a component like the traceback, quietly exit early. + if not isinstance(error, (tuple, list)) or len(error) != 3 or None in error: + return exc, value, tb = error diff --git a/newrelic/core/stats_engine.py b/newrelic/core/stats_engine.py index f44f82fe13..f4a0e98ff6 100644 --- a/newrelic/core/stats_engine.py +++ b/newrelic/core/stats_engine.py @@ -678,7 +678,6 @@ def record_time_metrics(self, metrics): def notice_error(self, error=None, attributes=None, expected=None, ignore=None, status_code=None): attributes = attributes if attributes is not None else {} settings = self.__settings - if not settings: return @@ -690,13 +689,19 @@ def notice_error(self, error=None, attributes=None, expected=None, ignore=None, if not settings.collect_errors and not settings.collect_error_events: return - # Pull from sys.exc_info if no exception is passed - if not error or None in error: + # If an exception instance is passed, attempt to unpack it into an exception tuple with traceback + if isinstance(error, BaseException): + error = (type(error), error, getattr(error, "__traceback__", None)) + + # Use current exception from sys.exc_info() if no exception was passed, + # or if the exception tuple is missing components like the traceback + if not error or (isinstance(error, (tuple, list)) and None in error): error = sys.exc_info() - # If no exception to report, exit - if not error or None in error: - return + # Error should be a tuple or list of 3 elements by this point. + # If it's falsey or missing a component like the traceback, quietly exit early. + if not isinstance(error, (tuple, list)) or len(error) != 3 or None in error: + return exc, value, tb = error diff --git a/tests/agent_features/test_notice_error.py b/tests/agent_features/test_notice_error.py index e698dee7be..60e617f9de 100644 --- a/tests/agent_features/test_notice_error.py +++ b/tests/agent_features/test_notice_error.py @@ -39,10 +39,8 @@ # =============== Test errors during a transaction =============== -_test_notice_error_sys_exc_info = [(_runtime_error_name, "one")] - -@validate_transaction_errors(errors=_test_notice_error_sys_exc_info) +@validate_transaction_errors(errors=[(_runtime_error_name, "one")]) @background_task() def test_notice_error_sys_exc_info(): try: @@ -51,10 +49,7 @@ def test_notice_error_sys_exc_info(): notice_error(sys.exc_info()) -_test_notice_error_no_exc_info = [(_runtime_error_name, "one")] - - -@validate_transaction_errors(errors=_test_notice_error_no_exc_info) +@validate_transaction_errors(errors=[(_runtime_error_name, "one")]) @background_task() def test_notice_error_no_exc_info(): try: @@ -63,10 +58,44 @@ def test_notice_error_no_exc_info(): notice_error() -_test_notice_error_custom_params = [(_runtime_error_name, "one")] +@validate_transaction_errors(errors=[(_runtime_error_name, "one")]) +@background_task() +def test_notice_error_exception_instance(): + """Test that notice_error works when passed an exception object directly""" + try: + raise RuntimeError("one") + except RuntimeError as e: + exc = e # Reassign name to ensure scope isn't lost + + # Call notice_error outside of try/except block to ensure it's not pulling from sys.exc_info() + notice_error(exc) + + +@validate_transaction_errors(errors=[(_runtime_error_name, "one"), (_type_error_name, "two")]) +@background_task() +def test_notice_error_exception_instance_multiple_exceptions(): + """Test that notice_error reports the passed exception object even when a different exception is active.""" + try: + raise RuntimeError("one") + except RuntimeError as e: + exc1 = e # Reassign name to ensure scope isn't lost + + try: + raise TypeError("two") + except TypeError as exc2: + notice_error(exc1) + notice_error(exc2) + + +@validate_transaction_error_event_count(0) +@background_task() +def test_notice_error_exception_instance_no_traceback(): + """Test that notice_error does not report an exception if it has not been raised as it has no __traceback__""" + exc = RuntimeError("one") + notice_error(exc) # Try once with no active exception -@validate_transaction_errors(errors=_test_notice_error_custom_params, required_params=[("key", "value")]) +@validate_transaction_errors(errors=[(_runtime_error_name, "one")], required_params=[("key", "value")]) @background_task() def test_notice_error_custom_params(): try: @@ -75,10 +104,7 @@ def test_notice_error_custom_params(): notice_error(sys.exc_info(), attributes={"key": "value"}) -_test_notice_error_multiple_different_type = [(_runtime_error_name, "one"), (_type_error_name, "two")] - - -@validate_transaction_errors(errors=_test_notice_error_multiple_different_type) +@validate_transaction_errors(errors=[(_runtime_error_name, "one"), (_type_error_name, "two")]) @background_task() def test_notice_error_multiple_different_type(): try: @@ -92,10 +118,7 @@ def test_notice_error_multiple_different_type(): notice_error() -_test_notice_error_multiple_same_type = [(_runtime_error_name, "one"), (_runtime_error_name, "two")] - - -@validate_transaction_errors(errors=_test_notice_error_multiple_same_type) +@validate_transaction_errors(errors=[(_runtime_error_name, "one"), (_runtime_error_name, "two")]) @background_task() def test_notice_error_multiple_same_type(): try: @@ -111,11 +134,9 @@ def test_notice_error_multiple_same_type(): # =============== Test errors outside a transaction =============== -_test_application_exception = [(_runtime_error_name, "one")] - @reset_core_stats_engine() -@validate_application_errors(errors=_test_application_exception) +@validate_application_errors(errors=[(_runtime_error_name, "one")]) def test_application_exception(): try: raise RuntimeError("one") @@ -124,11 +145,8 @@ def test_application_exception(): notice_error(application=application_instance) -_test_application_exception_sys_exc_info = [(_runtime_error_name, "one")] - - @reset_core_stats_engine() -@validate_application_errors(errors=_test_application_exception_sys_exc_info) +@validate_application_errors(errors=[(_runtime_error_name, "one")]) def test_application_exception_sys_exec_info(): try: raise RuntimeError("one") @@ -137,11 +155,8 @@ def test_application_exception_sys_exec_info(): notice_error(sys.exc_info(), application=application_instance) -_test_application_exception_custom_params = [(_runtime_error_name, "one")] - - @reset_core_stats_engine() -@validate_application_errors(errors=_test_application_exception_custom_params, required_params=[("key", "value")]) +@validate_application_errors(errors=[(_runtime_error_name, "one")], required_params=[("key", "value")]) def test_application_exception_custom_params(): try: raise RuntimeError("one") @@ -150,11 +165,8 @@ def test_application_exception_custom_params(): notice_error(attributes={"key": "value"}, application=application_instance) -_test_application_exception_multiple = [(_runtime_error_name, "one"), (_runtime_error_name, "one")] - - @reset_core_stats_engine() -@validate_application_errors(errors=_test_application_exception_multiple) +@validate_application_errors(errors=[(_runtime_error_name, "one"), (_runtime_error_name, "one")]) @background_task() def test_application_exception_multiple(): """Exceptions submitted straight to the stats engine doesn't check for @@ -174,12 +186,11 @@ def test_application_exception_multiple(): # =============== Test exception message stripping/allowlisting =============== -_test_notice_error_strip_message_disabled = [(_runtime_error_name, "one")] _strip_message_disabled_settings = {"strip_exception_messages.enabled": False} -@validate_transaction_errors(errors=_test_notice_error_strip_message_disabled) +@validate_transaction_errors(errors=[(_runtime_error_name, "one")]) @override_application_settings(_strip_message_disabled_settings) @background_task() def test_notice_error_strip_message_disabled(): @@ -215,12 +226,10 @@ def test_notice_error_strip_message_disabled_outside_transaction(): assert my_error.message == ErrorOne.message -_test_notice_error_strip_message_enabled = [(_runtime_error_name, STRIP_EXCEPTION_MESSAGE)] - _strip_message_enabled_settings = {"strip_exception_messages.enabled": True} -@validate_transaction_errors(errors=_test_notice_error_strip_message_enabled) +@validate_transaction_errors(errors=[(_runtime_error_name, STRIP_EXCEPTION_MESSAGE)]) @override_application_settings(_strip_message_enabled_settings) @background_task() def test_notice_error_strip_message_enabled(): @@ -256,15 +265,13 @@ def test_notice_error_strip_message_enabled_outside_transaction(): assert my_error.message == STRIP_EXCEPTION_MESSAGE -_test_notice_error_strip_message_in_allowlist = [(_runtime_error_name, "original error message")] - _strip_message_in_allowlist_settings = { "strip_exception_messages.enabled": True, "strip_exception_messages.allowlist": [_runtime_error_name], } -@validate_transaction_errors(errors=_test_notice_error_strip_message_in_allowlist) +@validate_transaction_errors(errors=[(_runtime_error_name, "original error message")]) @override_application_settings(_strip_message_in_allowlist_settings) @background_task() def test_notice_error_strip_message_in_allowlist(): @@ -307,15 +314,13 @@ def test_notice_error_strip_message_in_allowlist_outside_transaction(): assert my_error.message == ErrorThree.message -_test_notice_error_strip_message_not_in_allowlist = [(_runtime_error_name, STRIP_EXCEPTION_MESSAGE)] - _strip_message_not_in_allowlist_settings = { "strip_exception_messages.enabled": True, "strip_exception_messages.allowlist": ["FooError", "BarError"], } -@validate_transaction_errors(errors=_test_notice_error_strip_message_not_in_allowlist) +@validate_transaction_errors(errors=[(_runtime_error_name, STRIP_EXCEPTION_MESSAGE)]) @override_application_settings(_strip_message_not_in_allowlist_settings) @background_task() def test_notice_error_strip_message_not_in_allowlist():