From 7d8b17412e9771e1e3062465bfcd84d8b7470429 Mon Sep 17 00:00:00 2001
From: Amber Sistla <asistla@newrelic.com>
Date: Thu, 11 Apr 2024 14:42:04 -0700
Subject: [PATCH] fix(agent): Fix error reporting when EH_THROW is enabled   /*
    * For a the following error codes:    * E_WARNING || E_CORE_WARNING ||
 E_COMPILE_WARNING || E_USER_WARNING    * PHP triggers an exception if
 EH_THROW is toggled on and then immediately    * returns after throwing the
 exception. See for more info:    *
 https://github.com/php/php-src/blob/master/main/main.c In that case, we    *
 should not handle it, but we should exist and let the exception handler    *
 deal with it; otherwise, we could record an error even if an exception is   
 * caught.    */

---
 agent/php_error.c                             |  20 ++-
 .../test_EH_THROW_errors_caught_exception.php |  76 +++++++++++
 ...est_EH_THROW_errors_uncaught_exception.php | 119 ++++++++++++++++++
 3 files changed, 213 insertions(+), 2 deletions(-)
 create mode 100644 tests/integration/errors/test_EH_THROW_errors_caught_exception.php
 create mode 100644 tests/integration/errors/test_EH_THROW_errors_uncaught_exception.php

diff --git a/agent/php_error.c b/agent/php_error.c
index 59956d585..b4f6f68b1 100644
--- a/agent/php_error.c
+++ b/agent/php_error.c
@@ -561,12 +561,29 @@ static int nr_php_should_record_error(int type, const char* format TSRMLS_DC) {
     return 0;
   }
 
+  /*
+   * For a the following error codes:
+   * E_WARNING || E_CORE_WARNING || E_COMPILE_WARNING || E_USER_WARNING
+   * PHP triggers an exception if EH_THROW is toggled on and then immediately
+   * returns after throwing the exception. See for more info:
+   * https://github.com/php/php-src/blob/master/main/main.c In that case, we
+   * should not handle it, but we should exist and let the exception handler
+   * deal with it; otherwise, we could record an error even if an exception is
+   * caught.
+   */
+  if (EG(error_handling) == EH_THROW) {
+    if ((E_WARNING == type) || (E_CORE_WARNING == type)
+        || (E_COMPILE_WARNING == type) || (E_USER_WARNING == type)) {
+      return 0;
+    }
+  }
+
   errprio = nr_php_error_get_priority(type);
 
   if (0 == errprio) {
     return 0;
   }
-
+  
   if (NR_SUCCESS != nr_txn_record_error_worthy(NRPRG(txn), errprio)) {
     return 0;
   }
@@ -625,7 +642,6 @@ void nr_php_error_cb(int type,
 
     stack_json = nr_php_backtrace_to_json(0 TSRMLS_CC);
     errclass = nr_php_error_get_type_string(type);
-
     nr_txn_record_error(NRPRG(txn), nr_php_error_get_priority(type), true,
                         msg, errclass, stack_json);
 
diff --git a/tests/integration/errors/test_EH_THROW_errors_caught_exception.php b/tests/integration/errors/test_EH_THROW_errors_caught_exception.php
new file mode 100644
index 000000000..01a779379
--- /dev/null
+++ b/tests/integration/errors/test_EH_THROW_errors_caught_exception.php
@@ -0,0 +1,76 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+For a certain number of error codes, PHP triggers an exception if EH_THROW is toggled on.
+Verify we don't record the error if the exception is thrown and caught.
+There should be no traced errors, error events, or errors attached to span events.
+*/
+
+/*SKIPIF
+<?php
+if (version_compare(PHP_VERSION, "7.0", "<")) {
+  die("skip: PHP < 7.0 not supported\n");
+}
+*/
+
+/*INI
+display_errors=1
+log_errors=0
+error_reporting=E_ALL
+*/
+
+/*EXPECT_SPAN_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": 10000,
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "traceId": "??",
+        "duration": "??",
+        "transactionId": "??",
+        "name": "OtherTransaction\/php__FILE__",
+        "guid": "??",
+        "type": "Span",
+        "category": "generic",
+        "priority": "??",
+        "sampled": true,
+        "nr.entryPoint": true,
+        "timestamp": "??",
+        "transaction.name": "OtherTransaction\/php__FILE__"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+
+/*EXPECT_REGEX
+Exception caught*
+*/
+
+/*EXPECT_TRACED_ERRORS
+null
+*/
+
+/*EXPECT_ERROR_EVENTS
+null
+*/
+
+$base_directory = __DIR__ . '/nonexist';;
+
+try {
+       $n = new RecursiveDirectoryIterator( $base_directory ) ;
+}
+catch (\Exception $e) {
+       echo ("Exception caught: " . $e->getMessage());
+}
diff --git a/tests/integration/errors/test_EH_THROW_errors_uncaught_exception.php b/tests/integration/errors/test_EH_THROW_errors_uncaught_exception.php
new file mode 100644
index 000000000..79fe89087
--- /dev/null
+++ b/tests/integration/errors/test_EH_THROW_errors_uncaught_exception.php
@@ -0,0 +1,119 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+For a certain number of error codes, PHP triggers an exception if EH_THROW is toggled on.
+Verify we don't record the error if the exception is thrown and NOT caught.
+There should be  traced errors, error events, and an error attached to span events.
+*/
+
+/*SKIPIF
+<?php
+if (version_compare(PHP_VERSION, "7.0", "<")) {
+  die("skip: PHP < 7.0 not supported\n");
+}
+*/
+
+/*INI
+display_errors=1
+log_errors=0
+error_reporting=E_ALL
+*/
+
+/*EXPECT_SPAN_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": 10000,
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "traceId": "??",
+        "duration": "??",
+        "transactionId": "??",
+        "name": "OtherTransaction\/php__FILE__",
+        "guid": "??",
+        "type": "Span",
+        "category": "generic",
+        "priority": "??",
+        "sampled": true,
+        "nr.entryPoint": true,
+        "timestamp": "??",
+        "transaction.name": "OtherTransaction\/php__FILE__"
+      },
+      {},
+      {
+        "error.message": "?? Uncaught exception ??",
+        "error.class": "UnexpectedValueException"
+      }
+    ]
+  ]
+]
+*/
+
+
+
+/*EXPECT_REGEX
+Fatal error*
+*/
+
+/*EXPECT_TRACED_ERRORS
+[
+  "?? agent run id",
+  [
+    [
+      "?? when",
+      "OtherTransaction/php__FILE__",
+      "?? Uncaught exception ??",
+      "UnexpectedValueException",
+      {
+        "stack_trace": "??",
+        "agentAttributes": "??",
+        "intrinsics": "??"
+      },
+      "?? transaction ID"
+    ]
+  ]
+]
+*/
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "UnexpectedValueException",
+        "error.message": "?? Uncaught exception ??",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+$base_directory = __DIR__ . '/nonexist';;
+
+$n = new RecursiveDirectoryIterator( $base_directory );
+
+echo("Should not get here");
\ No newline at end of file