Skip to content

Commit 8de09b0

Browse files
mfulbzsistlahahuja2
authored
fix(agent): Fixes newrelic_notice_error() API for PHP 8+ (#960)
Starting with PHP 8.0 the `set_error_handler()` callback function signature drops the `$errcontext` final parameter so it only contains 4 parameters. This caused using `newrelic_notice_error()` as a callback handler for PHP 8+ to not work as the API call did not accept only 4 arguments. This PR adds support for this function signature. There are also numerous tests added that will hopefully caught this kind of issue in the future. --------- Co-authored-by: Amber Sistla <[email protected]> Co-authored-by: Hitesh Ahuja <[email protected]>
1 parent 393ca93 commit 8de09b0

10 files changed

+1141
-5
lines changed

‎agent/php_api.c‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,30 @@ void nr_php_api_add_supportability_metric(const char* name TSRMLS_DC) {
5858

5959
/*
6060
* Purpose : (New Relic API) Pretend that there is an error at this exact spot.
61-
* Useful for business logic errors. newrelic_notice_error($errstr)
61+
* Useful for business logic errors.
62+
* - newrelic_notice_error($errstr)
63+
* - $errstr : string : The error message to record
6264
* - newrelic_notice_error($exception)
65+
* - $exception : object : The exception to use to record the exception
66+
* NOTE: This version is compatible with being a callback for set_exception_handler()
6367
* - newrelic_notice_error($errstr,$exception)
68+
* - $errstr : string : The error message to record
69+
* - $exception : object : The exception to use to record the exception
70+
* NOTE: The $errstr value is ignored! Started with agent version 4.23
71+
* - newrelic_notice_error($errno,$errstr,$fname,$line_nr)
72+
* - $errno : int : The error number
73+
* - $errstr : string : The error message
74+
* - $fname : string : The filename where the error occurred
75+
* - $line_nr : int : The line number where the error occurred
76+
* NOTE: This version is compatible with being a callback for set_error_handler() for PHP 8+
6477
* - newrelic_notice_error($errno,$errstr,$fname,$line_nr,$ctx)
78+
* - $errno : int : The error number
79+
* - $errstr : string : The error message
80+
* - $fname : string : The filename where the error occurred
81+
* - $line_nr : int : The line number where the error occurred
82+
* - $ctx : array : The context of the error
83+
* NOTE: This version is compatible with being a callback for set_error_handler() for PHP < 8
84+
* The $ctx is ignored!
6585
*/
6686
#ifdef TAGS
6787
void zif_newrelic_notice_error(void); /* ctags landing pad only */
@@ -141,10 +161,15 @@ PHP_FUNCTION(newrelic_notice_error) {
141161
}
142162
break;
143163

164+
case 4:
144165
case 5:
166+
/* PHP 8+ will only pass the first 4 parameters so the 5th parameter is
167+
* declared to be optional. Also this parameter is completely ignored
168+
* so it doesn't matter if it is passed or not.
169+
*/
145170
if (FAILURE
146171
== zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET,
147-
ZEND_NUM_ARGS() TSRMLS_CC, "lsslz!",
172+
ZEND_NUM_ARGS() TSRMLS_CC, "lssl|z!",
148173
&ignore1, &errormsgstr, &errormsglen,
149174
&ignore2, &ignore3, &ignore4, &ignore5)) {
150175
nrl_debug(NRL_API, "newrelic_notice_error: invalid five arguments");
@@ -153,7 +178,7 @@ PHP_FUNCTION(newrelic_notice_error) {
153178
break;
154179

155180
default:
156-
nrl_debug(NRL_API, "newrelic_notice_error: invalid number of arguments");
181+
nrl_debug(NRL_API, "newrelic_notice_error: invalid number of arguments: %d", ZEND_NUM_ARGS());
157182
RETURN_NULL();
158183
}
159184

‎tests/integration/api/notice_error/test_bad_inputs.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
ok - 2 args
1616
ok - 3 args
1717
ok - 4 args
18+
ok - 4 args
19+
ok - 4 args
20+
ok - 4 args
1821
ok - 5 args
1922
ok - 6 args
2023
*/
@@ -32,9 +35,15 @@
3235
tap_equal(null, newrelic_notice_error("", 42), "2 args");
3336
tap_equal(null, newrelic_notice_error("", array()), "2 args");
3437

35-
// Three and four argument forms are not allowed.
38+
// Three argument forms are not allowed.
3639
tap_equal(null, newrelic_notice_error(42, "message", "file"), "3 args");
37-
tap_equal(null, newrelic_notice_error(42, "message", "file", __LINE__), "4 args");
40+
41+
// Four argument form requires integer, string, string, integer
42+
// This is like the five argument form but for PHP 8+ where the context is not supplied
43+
tap_equal(null, newrelic_notice_error("", "message", "file", __LINE__), "4 args");
44+
tap_equal(null, newrelic_notice_error(42, array(), "file", __LINE__), "4 args");
45+
tap_equal(null, newrelic_notice_error("", "message", array(), __LINE__), "4 args");
46+
tap_equal(null, newrelic_notice_error("", "message", "file", ""), "4 args");
3847

3948
// Five argument form requires second arg to be convertible to a string.
4049
tap_equal(null, newrelic_notice_error("", curl_init()), "5 args");

‎tests/integration/api/notice_error/test_good_1_arg_exception.php‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
<?php
2+
/*
3+
* Copyright 2020 New Relic Corporation. All rights reserved.
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/*DESCRIPTION
8+
The agent should record a traced error when newrelic_notice_error is
9+
called with 1 argument which is an exception.
10+
*/
11+
12+
/*EXPECT_TRACED_ERRORS
13+
[
14+
"?? agent run id",
15+
[
16+
[
17+
"?? when",
18+
"OtherTransaction/php__FILE__",
19+
"Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
20+
"Exception",
21+
{
22+
"stack_trace": [
23+
" in a called at __FILE__ (??)"
24+
],
25+
"agentAttributes": "??",
26+
"intrinsics": "??"
27+
},
28+
"?? transaction ID"
29+
]
30+
]
31+
]
32+
*/
33+
34+
/*EXPECT_ERROR_EVENTS
35+
[
36+
"?? agent run id",
37+
{
38+
"reservoir_size": "??",
39+
"events_seen": 1
40+
},
41+
[
42+
[
43+
{
44+
"type": "TransactionError",
45+
"timestamp": "??",
46+
"error.class": "Exception",
47+
"error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
48+
"transactionName": "OtherTransaction\/php__FILE__",
49+
"duration": "??",
50+
"nr.transactionGuid": "??",
51+
"guid": "??",
52+
"sampled": true,
53+
"priority": "??",
54+
"traceId": "??",
55+
"spanId": "??"
56+
},
57+
{},
58+
{}
59+
]
60+
]
61+
]
62+
*/
63+
64+
/*EXPECT_SPAN_EVENTS_LIKE
65+
[
66+
[
67+
{
68+
"traceId": "??",
69+
"duration": "??",
70+
"transactionId": "??",
71+
"name": "Custom\/a",
72+
"guid": "??",
73+
"type": "Span",
74+
"category": "generic",
75+
"priority": "??",
76+
"sampled": true,
77+
"timestamp": "??",
78+
"parentId": "??"
79+
},
80+
{},
81+
{
82+
"error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
83+
"error.class": "Exception",
84+
"code.lineno": "??",
85+
"code.filepath": "__FILE__",
86+
"code.function": "a"
87+
}
88+
]
89+
]
90+
*/
91+
92+
/*EXPECT_ANALYTICS_EVENTS
93+
[
94+
"?? agent run id",
95+
{
96+
"reservoir_size": "??",
97+
"events_seen": "??"
98+
},
99+
[
100+
[
101+
{
102+
"type": "Transaction",
103+
"name": "OtherTransaction\/php__FILE__",
104+
"timestamp": "??",
105+
"duration": "??",
106+
"totalTime": "??",
107+
"guid": "??",
108+
"sampled": true,
109+
"priority": "??",
110+
"traceId": "??",
111+
"error": true
112+
},
113+
{},
114+
{
115+
"errorType": "Exception",
116+
"errorMessage": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??"
117+
}
118+
]
119+
]
120+
]
121+
*/
122+
123+
require_once(realpath(dirname(__FILE__)) . '/../../../include/tap.php');
124+
125+
function a()
126+
{
127+
// One argument with an exception - this is tricky because most
128+
// values are implicitly convertible to strings. Use a resource to prevent this.
129+
newrelic_notice_error("1 arg", new Exception("1 arg exception"));
130+
}
131+
132+
133+
a();

‎tests/integration/api/notice_error/test_good_1_arg_exception_handler.php‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
<?php
2+
/*
3+
* Copyright 2020 New Relic Corporation. All rights reserved.
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/*DESCRIPTION
8+
The agent should record a traced error when newrelic_notice_error is
9+
used as a callback handler for set_exception_handler().
10+
*/
11+
12+
/*EXPECT_TRACED_ERRORS
13+
[
14+
"?? agent run id",
15+
[
16+
[
17+
"?? when",
18+
"OtherTransaction/php__FILE__",
19+
"Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
20+
"Exception",
21+
{
22+
"stack_trace": [
23+
" in a called at __FILE__ (??)"
24+
],
25+
"agentAttributes": "??",
26+
"intrinsics": "??"
27+
},
28+
"?? transaction ID"
29+
]
30+
]
31+
]
32+
*/
33+
34+
/*EXPECT_ERROR_EVENTS
35+
[
36+
"?? agent run id",
37+
{
38+
"reservoir_size": "??",
39+
"events_seen": 1
40+
},
41+
[
42+
[
43+
{
44+
"type": "TransactionError",
45+
"timestamp": "??",
46+
"error.class": "Exception",
47+
"error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
48+
"transactionName": "OtherTransaction\/php__FILE__",
49+
"duration": "??",
50+
"nr.transactionGuid": "??",
51+
"guid": "??",
52+
"sampled": true,
53+
"priority": "??",
54+
"traceId": "??",
55+
"spanId": "??"
56+
},
57+
{},
58+
{}
59+
]
60+
]
61+
]
62+
*/
63+
64+
/*EXPECT_SPAN_EVENTS_LIKE
65+
[
66+
[
67+
{
68+
"traceId": "??",
69+
"duration": "??",
70+
"transactionId": "??",
71+
"name": "OtherTransaction\/php__FILE__",
72+
"guid": "??",
73+
"type": "Span",
74+
"category": "generic",
75+
"priority": "??",
76+
"sampled": true,
77+
"timestamp": "??",
78+
"nr.entryPoint": true,
79+
"transaction.name": "OtherTransaction\/php__FILE__"
80+
},
81+
{},
82+
{
83+
"error.message": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??",
84+
"error.class": "Exception"
85+
}
86+
],
87+
[
88+
{
89+
"traceId": "??",
90+
"duration": "??",
91+
"transactionId": "??",
92+
"name": "Custom\/a",
93+
"guid": "??",
94+
"type": "Span",
95+
"category": "generic",
96+
"priority": "??",
97+
"sampled": true,
98+
"timestamp": "??",
99+
"parentId": "??"
100+
},
101+
{},
102+
{
103+
"error.message": "Uncaught exception 'Exception' with message '1 arg exception' in __FILE__:??",
104+
"error.class": "Exception",
105+
"code.lineno": "??",
106+
"code.filepath": "__FILE__",
107+
"code.function": "a"
108+
}
109+
]
110+
]
111+
*/
112+
113+
/*EXPECT_ANALYTICS_EVENTS
114+
[
115+
"?? agent run id",
116+
{
117+
"reservoir_size": "??",
118+
"events_seen": "??"
119+
},
120+
[
121+
[
122+
{
123+
"type": "Transaction",
124+
"name": "OtherTransaction\/php__FILE__",
125+
"timestamp": "??",
126+
"duration": "??",
127+
"totalTime": "??",
128+
"guid": "??",
129+
"sampled": true,
130+
"priority": "??",
131+
"traceId": "??",
132+
"error": true
133+
},
134+
{},
135+
{
136+
"errorType": "Exception",
137+
"errorMessage": "Noticed exception 'Exception' with message '1 arg exception' in __FILE__:??"
138+
}
139+
]
140+
]
141+
]
142+
*/
143+
144+
function a()
145+
{
146+
throw new Exception("1 arg exception");
147+
}
148+
149+
set_exception_handler('newrelic_notice_error');
150+
a();

0 commit comments

Comments
 (0)