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

Catch and log exceptions thrown when actions can't be created, e.g. under a corrupt database schema #1000

Merged
merged 11 commits into from
Oct 24, 2023
Merged
23 changes: 19 additions & 4 deletions classes/ActionScheduler_ActionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class ActionScheduler_ActionFactory {
* @param array $args Args to pass to callbacks when the hook is triggered.
* @param ActionScheduler_Schedule $schedule The action's schedule.
* @param string $group A group to put the action in.
* phpcs:ignore Squiz.Commenting.FunctionComment.ExtraParamComment
* @param int $priority The action priority.
*
* @return ActionScheduler_Action An instance of the stored action.
Expand Down Expand Up @@ -246,7 +247,6 @@ public function repeat( $action ) {
* async_unique(), single() or single_unique(), etc.
*
* @internal Not intended for public use, should not be overriden by subclasses.
* @throws Exception May be thrown if invalid options are passed.
*
* @param array $options {
* Describes the action we wish to schedule.
Expand All @@ -263,7 +263,7 @@ public function repeat( $action ) {
* @type int $priority Lower values means higher priority. Should be in the range 0-255.
* }
*
* @return int
* @return int The action ID. Zero if there was an error scheduling the action.
*/
public function create( array $options = array() ) {
$defaults = array(
Expand Down Expand Up @@ -307,12 +307,27 @@ public function create( array $options = array() ) {
break;

default:
throw new Exception( "Unknown action type '{$options['type']}' specified when trying to create an action for '{$options['hook']}'." );
error_log( "Unknown action type '{$options['type']}' specified when trying to create an action for '{$options['hook']}'." );
return 0;
}

$action = new ActionScheduler_Action( $options['hook'], $options['arguments'], $schedule, $options['group'] );
$action->set_priority( $options['priority'] );
return $options['unique'] ? $this->store_unique_action( $action ) : $this->store( $action );

$action_id = 0;
try {
$action_id = $options['unique'] ? $this->store_unique_action( $action ) : $this->store( $action );
} catch ( Exception $e ) {
error_log(
sprintf(
/* translators: %1$s is the name of the hook to be enqueued, %2$s is the exception message. */
__( 'Caught exception while enqueuing action "%1$s": %2$s', 'action-scheduler' ),
$options['hook'],
$e->getMessage()
)
);
}
return $action_id;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/data-stores/ActionScheduler_DBLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private function create_entry_from_db_record( $record ) {
}

/**
* Retrieve the an action's log entries from the database.
* Retrieve an action's log entries from the database.
*
* @param int $action_id Action ID.
*
Expand Down
17 changes: 5 additions & 12 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ as_enqueue_async_action( $hook, $args, $group, $unique, $priority );

### Return value

`(integer)` the action's ID.

`(integer)` the action's ID. Zero if there was an error scheduling the action. The error will be sent to `error_log`.

## Function Reference / `as_schedule_single_action()`

Expand All @@ -75,8 +74,7 @@ as_schedule_single_action( $timestamp, $hook, $args, $group, $unique, $priority

### Return value

`(integer)` the action's ID.

`(integer)` the action's ID. Zero if there was an error scheduling the action. The error will be sent to `error_log`.

## Function Reference / `as_schedule_recurring_action()`

Expand All @@ -102,14 +100,13 @@ as_schedule_recurring_action( $timestamp, $interval_in_seconds, $hook, $args, $g

### Return value

`(integer)` the action's ID.

`(integer)` the action's ID. Zero if there was an error scheduling the action. The error will be sent to `error_log`.

## Function Reference / `as_schedule_cron_action()`

### Description

Schedule an action that recurs on a cron-like schedule.
Schedule an action that recurs on a cron-like schedule.

If execution of a cron-like action is delayed, the next attempt will still be scheduled according to the provided cron expression.

Expand All @@ -131,8 +128,7 @@ as_schedule_cron_action( $timestamp, $schedule, $hook, $args, $group, $unique, $

### Return value

`(integer)` the action's ID.

`(integer)` the action's ID. Zero if there was an error scheduling the action. The error will be sent to `error_log`.

## Function Reference / `as_unschedule_action()`

Expand Down Expand Up @@ -178,7 +174,6 @@ as_unschedule_all_actions( $hook, $args, $group )

`(string|null)` The scheduled action ID if a scheduled action was found, or null if no matching action found.


## Function Reference / `as_next_scheduled_action()`

### Description
Expand All @@ -201,7 +196,6 @@ as_next_scheduled_action( $hook, $args, $group );

`(integer|boolean)` The timestamp for the next occurrence of a pending scheduled action, true for an async or in-progress action or false if there is no matching action.


## Function Reference / `as_has_scheduled_action()`

### Description
Expand All @@ -224,7 +218,6 @@ as_has_scheduled_action( $hook, $args, $group );

`(boolean)` True if a matching action is pending or in-progress, false otherwise.


## Function Reference / `as_get_scheduled_actions()`

### Description
Expand Down
15 changes: 8 additions & 7 deletions functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* @param bool $unique Whether the action should be unique.
* @param int $priority Lower values take precedence over higher values. Defaults to 10, with acceptable values falling in the range 0-255.
*
* @return int The action ID.
* @return int The action ID. Zero if there was an error scheduling the action.
*/
function as_enqueue_async_action( $hook, $args = array(), $group = '', $unique = false, $priority = 10 ) {
if ( ! ActionScheduler::is_initialized( __FUNCTION__ ) ) {
Expand Down Expand Up @@ -63,7 +63,7 @@ function as_enqueue_async_action( $hook, $args = array(), $group = '', $unique =
* @param bool $unique Whether the action should be unique.
* @param int $priority Lower values take precedence over higher values. Defaults to 10, with acceptable values falling in the range 0-255.
*
* @return int The action ID.
* @return int The action ID. Zero if there was an error scheduling the action.
*/
function as_schedule_single_action( $timestamp, $hook, $args = array(), $group = '', $unique = false, $priority = 10 ) {
if ( ! ActionScheduler::is_initialized( __FUNCTION__ ) ) {
Expand Down Expand Up @@ -115,7 +115,7 @@ function as_schedule_single_action( $timestamp, $hook, $args = array(), $group =
* @param bool $unique Whether the action should be unique.
* @param int $priority Lower values take precedence over higher values. Defaults to 10, with acceptable values falling in the range 0-255.
*
* @return int The action ID.
* @return int The action ID. Zero if there was an error scheduling the action.
*/
function as_schedule_recurring_action( $timestamp, $interval_in_seconds, $hook, $args = array(), $group = '', $unique = false, $priority = 10 ) {
if ( ! ActionScheduler::is_initialized( __FUNCTION__ ) ) {
Expand Down Expand Up @@ -200,7 +200,7 @@ function as_schedule_recurring_action( $timestamp, $interval_in_seconds, $hook,
* @param bool $unique Whether the action should be unique.
* @param int $priority Lower values take precedence over higher values. Defaults to 10, with acceptable values falling in the range 0-255.
*
* @return int The action ID.
* @return int The action ID. Zero if there was an error scheduling the action.
*/
function as_schedule_cron_action( $timestamp, $schedule, $hook, $args = array(), $group = '', $unique = false, $priority = 10 ) {
if ( ! ActionScheduler::is_initialized( __FUNCTION__ ) ) {
Expand Down Expand Up @@ -283,9 +283,10 @@ function as_unschedule_action( $hook, $args = array(), $group = '' ) {
ActionScheduler::logger()->log(
$action_id,
sprintf(
/* translators: %s is the name of the hook to be cancelled. */
__( 'Caught exception while cancelling action: %s', 'action-scheduler' ),
esc_attr( $hook )
/* translators: %1$s is the name of the hook to be cancelled, %2$s is the exception message. */
__( 'Caught exception while cancelling action "%1$s": %2$s', 'action-scheduler' ),
$hook,
$exception->getMessage()
)
);

Expand Down
53 changes: 53 additions & 0 deletions tests/phpunit/procedural_api/procedural_api_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,59 @@ public function test_as_schedule_cron_action() {
$this->assertEquals( 0, $action_id_duplicate );
}

/**
* Test recovering from an incorrect database schema when scheduling a single action.
*/
public function test_as_recover_from_incorrect_schema() {
// custom error reporting so we can test for errors sent to error_log.
global $wpdb;
$wpdb->suppress_errors( true );
$error_capture = tmpfile();
$actual_error_log = ini_set( 'error_log', stream_get_meta_data( $error_capture )['uri'] );

// we need a hybrid store so that dropping the priority column will cause an exception.
$this->set_action_scheduler_store( new ActionScheduler_HybridStore() );
$this->assertEquals( 'ActionScheduler_HybridStore', get_class( ActionScheduler::store() ) );

// drop the priority column from the actions table.
$wpdb->query( "ALTER TABLE {$wpdb->actionscheduler_actions} DROP COLUMN priority" );

// try to schedule a single action.
$action_id = as_schedule_single_action( time(), 'hook_17', array( 'a', 'b' ), 'dummytest', true );

// ensure that no exception was thrown and zero was returned.
$this->assertEquals( 0, $action_id );

// try to schedule an async action.
$action_id = as_enqueue_async_action( 'hook_18', array( 'a', 'b' ), 'dummytest', true );
// ensure that no exception was thrown and zero was returned.
$this->assertEquals( 0, $action_id );

// try to schedule a recurring action.
$action_id = as_schedule_recurring_action( time(), MINUTE_IN_SECONDS, 'hook_19', array( 'a', 'b' ), 'dummytest', true );
// ensure that no exception was thrown and zero was returned.
$this->assertEquals( 0, $action_id );

// try to schedule a cron action.
$action_id = as_schedule_cron_action( time(), '0 0 * * *', 'hook_20', array( 'a', 'b' ), 'dummytest', true );
// ensure that no exception was thrown and zero was returned.
$this->assertEquals( 0, $action_id );

// ensure that all four errors were logged to error_log.
$logged_errors = stream_get_contents( $error_capture );
$this->assertContains( 'Caught exception while enqueuing action "hook_17": Error saving action', $logged_errors );
$this->assertContains( 'Caught exception while enqueuing action "hook_18": Error saving action', $logged_errors );
$this->assertContains( 'Caught exception while enqueuing action "hook_19": Error saving action', $logged_errors );
$this->assertContains( 'Caught exception while enqueuing action "hook_20": Error saving action', $logged_errors );
$this->assertContains( "Unknown column 'priority' in 'field list'", $logged_errors );

// recreate the priority column.
$wpdb->query( "ALTER TABLE {$wpdb->actionscheduler_actions} ADD COLUMN priority tinyint(10) UNSIGNED NOT NULL DEFAULT 10" );
// restore error logging.
$wpdb->suppress_errors( false );
ini_set( 'error_log', $actual_error_log );
}

/**
* Helper method to set actions scheduler store.
*
Expand Down
Loading