fix(agent): Fixes newrelic_notice_error() API for PHP 8+

agent/php_api.c

@@ -58,10 +58,30 @@ void nr_php_api_add_supportability_metric(const char* name TSRMLS_DC) {
 
 /*
  * Purpose : (New Relic API) Pretend that there is an error at this exact spot.
- *           Useful for business logic errors. newrelic_notice_error($errstr)
+ *           Useful for business logic errors. 
+ *              - newrelic_notice_error($errstr)
+ *                   - $errstr : string : The error message to record
  *              - newrelic_notice_error($exception)
+ *                   - $exception : object : The exception to use to record the exception
+ *                  NOTE: This version is compatible with being a callback for set_exception_handler()
  *              - newrelic_notice_error($errstr,$exception)
+ *                   - $errstr : string : The error message to record
+ *                   - $exception : object : The exception to use to record the exception 
+ *                  NOTE: The $errstr value is ignored!  Started with agent version 4.23
+ *              - newrelic_notice_error($errno,$errstr,$fname,$line_nr)
+ *                   - $errno : int : The error number
+ *                   - $errstr : string : The error message
+ *                   - $fname : string : The filename where the error occurred
+ *                   - $line_nr : int : The line number where the error occurred
+ *                  NOTE: This version is compatible with being a callback for set_error_handler() for PHP 8+
  *              - newrelic_notice_error($errno,$errstr,$fname,$line_nr,$ctx)
+ *                   - $errno : int : The error number
+ *                   - $errstr : string : The error message
+ *                   - $fname : string : The filename where the error occurred
+ *                   - $line_nr : int : The line number where the error occurred
+ *                   - $ctx : array : The context of the error  
+ *                  NOTE: This version is compatible with being a callback for set_error_handler() for PHP < 8
+ *                        The $ctx is ignored!
  */
 #ifdef TAGS
 void zif_newrelic_notice_error(void); /* ctags landing pad only */
@@ -141,10 +161,15 @@ PHP_FUNCTION(newrelic_notice_error) {
       }
       break;
 
+    case 4:
     case 5:
+      /* PHP 8+ will only pass the first 4 parameters so the 5th parameter is 
+       * declared to be optional.  Also this parameter is completely ignored
+       * so it doesn't matter if it is passed or not.
+       */
       if (FAILURE
           == zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET,
-                                      ZEND_NUM_ARGS() TSRMLS_CC, "lsslz!",
+                                      ZEND_NUM_ARGS() TSRMLS_CC, "lssl|z!",
                                       &ignore1, &errormsgstr, &errormsglen,
                                       &ignore2, &ignore3, &ignore4, &ignore5)) {
         nrl_debug(NRL_API, "newrelic_notice_error: invalid five arguments");
@@ -153,7 +178,7 @@ PHP_FUNCTION(newrelic_notice_error) {
       break;
 
     default:
-      nrl_debug(NRL_API, "newrelic_notice_error: invalid number of arguments");
+      nrl_debug(NRL_API, "newrelic_notice_error: invalid number of arguments: %d", ZEND_NUM_ARGS());
       RETURN_NULL();
   }
 

tests/integration/api/notice_error/test_bad_inputs.php

@@ -15,6 +15,9 @@
 ok - 2 args
 ok - 3 args
 ok - 4 args
+ok - 4 args
+ok - 4 args
+ok - 4 args
 ok - 5 args
 ok - 6 args
 */
@@ -32,9 +35,15 @@
 tap_equal(null, newrelic_notice_error("", 42), "2 args");
 tap_equal(null, newrelic_notice_error("", array()), "2 args");
 
-// Three and four argument forms are not allowed.
+// Three argument forms are not allowed.
 tap_equal(null, newrelic_notice_error(42, "message", "file"), "3 args");
-tap_equal(null, newrelic_notice_error(42, "message", "file", __LINE__), "4 args");
+
+// Four argument form requires integer, string, string, integer
+// This is like the five argument form but for PHP 8+ where the context is not supplied
+tap_equal(null, newrelic_notice_error("", "message", "file", __LINE__), "4 args");
+tap_equal(null, newrelic_notice_error(42, array(), "file", __LINE__), "4 args");
+tap_equal(null, newrelic_notice_error("", "message", array(), __LINE__), "4 args");
+tap_equal(null, newrelic_notice_error("", "message", "file", ""), "4 args");
 
 // Five argument form requires second arg to be convertible to a string.
 tap_equal(null, newrelic_notice_error("", curl_init()), "5 args");

tests/integration/api/notice_error/test_good_1_arg_exception.php

@@ -0,0 +1,133 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+The agent should record a traced error when newrelic_notice_error is
+called with 1 argument which is an exception.
+*/
+
+/*EXPECT_TRACED_ERRORS
+[
+  "?? agent run id",
+  [
+    [
+      "?? when",
+      "OtherTransaction/php__FILE__",
+      "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+      "Exception",
+      {
+        "stack_trace": [
+          " in a called at __FILE__ (??)"
+     ],
+        "agentAttributes": "??",
+        "intrinsics": "??"
+      },
+      "?? transaction ID"
+    ]
+  ]
+]
+*/
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "Exception",
+        "error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+/*EXPECT_SPAN_EVENTS_LIKE
+[
+  [
+    {
+      "traceId": "??",
+      "duration": "??",
+      "transactionId": "??",
+      "name": "Custom\/a",
+      "guid": "??",
+      "type": "Span",
+      "category": "generic",
+      "priority": "??",
+      "sampled": true,
+      "timestamp": "??",
+      "parentId": "??"
+    },
+    {},
+    {
+      "error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+      "error.class": "Exception",
+      "code.lineno": "??",
+      "code.filepath": "__FILE__",
+      "code.function": "a"
+    }
+  ]
+]
+*/
+
+/*EXPECT_ANALYTICS_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": "??"
+  },
+  [
+    [
+      {
+        "type": "Transaction",
+        "name": "OtherTransaction\/php__FILE__",
+        "timestamp": "??",
+        "duration": "??",
+        "totalTime": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "error": true
+      },
+      {},
+      {
+        "errorType": "Exception",
+        "errorMessage": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??"
+      }
+    ]
+  ]
+]
+*/
+
+require_once(realpath(dirname(__FILE__)) . '/../../../include/tap.php');
+
+function a()
+{
+  // One argument with an exception - this is tricky because most
+  // values are implicitly convertible to strings. Use a resource to prevent this.
+  newrelic_notice_error("1 arg", new Exception("1 arg exception"));
+}
+
+
+a();

tests/integration/api/notice_error/test_good_1_arg_exception_handler.php

@@ -0,0 +1,150 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+The agent should record a traced error when newrelic_notice_error is
+used as a callback handler for set_exception_handler().
+*/
+
+/*EXPECT_TRACED_ERRORS
+[
+  "?? agent run id",
+  [
+    [
+      "?? when",
+      "OtherTransaction/php__FILE__",
+      "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+      "Exception",
+      {
+        "stack_trace": [
+          " in a called at __FILE__ (??)"
+     ],
+        "agentAttributes": "??",
+        "intrinsics": "??"
+      },
+      "?? transaction ID"
+    ]
+  ]
+]
+*/
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "Exception",
+        "error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+/*EXPECT_SPAN_EVENTS_LIKE
+[
+  [
+    {
+      "traceId": "??",
+      "duration": "??",
+      "transactionId": "??",
+      "name": "OtherTransaction\/php__FILE__",
+      "guid": "??",
+      "type": "Span",
+      "category": "generic",
+      "priority": "??",
+      "sampled": true,
+      "timestamp": "??",
+      "nr.entryPoint": true,
+      "transaction.name": "OtherTransaction\/php__FILE__"
+    },
+    {},
+    {
+      "error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
+      "error.class": "Exception"
+    }
+  ],
+  [
+    {
+      "traceId": "??",
+      "duration": "??",
+      "transactionId": "??",
+      "name": "Custom\/a",
+      "guid": "??",
+      "type": "Span",
+      "category": "generic",
+      "priority": "??",
+      "sampled": true,
+      "timestamp": "??",
+      "parentId": "??"
+    },
+    {},
+    {
+      "error.message": "Uncaught exception 'Exception' with message '1 arg exception' in __FILE__:??",
+      "error.class": "Exception",
+      "code.lineno": "??",
+      "code.filepath": "__FILE__",
+      "code.function": "a"
+    }
+  ]
+]
+*/
+
+/*EXPECT_ANALYTICS_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": "??"
+  },
+  [
+    [
+      {
+        "type": "Transaction",
+        "name": "OtherTransaction\/php__FILE__",
+        "timestamp": "??",
+        "duration": "??",
+        "totalTime": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "error": true
+      },
+      {},
+      {
+        "errorType": "Exception",
+        "errorMessage": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??"
+      }
+    ]
+  ]
+]
+*/
+
+function a()
+{
+  throw new Exception("1 arg exception");
+}
+
+set_exception_handler('newrelic_notice_error');
+a();