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.

Sign up for GitHub

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

Jump to bottom

Throw an ErrorException rather than a standard exception when an error is caught during action processing #1057

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Throw an ErrorException rather than a standard exception when an error is caught during action processing #1057

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9990dee
Throw error exception when a error occurs during action processing
james-allan May 9, 2024
d1cac98
Fix typos
james-allan May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions classes/abstracts/ActionScheduler_Abstract_QueueRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,25 @@ public function __construct( ActionScheduler_Store $store = null, ActionSchedule
* Process an individual action.
*
* @param int $action_id The action ID to process.
* @param string $context Optional identifer for the context in which this action is being processed, e.g. 'WP CLI' or 'WP Cron'
* @param string $context Optional identifier for the context in which this action is being processed, e.g. 'WP CLI' or 'WP Cron'
* Generally, this should be capitalised and not localised as it's a proper noun.
*/
public function process_action( $action_id, $context = '' ) {
// Temporarily override the error handler while we process the current action.
set_error_handler(
/**
* Temporary error handler which can catch errors and convert them into exceptions. This faciliates more
* Temporary error handler which can catch errors and convert them into exceptions. This facilitates more
* robust error handling across all supported PHP versions.
*
* @throws Exception
*
* @param int $type Error level expressed as an integer.
* @param string $message Error message.
* @param string $file File in which the error occurred.
* @param int $line Line number at which the error occurred.
*/
function ( $type, $message ) {
throw new Exception( $message );
function ( $type, $message, $file, $line ) {
throw new ErrorException( $message, 0, $type, $file, $line );
Copy link
Member

@barryhughes barryhughes May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@james-allan I'm not sure this is enough (or at least, may not work as expected with recent PHP runtimes). When I test:

  • action_scheduler_begin_execute is fired from within a try {} block, and the test snippet then triggers an error.
  • We enter this error handler, and throw the ErrorException.
  • We now return to the matching catch {} block which converts the throwable (in this case, the ErrorException) into a regular exception.

So, in my own local testing, I do not see the same result you document in the testing instructions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, I tested with PHP 8.3.

},
E_USER_ERROR | E_RECOVERABLE_ERROR
);
Expand Down
Loading