From 955697b1aad5f40fc74c478c7e086887bfcee33e Mon Sep 17 00:00:00 2001 From: Brian Duranleau Date: Wed, 11 Dec 2024 16:55:49 -0600 Subject: [PATCH 1/5] fix(agent): fix Drupal error and exception handling --- agent/fw_drupal8.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 541cb8b87..278c298e1 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -9,6 +9,7 @@ #include "php_user_instrument.h" #include "php_execute.h" #include "php_wrapper.h" +#include "php_error.h" #include "fw_drupal_common.h" #include "fw_hooks.h" #include "fw_support.h" @@ -20,6 +21,60 @@ #define PHP_PACKAGE_NAME "drupal/core" +NR_PHP_WRAPPER(nr_drupal_exception) { + int priority = nr_php_error_get_priority(E_ERROR); + zval* event = NULL; + zval* exception = NULL; + + /* Warning avoidance */ + (void)wraprec; + + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL8); + + if (NR_SUCCESS != nr_txn_record_error_worthy(NRPRG(txn), priority)) { + NR_PHP_WRAPPER_CALL; + goto end; + } + + /* Get the event that was given. */ + event = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS TSRMLS_CC); + + /* Call the original function. */ + NR_PHP_WRAPPER_CALL; + + if (0 == nr_php_is_zval_valid_object(event)) { + nrl_verbosedebug(NRL_TXN, + "Drupal: ExceptionSubscriber::onException() does not " + "have an `event` parameter"); + goto end; + } + + /* + * Get the exception from the event. + */ + exception = nr_php_call(event, "getThrowable"); + if (!nr_php_is_zval_valid_object(exception)) { + exception = nr_php_call(event, "getException"); + } + + if (!nr_php_is_zval_valid_object(exception)) { + nrl_verbosedebug(NRL_TXN, "Drupal: getException() returned a non-object"); + goto end; + } + + if (NR_SUCCESS + != nr_php_error_record_exception(NRPRG(txn), exception, priority, true, + NULL, + &NRPRG(exception_filters) TSRMLS_CC)) { + nrl_verbosedebug(NRL_TXN, "Drupal: unable to record exception"); + } + +end: + nr_php_arg_release(&event); + nr_php_zval_free(&exception); +} +NR_PHP_WRAPPER_END + /* * Purpose : Convenience function to handle adding a callback to a method, * given a class entry and a method name. This will check the @@ -730,6 +785,32 @@ void nr_drupal8_enable(TSRMLS_D) { "er::getControllerFromDefinition"), nr_drupal8_name_the_wt TSRMLS_CC); + /* + * ExceptionSubscribers handle Drupal errors and exceptions before + * the agent has the opportunity to capture them. Instrument several + * of these ExceptionSubscriber function `onException` methods in order + * to capture Exceptions and Errors in Drupal 9.x+ + */ + // clang-format off + /* + * Utility base class for exception subscribers. + */ + nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\HttpExceptionSubscriberBase::onException"), + nr_drupal_exception); + + /* + * Log exceptions without further handling. + */ + nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\ExceptionLoggingSubscriber::onException"), + nr_drupal_exception); + + /* + * Last-chance handler for exceptions: the final exception subscriber. + */ + nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\FinalExceptionSubscriber::onException"), + nr_drupal_exception); + // clang-format on + /* * The drupal_modules config setting controls instrumentation of modules, * hooks, and views. From dfd1392e2aff9dd21ac3f0bb1ee50051f07fb14a Mon Sep 17 00:00:00 2001 From: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:31:12 -0600 Subject: [PATCH 2/5] Update agent/fw_drupal8.c Co-authored-by: Michal Nowacki --- agent/fw_drupal8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 278c298e1..f928d9507 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -65,7 +65,7 @@ NR_PHP_WRAPPER(nr_drupal_exception) { if (NR_SUCCESS != nr_php_error_record_exception(NRPRG(txn), exception, priority, true, NULL, - &NRPRG(exception_filters) TSRMLS_CC)) { + &NRPRG(exception_filters))) { nrl_verbosedebug(NRL_TXN, "Drupal: unable to record exception"); } From 47a930d1f11fd18ba5f20ff73feb56dca72ad2a6 Mon Sep 17 00:00:00 2001 From: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:31:21 -0600 Subject: [PATCH 3/5] Update agent/fw_drupal8.c Co-authored-by: Michal Nowacki --- agent/fw_drupal8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index f928d9507..a0d7cf0c4 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -37,7 +37,7 @@ NR_PHP_WRAPPER(nr_drupal_exception) { } /* Get the event that was given. */ - event = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS TSRMLS_CC); + event = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS); /* Call the original function. */ NR_PHP_WRAPPER_CALL; From 7ab64fba3ea58c5504284e08f5b55e5464049fd2 Mon Sep 17 00:00:00 2001 From: Brian Duranleau Date: Tue, 17 Dec 2024 15:06:41 -0600 Subject: [PATCH 4/5] fix: remove HttpExceptionSubscriber wrapper --- agent/fw_drupal8.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index a0d7cf0c4..5b4a3b6af 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -792,12 +792,6 @@ void nr_drupal8_enable(TSRMLS_D) { * to capture Exceptions and Errors in Drupal 9.x+ */ // clang-format off - /* - * Utility base class for exception subscribers. - */ - nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\HttpExceptionSubscriberBase::onException"), - nr_drupal_exception); - /* * Log exceptions without further handling. */ From 82ec0d73ea92f604463b1623b5e93f43499ef474 Mon Sep 17 00:00:00 2001 From: Brian Duranleau Date: Tue, 17 Dec 2024 15:17:22 -0600 Subject: [PATCH 5/5] fix: defensively free exception if getThrowable fails --- agent/fw_drupal8.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 5b4a3b6af..32976c64a 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -54,6 +54,8 @@ NR_PHP_WRAPPER(nr_drupal_exception) { */ exception = nr_php_call(event, "getThrowable"); if (!nr_php_is_zval_valid_object(exception)) { + // be abundantly cautious: free exception before attempting to re-assign + nr_php_zval_free(&exception); exception = nr_php_call(event, "getException"); }