fix: auto-recover from a stuck sync instead of blocking for hours
Root cause of the 2-hour stall: a category page's PHP process hung (CLI-run cron processes typically have no execution time limit) and never returned, so it never reached the code path that marks a page complete, queues the next page, or marks the run failed. Action Scheduler's own watchdog eventually marked the action failed after its 3600s timeout, but by then the plugin's own run-tracking option was permanently stuck at status=running with no further pages ever queued - and since manual and scheduled syncs both check the same lock, this silently blocked all future syncs (not just this one) for the full 4-hour lock safety-net. Adds an 'updated_at' timestamp refreshed on every batch/page, and checks it in queue_full_sync(): if a "running" sync hasn't made progress in over 10 minutes, treat it as abandoned, clear the lock, and start fresh instead of waiting out the multi-hour safety-net. Also caps the 429 retry-after sleep at 30s rather than trusting an external API-supplied value without bound, as a defensive measure against another unbounded stall. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -189,6 +189,10 @@ class WBC_API_Client {
|
||||
// Handle rate limiting (429)
|
||||
if ( $http_code === 429 && $retry_count < self::MAX_RETRIES ) {
|
||||
$retry_after = isset( $data['error']['retryAfterSeconds'] ) ? (int) $data['error']['retryAfterSeconds'] : self::RETRY_DELAY * ( $retry_count + 1 );
|
||||
// Cap it - CLI-run cron processes often have no execution time
|
||||
// limit, so blindly trusting an external retry-after value could
|
||||
// stall a whole run for an unbounded amount of time.
|
||||
$retry_after = min( $retry_after, 30 );
|
||||
|
||||
WBC_Logger::warning( 'API', 'Rate limited, retrying after delay', array(
|
||||
'retry_after' => $retry_after,
|
||||
|
||||
@@ -65,6 +65,19 @@ class WBC_Batch_Sync {
|
||||
*/
|
||||
const ACTION_GROUP = 'wbc_product_sync';
|
||||
|
||||
/**
|
||||
* How long a "running" sync can go without progress before it's treated
|
||||
* as abandoned and cleared automatically on the next sync attempt.
|
||||
*
|
||||
* Normal pages/batches complete in well under a minute; 10 minutes gives
|
||||
* generous slack for a slow page or a couple of missed cron ticks while
|
||||
* still recovering quickly from a genuinely stuck run - which otherwise
|
||||
* blocks every sync (manual and scheduled) for the full 4-hour lock
|
||||
* safety-net, since a hung PHP process never reaches the code that would
|
||||
* mark the run failed or complete.
|
||||
*/
|
||||
const STALE_THRESHOLD = 10 * MINUTE_IN_SECONDS;
|
||||
|
||||
/**
|
||||
* Register the Action Scheduler batch handlers
|
||||
*/
|
||||
@@ -92,7 +105,7 @@ class WBC_Batch_Sync {
|
||||
* @return array Result with 'success' and 'message'.
|
||||
*/
|
||||
public static function queue_full_sync() {
|
||||
if ( get_transient( self::LOCK_TRANSIENT ) ) {
|
||||
if ( get_transient( self::LOCK_TRANSIENT ) && ! self::current_run_is_stale() ) {
|
||||
$run = get_option( self::RUN_OPTION, array() );
|
||||
return array(
|
||||
'success' => true,
|
||||
@@ -102,6 +115,21 @@ class WBC_Batch_Sync {
|
||||
);
|
||||
}
|
||||
|
||||
if ( get_transient( self::LOCK_TRANSIENT ) ) {
|
||||
// The lock exists but the run went stale (no progress in a while) -
|
||||
// the process handling it almost certainly died mid-execution
|
||||
// without reaching either the "next page" or "completed/failed"
|
||||
// path (e.g. a hung request in a CLI process with no execution
|
||||
// time limit). Don't make every future sync wait out the full
|
||||
// multi-hour lock safety-net for a one-off stuck run.
|
||||
$stale_run = get_option( self::RUN_OPTION, array() );
|
||||
WBC_Logger::warning( 'BatchSync', 'Previous sync run went stale, clearing and starting fresh', array(
|
||||
'stale_run_id' => $stale_run['run_id'] ?? '',
|
||||
'updated_at' => $stale_run['updated_at'] ?? ( $stale_run['started_at'] ?? '' ),
|
||||
) );
|
||||
delete_transient( self::LOCK_TRANSIENT );
|
||||
}
|
||||
|
||||
if ( get_option( 'wbc_enable_stock_sync', 'yes' ) !== 'yes' && get_option( 'wbc_enable_price_sync', 'yes' ) !== 'yes' ) {
|
||||
WBC_Logger::info( 'BatchSync', 'Sync skipped - both stock and price sync are disabled' );
|
||||
return array(
|
||||
@@ -170,6 +198,7 @@ class WBC_Batch_Sync {
|
||||
'failed' => 0,
|
||||
'skipped' => 0,
|
||||
'started_at' => current_time( 'mysql' ),
|
||||
'updated_at' => current_time( 'timestamp' ), // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- staleness math only, see current_run_is_stale().
|
||||
'finished_at' => '',
|
||||
), false );
|
||||
|
||||
@@ -201,6 +230,30 @@ class WBC_Batch_Sync {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the current tracked run has stopped making progress
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
private static function current_run_is_stale() {
|
||||
$run = get_option( self::RUN_OPTION, array() );
|
||||
|
||||
if ( empty( $run ) || ( $run['status'] ?? '' ) !== 'running' ) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$last_activity = (int) ( $run['updated_at'] ?? 0 );
|
||||
|
||||
if ( empty( $last_activity ) ) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Raw Unix timestamps on both sides (current_time('timestamp') is
|
||||
// WP-local but consistent with what's stored), so no string-parsing
|
||||
// timezone ambiguity between this and how 'updated_at' was written.
|
||||
return ( current_time( 'timestamp' ) - $last_activity ) > self::STALE_THRESHOLD; // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- comparing against a stored WP-local timestamp, not doing date math that needs UTC.
|
||||
}
|
||||
|
||||
/**
|
||||
* Kick off a BC-first, category-filtered sync
|
||||
*
|
||||
@@ -228,6 +281,7 @@ class WBC_Batch_Sync {
|
||||
'failed' => 0,
|
||||
'skipped' => 0,
|
||||
'started_at' => current_time( 'mysql' ),
|
||||
'updated_at' => current_time( 'timestamp' ), // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- staleness math only, see current_run_is_stale().
|
||||
'finished_at' => '',
|
||||
), false );
|
||||
|
||||
@@ -294,6 +348,7 @@ class WBC_Batch_Sync {
|
||||
|
||||
$run['status'] = 'failed';
|
||||
$run['finished_at'] = current_time( 'mysql' );
|
||||
$run['updated_at'] = current_time( 'timestamp' ); // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- staleness math only.
|
||||
update_option( self::RUN_OPTION, $run, false );
|
||||
delete_transient( self::LOCK_TRANSIENT );
|
||||
return;
|
||||
@@ -319,6 +374,7 @@ class WBC_Batch_Sync {
|
||||
$run['success'] = (int) ( $run['success'] ?? 0 ) + $page_results['success'];
|
||||
$run['failed'] = (int) ( $run['failed'] ?? 0 ) + $page_results['failed'];
|
||||
$run['skipped'] = (int) ( $run['skipped'] ?? 0 ) + $page_results['skipped'];
|
||||
$run['updated_at'] = current_time( 'timestamp' ); // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- staleness math only.
|
||||
|
||||
$has_more_pages = count( $items ) === self::CATEGORY_PAGE_SIZE;
|
||||
|
||||
@@ -374,6 +430,7 @@ class WBC_Batch_Sync {
|
||||
$run['success'] = (int) ( $run['success'] ?? 0 ) + $batch_results['success'];
|
||||
$run['failed'] = (int) ( $run['failed'] ?? 0 ) + $batch_results['failed'];
|
||||
$run['skipped'] = (int) ( $run['skipped'] ?? 0 ) + $batch_results['skipped'];
|
||||
$run['updated_at'] = current_time( 'timestamp' ); // phpcs:ignore WordPress.DateTime.CurrentTimeTimestamp -- staleness math only.
|
||||
|
||||
if ( $run['batches_done'] >= $run['total_batches'] ) {
|
||||
$run['status'] = 'completed';
|
||||
|
||||
Reference in New Issue
Block a user