Skip to content

Commit

Permalink
fix(agent): Fix error reporting when EH_THROW is enabled
Browse files Browse the repository at this point in the history
  /*
   * 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.
   */
  • Loading branch information
zsistla committed Apr 11, 2024
1 parent e0d2d12 commit 0db7169
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 2 deletions.
20 changes: 18 additions & 2 deletions agent/php_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);

Expand Down
76 changes: 76 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,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());
}
119 changes: 119 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,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");

0 comments on commit 0db7169

Please sign in to comment.