fix(agent): improve exception handler instrumentation for PHPs 8.0+ (#…

…877) When user exception handler is installed it can call restore_exception_handler, and that will reset is_exception_handler flag in the wraprec which will cause the agent not to use the correct code path after user exception handler returns. The information that nr_php_instrument_func_end is handling return from user exception handler needs to be stored in the segment associated with the user exception handler. --------- Co-authored-by: ZNeumann <[email protected]>
newrelic · Apr 19, 2024 · eecccd5 · eecccd5
1 parent e6959a4
commit eecccd5
Show file tree

Hide file tree

Showing 8 changed files with 1,069 additions and 1 deletion.
diff --git a/agent/php_execute.c b/agent/php_execute.c
@@ -1946,6 +1946,14 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) {
   if (NULL == wraprec) {
     return;
   }
+
+  /* Store information that the segment is exception handler segment directly in
+   * the segment, because exception handler can call restore_exception_handler,
+   * and that will reset is_exception_handler flag in the wraprec */
+  if (wraprec->is_exception_handler) {
+    segment->is_exception_handler = 1;
+  }
+
   /*
    * If a function needs to have arguments modified, do so in
    * nr_zend_call_oapi_special_before.
@@ -2027,7 +2035,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) {
 
   wraprec = segment->wraprec;
 
-  if (wraprec && wraprec->is_exception_handler) {
+  if (segment->is_exception_handler) {
     /*
      * After running the exception handler segment, create an error from
      * the exception it handled, and save the error in the transaction.

diff --git a/axiom/nr_segment.h b/axiom/nr_segment.h
@@ -192,6 +192,8 @@ typedef struct _nr_segment_t {
    */
   void* wraprec; /* wraprec, if one is associated with this segment, to reduce
                     wraprec lookups */
+  int is_exception_handler; /* 1 if segment is associated with exception
+                               handler, 0 otherwise */
 #endif
 
 } nr_segment_t;

diff --git a/tests/integration/errors/test_uncaught_handled_exception_01.php b/tests/integration/errors/test_uncaught_handled_exception_01.php
@@ -0,0 +1,170 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+When a user exception handler unregisters itself as an exception handler when it handles uncaught exception,
+the agent should record the error and add error attributes on all spans leading to uncaught exception as
+well as the one throwing the exception. Error attributtes are not expected on the root span (because
+the exception has been handled) as well as on the span created for exception handler.
+*/
+
+/*INI
+newrelic.distributed_tracing_enabled=1
+newrelic.transaction_tracer.threshold = 0
+newrelic.span_events_enabled=1
+newrelic.code_level_metrics.enabled = 0
+display_errors=1
+log_errors=0
+*/
+
+
+/*SKIPIF
+<?php
+if (version_compare(PHP_VERSION, "8.0", "<")) {
+  die("skip: PHP < 8.0.0 not supported\n");
+}
+*/
+
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "RuntimeException",
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      "??"
+    ]
+  ]
+]
+*/
+
+/*EXPECT_SPAN_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": 10000,
+    "events_seen": 4
+  },
+  [
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "OtherTransaction\/php__FILE__",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "nr.entryPoint": true,
+        "transaction.name": "OtherTransaction\/php__FILE__"
+      },
+      {},
+      {}
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom\/call_throw_it",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "error.class": "RuntimeException"
+      }
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom\/throw_it",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "error.class": "RuntimeException"
+      }
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom\/user_exception_handler",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+
+/*EXPECT_REGEX
+Handled uncaught exception
+*/
+
+
+function user_exception_handler(Throwable $ex) {
+  restore_exception_handler();
+  echo "Handled uncaught exception";
+}
+
+function throw_it() {
+  throw new RuntimeException('Expected unexpected happened');
+}
+
+function call_throw_it() {
+  throw_it();
+}
+
+set_exception_handler('user_exception_handler');
+
+call_throw_it();
diff --git a/tests/integration/errors/test_uncaught_handled_exception_02.php b/tests/integration/errors/test_uncaught_handled_exception_02.php
@@ -0,0 +1,168 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+When a user exception handler unregisters itself as an exception handler when it handles uncaught exception,
+the agent should record the error and add error attributes on all spans leading to uncaught exception as
+well as the one throwing the exception. Error attributtes are not expected on the root span (because
+the exception has been handled) as well as on the span created for exception handler.
+*/
+
+/*INI
+newrelic.distributed_tracing_enabled=1
+newrelic.transaction_tracer.threshold = 0
+newrelic.span_events_enabled=1
+newrelic.code_level_metrics.enabled = 0
+display_errors=1
+log_errors=0
+*/
+
+
+/*SKIPIF
+<?php
+if (version_compare(PHP_VERSION, "8.0", "<")) {
+  die("skip: PHP < 8.0.0 not supported\n");
+}
+*/
+
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "RuntimeException",
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      "??"
+    ]
+  ]
+]
+*/
+
+/*EXPECT_SPAN_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": 10000,
+    "events_seen": 4
+  },
+  [
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "OtherTransaction\/php__FILE__",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "nr.entryPoint": true,
+        "transaction.name": "OtherTransaction\/php__FILE__"
+      },
+      {},
+      {}
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom\/call_throw_it",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "error.class": "RuntimeException"
+      }
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom\/throw_it",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {
+        "error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
+        "error.class": "RuntimeException"
+      }
+    ],
+    [
+      {
+        "category": "generic",
+        "type": "Span",
+        "guid": "??",
+        "traceId": "??",
+        "transactionId": "??",
+        "name": "Custom/{closure}",
+        "timestamp": "??",
+        "duration": "??",
+        "priority": "??",
+        "sampled": true,
+        "parentId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+
+/*EXPECT_REGEX
+Handled uncaught exception
+*/
+
+
+function throw_it() {
+  throw new RuntimeException('Expected unexpected happened');
+}
+
+function call_throw_it() {
+  throw_it();
+}
+
+set_exception_handler(function (Throwable $ex) {
+  restore_exception_handler();
+  echo "Handled uncaught exception";
+});
+
+call_throw_it();