Skip to content

Commit eecccd5

Browse files
lavarouZNeumann
lavarou
and
ZNeumann
authored
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]>
1 parent e6959a4 commit eecccd5

8 files changed

+1069
-1
lines changed

‎agent/php_execute.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1946,6 +1946,14 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) {
19461946
if (NULL == wraprec) {
19471947
return;
19481948
}
1949+
1950+
/* Store information that the segment is exception handler segment directly in
1951+
* the segment, because exception handler can call restore_exception_handler,
1952+
* and that will reset is_exception_handler flag in the wraprec */
1953+
if (wraprec->is_exception_handler) {
1954+
segment->is_exception_handler = 1;
1955+
}
1956+
19491957
/*
19501958
* If a function needs to have arguments modified, do so in
19511959
* nr_zend_call_oapi_special_before.
@@ -2027,7 +2035,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) {
20272035

20282036
wraprec = segment->wraprec;
20292037

2030-
if (wraprec && wraprec->is_exception_handler) {
2038+
if (segment->is_exception_handler) {
20312039
/*
20322040
* After running the exception handler segment, create an error from
20332041
* the exception it handled, and save the error in the transaction.

‎axiom/nr_segment.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ typedef struct _nr_segment_t {
192192
*/
193193
void* wraprec; /* wraprec, if one is associated with this segment, to reduce
194194
wraprec lookups */
195+
int is_exception_handler; /* 1 if segment is associated with exception
196+
handler, 0 otherwise */
195197
#endif
196198

197199
} nr_segment_t;

‎tests/integration/errors/test_uncaught_handled_exception_01.php

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
<?php
2+
/*
3+
* Copyright 2020 New Relic Corporation. All rights reserved.
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/*DESCRIPTION
8+
When a user exception handler unregisters itself as an exception handler when it handles uncaught exception,
9+
the agent should record the error and add error attributes on all spans leading to uncaught exception as
10+
well as the one throwing the exception. Error attributtes are not expected on the root span (because
11+
the exception has been handled) as well as on the span created for exception handler.
12+
*/
13+
14+
/*INI
15+
newrelic.distributed_tracing_enabled=1
16+
newrelic.transaction_tracer.threshold = 0
17+
newrelic.span_events_enabled=1
18+
newrelic.code_level_metrics.enabled = 0
19+
display_errors=1
20+
log_errors=0
21+
*/
22+
23+
24+
/*SKIPIF
25+
<?php
26+
if (version_compare(PHP_VERSION, "8.0", "<")) {
27+
die("skip: PHP < 8.0.0 not supported\n");
28+
}
29+
*/
30+
31+
32+
/*EXPECT_ERROR_EVENTS
33+
[
34+
"?? agent run id",
35+
{
36+
"reservoir_size": "??",
37+
"events_seen": 1
38+
},
39+
[
40+
[
41+
{
42+
"type": "TransactionError",
43+
"timestamp": "??",
44+
"error.class": "RuntimeException",
45+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
46+
"transactionName": "OtherTransaction\/php__FILE__",
47+
"duration": "??",
48+
"nr.transactionGuid": "??",
49+
"guid": "??",
50+
"sampled": true,
51+
"priority": "??",
52+
"traceId": "??",
53+
"spanId": "??"
54+
},
55+
{},
56+
"??"
57+
]
58+
]
59+
]
60+
*/
61+
62+
/*EXPECT_SPAN_EVENTS
63+
[
64+
"?? agent run id",
65+
{
66+
"reservoir_size": 10000,
67+
"events_seen": 4
68+
},
69+
[
70+
[
71+
{
72+
"category": "generic",
73+
"type": "Span",
74+
"guid": "??",
75+
"traceId": "??",
76+
"transactionId": "??",
77+
"name": "OtherTransaction\/php__FILE__",
78+
"timestamp": "??",
79+
"duration": "??",
80+
"priority": "??",
81+
"sampled": true,
82+
"nr.entryPoint": true,
83+
"transaction.name": "OtherTransaction\/php__FILE__"
84+
},
85+
{},
86+
{}
87+
],
88+
[
89+
{
90+
"category": "generic",
91+
"type": "Span",
92+
"guid": "??",
93+
"traceId": "??",
94+
"transactionId": "??",
95+
"name": "Custom\/call_throw_it",
96+
"timestamp": "??",
97+
"duration": "??",
98+
"priority": "??",
99+
"sampled": true,
100+
"parentId": "??"
101+
},
102+
{},
103+
{
104+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
105+
"error.class": "RuntimeException"
106+
}
107+
],
108+
[
109+
{
110+
"category": "generic",
111+
"type": "Span",
112+
"guid": "??",
113+
"traceId": "??",
114+
"transactionId": "??",
115+
"name": "Custom\/throw_it",
116+
"timestamp": "??",
117+
"duration": "??",
118+
"priority": "??",
119+
"sampled": true,
120+
"parentId": "??"
121+
},
122+
{},
123+
{
124+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
125+
"error.class": "RuntimeException"
126+
}
127+
],
128+
[
129+
{
130+
"category": "generic",
131+
"type": "Span",
132+
"guid": "??",
133+
"traceId": "??",
134+
"transactionId": "??",
135+
"name": "Custom\/user_exception_handler",
136+
"timestamp": "??",
137+
"duration": "??",
138+
"priority": "??",
139+
"sampled": true,
140+
"parentId": "??"
141+
},
142+
{},
143+
{}
144+
]
145+
]
146+
]
147+
*/
148+
149+
150+
/*EXPECT_REGEX
151+
Handled uncaught exception
152+
*/
153+
154+
155+
function user_exception_handler(Throwable $ex) {
156+
restore_exception_handler();
157+
echo "Handled uncaught exception";
158+
}
159+
160+
function throw_it() {
161+
throw new RuntimeException('Expected unexpected happened');
162+
}
163+
164+
function call_throw_it() {
165+
throw_it();
166+
}
167+
168+
set_exception_handler('user_exception_handler');
169+
170+
call_throw_it();

‎tests/integration/errors/test_uncaught_handled_exception_02.php

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
<?php
2+
/*
3+
* Copyright 2020 New Relic Corporation. All rights reserved.
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/*DESCRIPTION
8+
When a user exception handler unregisters itself as an exception handler when it handles uncaught exception,
9+
the agent should record the error and add error attributes on all spans leading to uncaught exception as
10+
well as the one throwing the exception. Error attributtes are not expected on the root span (because
11+
the exception has been handled) as well as on the span created for exception handler.
12+
*/
13+
14+
/*INI
15+
newrelic.distributed_tracing_enabled=1
16+
newrelic.transaction_tracer.threshold = 0
17+
newrelic.span_events_enabled=1
18+
newrelic.code_level_metrics.enabled = 0
19+
display_errors=1
20+
log_errors=0
21+
*/
22+
23+
24+
/*SKIPIF
25+
<?php
26+
if (version_compare(PHP_VERSION, "8.0", "<")) {
27+
die("skip: PHP < 8.0.0 not supported\n");
28+
}
29+
*/
30+
31+
32+
/*EXPECT_ERROR_EVENTS
33+
[
34+
"?? agent run id",
35+
{
36+
"reservoir_size": "??",
37+
"events_seen": 1
38+
},
39+
[
40+
[
41+
{
42+
"type": "TransactionError",
43+
"timestamp": "??",
44+
"error.class": "RuntimeException",
45+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
46+
"transactionName": "OtherTransaction\/php__FILE__",
47+
"duration": "??",
48+
"nr.transactionGuid": "??",
49+
"guid": "??",
50+
"sampled": true,
51+
"priority": "??",
52+
"traceId": "??",
53+
"spanId": "??"
54+
},
55+
{},
56+
"??"
57+
]
58+
]
59+
]
60+
*/
61+
62+
/*EXPECT_SPAN_EVENTS
63+
[
64+
"?? agent run id",
65+
{
66+
"reservoir_size": 10000,
67+
"events_seen": 4
68+
},
69+
[
70+
[
71+
{
72+
"category": "generic",
73+
"type": "Span",
74+
"guid": "??",
75+
"traceId": "??",
76+
"transactionId": "??",
77+
"name": "OtherTransaction\/php__FILE__",
78+
"timestamp": "??",
79+
"duration": "??",
80+
"priority": "??",
81+
"sampled": true,
82+
"nr.entryPoint": true,
83+
"transaction.name": "OtherTransaction\/php__FILE__"
84+
},
85+
{},
86+
{}
87+
],
88+
[
89+
{
90+
"category": "generic",
91+
"type": "Span",
92+
"guid": "??",
93+
"traceId": "??",
94+
"transactionId": "??",
95+
"name": "Custom\/call_throw_it",
96+
"timestamp": "??",
97+
"duration": "??",
98+
"priority": "??",
99+
"sampled": true,
100+
"parentId": "??"
101+
},
102+
{},
103+
{
104+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
105+
"error.class": "RuntimeException"
106+
}
107+
],
108+
[
109+
{
110+
"category": "generic",
111+
"type": "Span",
112+
"guid": "??",
113+
"traceId": "??",
114+
"transactionId": "??",
115+
"name": "Custom\/throw_it",
116+
"timestamp": "??",
117+
"duration": "??",
118+
"priority": "??",
119+
"sampled": true,
120+
"parentId": "??"
121+
},
122+
{},
123+
{
124+
"error.message": "Uncaught exception 'RuntimeException' with message 'Expected unexpected happened' in __FILE__:??",
125+
"error.class": "RuntimeException"
126+
}
127+
],
128+
[
129+
{
130+
"category": "generic",
131+
"type": "Span",
132+
"guid": "??",
133+
"traceId": "??",
134+
"transactionId": "??",
135+
"name": "Custom/{closure}",
136+
"timestamp": "??",
137+
"duration": "??",
138+
"priority": "??",
139+
"sampled": true,
140+
"parentId": "??"
141+
},
142+
{},
143+
{}
144+
]
145+
]
146+
]
147+
*/
148+
149+
150+
/*EXPECT_REGEX
151+
Handled uncaught exception
152+
*/
153+
154+
155+
function throw_it() {
156+
throw new RuntimeException('Expected unexpected happened');
157+
}
158+
159+
function call_throw_it() {
160+
throw_it();
161+
}
162+
163+
set_exception_handler(function (Throwable $ex) {
164+
restore_exception_handler();
165+
echo "Handled uncaught exception";
166+
});
167+
168+
call_throw_it();

0 commit comments

Comments
 (0)