Merge pull request #328 from Prospress/tweak_fetch_exception_handling

Tweak fetch exception handling

classes/ActionScheduler_InvalidActionException.php

@@ -9,6 +9,22 @@
  */
 class ActionScheduler_InvalidActionException extends \InvalidArgumentException implements ActionScheduler_Exception {
 
+	/**
+	 * Create a new exception when the action's schedule cannot be fetched.
+	 *
+	 * @param string $action_id The action ID with bad args.
+	 * @return static
+	 */
+	public static function from_schedule( $action_id, $schedule ) {
+		$message = sprintf(
+			__( 'Action [%s] has an invalid schedule: %s', 'action-scheduler' ),
+			$action_id,
+			var_export( $schedule, true )
+		);
+
+		return new static( $message );
+	}
+
 	/**
 	 * Create a new exception when the action's args cannot be decoded to an array.
 	 *
@@ -17,10 +33,11 @@ class ActionScheduler_InvalidActionException extends \InvalidArgumentException i
 	 * @param string $action_id The action ID with bad args.
 	 * @return static
 	 */
-	public static function from_decoding_args( $action_id ) {
+	public static function from_decoding_args( $action_id, $args = array() ) {
 		$message = sprintf(
-			__( 'Action [%s] has invalid arguments. It cannot be JSON decoded to an array.', 'action-scheduler' ),
-			$action_id
+			__( 'Action [%s] has invalid arguments. It cannot be JSON decoded to an array. $args = %s', 'action-scheduler' ),
+			$action_id,
+			var_export( $args, true )
 		);
 
 		return new static( $message );

classes/abstracts/ActionScheduler_Logger.php

@@ -55,7 +55,7 @@ public function init() {
 		add_action( 'action_scheduler_unexpected_shutdown', array( $this, 'log_unexpected_shutdown' ), 10, 2 );
 		add_action( 'action_scheduler_reset_action', array( $this, 'log_reset_action' ), 10, 1 );
 		add_action( 'action_scheduler_execution_ignored', array( $this, 'log_ignored_action' ), 10, 1 );
-		add_action( 'action_scheduler_failed_fetch_action', array( $this, 'log_failed_fetch_action' ), 10, 1 );
+		add_action( 'action_scheduler_failed_fetch_action', array( $this, 'log_failed_fetch_action' ), 10, 2 );
 	}
 
 	public function hook_stored_action() {
@@ -104,7 +104,20 @@ public function log_ignored_action( $action_id ) {
 		$this->log( $action_id, __( 'action ignored', 'action-scheduler' ) );
 	}
 
-	public function log_failed_fetch_action( $action_id ) {
-		$this->log( $action_id, __( 'There was a failure fetching this action', 'action-scheduler' ) );
+	/**
+	 * @param string $action_id
+	 * @param Exception|NULL $exception The exception which occured when fetching the action. NULL by default for backward compatibility.
+	 *
+	 * @return ActionScheduler_LogEntry[]
+	 */
+	public function log_failed_fetch_action( $action_id, Exception $exception = NULL ) {
+
+		if ( ! is_null( $exception ) ) {
+			$log_message = sprintf( __( 'There was a failure fetching this action: %s', 'action-scheduler' ), $exception->getMessage() );
+		} else {
+			$log_message = __( 'There was a failure fetching this action', 'action-scheduler' );
+		}
+
+		$this->log( $action_id, $log_message );
 	}
 }

classes/abstracts/ActionScheduler_Store.php

@@ -174,6 +174,40 @@ protected function get_scheduled_date_string_local( ActionScheduler_Action $acti
 		return $next->format( 'Y-m-d H:i:s' );
 	}
 
+	/**
+	 * Validate that we could decode action arguments.
+	 *
+	 * @param mixed $args      The decoded arguments.
+	 * @param int   $action_id The action ID.
+	 *
+	 * @throws ActionScheduler_InvalidActionException When the decoded arguments are invalid.
+	 */
+	protected function validate_args( $args, $action_id ) {
+		// Ensure we have an array of args.
+		if ( ! is_array( $args ) ) {
+			throw ActionScheduler_InvalidActionException::from_decoding_args( $action_id );
+		}
+
+		// Validate JSON decoding if possible.
+		if ( function_exists( 'json_last_error' ) && JSON_ERROR_NONE !== json_last_error() ) {
+			throw ActionScheduler_InvalidActionException::from_decoding_args( $action_id, $args );
+		}
+	}
+
+	/**
+	 * Validate a ActionScheduler_Schedule object.
+	 *
+	 * @param mixed $schedule  The unserialized ActionScheduler_Schedule object.
+	 * @param int   $action_id The action ID.
+	 *
+	 * @throws ActionScheduler_InvalidActionException When the schedule is invalid.
+	 */
+	protected function validate_schedule( $schedule, $action_id ) {
+		if ( empty( $schedule ) || ! is_a( $schedule, 'ActionScheduler_Schedule' ) ) {
+			throw ActionScheduler_InvalidActionException::from_schedule( $post->ID, $schedule );
+		}
+	}
+
 	/**
 	 * @return array
 	 */

classes/data-stores/ActionScheduler_DBStore.php

@@ -114,7 +114,14 @@ public function fetch_action( $action_id ) {
 			return $this->get_null_action();
 		}
 
-		return $this->make_action_from_db_record( $data );
+		try {
+			$action = $this->make_action_from_db_record( $data );
+		} catch ( ActionScheduler_InvalidActionException $exception ) {
+			do_action( 'action_scheduler_failed_fetch_action', $action_id, $exception );
+			return $this->get_null_action();
+		}
+
+		return $action;
 	}
 
 	/**
@@ -138,6 +145,10 @@ protected function make_action_from_db_record( $data ) {
 		$hook     = $data->hook;
 		$args     = json_decode( $data->args, true );
 		$schedule = unserialize( $data->schedule );
+
+		$this->validate_args( $args, $data->action_id );
+		$this->validate_schedule( $schedule, $data->action_id );
+
 		if ( empty( $schedule ) ) {
 			$schedule = new ActionScheduler_NullSchedule();
 		}

classes/data-stores/ActionScheduler_wpPostStore.php

@@ -134,7 +134,15 @@ public function fetch_action( $action_id ) {
 		if ( empty($post) || $post->post_type != self::POST_TYPE ) {
 			return $this->get_null_action();
 		}
-		return $this->make_action_from_post($post);
+
+		try {
+			$action = $this->make_action_from_post( $post );
+		} catch ( ActionScheduler_InvalidActionException $exception ) {
+			do_action( 'action_scheduler_failed_fetch_action', $post->ID, $exception );
+			return $this->get_null_action();
+		}
+
+		return $action;
 	}
 
 	protected function get_post( $action_id ) {
@@ -151,19 +159,11 @@ protected function get_null_action() {
 	protected function make_action_from_post( $post ) {
 		$hook = $post->post_title;
 
-		try {
-			$args = json_decode( $post->post_content, true );
-			$this->validate_args( $args, $post->ID );
+		$args = json_decode( $post->post_content, true );
+		$this->validate_args( $args, $post->ID );
 
-			$schedule = get_post_meta( $post->ID, self::SCHEDULE_META_KEY, true );
-			if ( empty( $schedule ) || ! is_a( $schedule, 'ActionScheduler_Schedule' ) ) {
-				throw ActionScheduler_InvalidActionException::from_decoding_args( $post->ID );
-			}
-		} catch ( ActionScheduler_InvalidActionException $exception ) {
-			$schedule = new ActionScheduler_NullSchedule();
-			$args = array();
-			do_action( 'action_scheduler_failed_fetch_action', $post->ID );
-		}
+		$schedule = get_post_meta( $post->ID, self::SCHEDULE_META_KEY, true );
+		$this->validate_schedule( $schedule, $post->ID );
 
 		$group = wp_get_object_terms( $post->ID, self::GROUP_TAXONOMY, array('fields' => 'names') );
 		$group = empty( $group ) ? '' : reset($group);
@@ -812,24 +812,4 @@ public function init() {
 		$taxonomy_registrar = new ActionScheduler_wpPostStore_TaxonomyRegistrar();
 		$taxonomy_registrar->register();
 	}
-
-	/**
-	 * Validate that we could decode action arguments.
-	 *
-	 * @param mixed $args      The decoded arguments.
-	 * @param int   $action_id The action ID.
-	 *
-	 * @throws ActionScheduler_InvalidActionException When the decoded arguments are invalid.
-	 */
-	private function validate_args( $args, $action_id ) {
-		// Ensure we have an array of args.
-		if ( ! is_array( $args ) ) {
-			throw ActionScheduler_InvalidActionException::from_decoding_args( $action_id );
-		}
-
-		// Validate JSON decoding if possible.
-		if ( function_exists( 'json_last_error' ) && JSON_ERROR_NONE !== json_last_error() ) {
-			throw ActionScheduler_InvalidActionException::from_decoding_args( $action_id );
-		}
-	}
 }