Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(agent): Fix error reporting when EH_THROW is enabled #876

Merged
merged 11 commits into from
Apr 18, 2024
27 changes: 25 additions & 2 deletions agent/php_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -561,12 +561,36 @@ static int nr_php_should_record_error(int type, const char* format TSRMLS_DC) {
return 0;
}

/*
* For a the following error types:
* 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. PHP 7 additionally triggers an exception
* for E_RECOVERABLE_ERROR.
* 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 exit and let the exception handler
* deal with it; otherwise, we could record an error even if an exception is
* caught.
*/
#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO
#define E_ERRORS_THROWING_EXCEPTIONS (E_WARNING | E_CORE_WARNING | E_COMPILE_WARNING | E_USER_WARNING | E_RECOVERABLE_ERROR)
#else
#define E_ERRORS_THROWING_EXCEPTIONS (E_WARNING | E_CORE_WARNING | E_COMPILE_WARNING | E_USER_WARNING)
#endif

#define E_THROWS_EXCEPTION(e) ((E_ERRORS_THROWING_EXCEPTIONS & e) == e)

if (EG(error_handling) == EH_THROW && E_THROWS_EXCEPTION(type)) {
// let the exception handler deal with this error
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;
}
Expand Down Expand Up @@ -625,7 +649,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);

Expand Down
69 changes: 69 additions & 0 deletions tests/integration/errors/test_EH_THROW_errors_caught_exception.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

/*DESCRIPTION
For a certain number of error types, 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.
*/

/*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());
}
112 changes: 112 additions & 0 deletions tests/integration/errors/test_EH_THROW_errors_uncaught_exception.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

/*DESCRIPTION
For a certain number of error types, PHP triggers an exception if EH_THROW is toggled on.
Verify we 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.
*/

/*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");
Loading