Commit c5f1887502d for woocommerce
commit c5f1887502ddac54fbd5d9cf9e2b71beb4c0a1aa
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Thu Sep 3 12:59:12 2026 +0300
Bound WC Tracker retries when the snapshot cannot be built (#68225)
* Bound retries for a WC Tracker snapshot that cannot be built
[Context]
The tracker retries a failed snapshot delivery on the next daily run
and gives up after MAX_CONSECUTIVE_SEND_FAILURES attempts, tracked in
woocommerce_tracker_send_failures.
[Problem]
That counter only moves inside record_send_failure(), which runs after
the POST returns. A fatal or timeout inside get_tracking_data() leaves
send_tracking_data() before any outcome is recorded, so the counter
never advances and woocommerce_tracker_last_send stays stale. The
scheduled gate reads only last_send, so the daily action rebuilt and
re-failed the heavy snapshot every day, forever.
The pre-send last_send stamp that used to cover this was removed when
retries were introduced; last_attempt replaced it but is read only by
the override branch. So the one failure mode that escapes the retry
bound is exactly the one the old safeguard handled.
[Solution]
Count the attempt before the snapshot is built and give up once it
exceeds the same limit a failed delivery gets, which puts the store
back on the weekly interval. record_send_failure() now reads that
already-persisted count instead of incrementing its own.
Opting out mid-request now deletes the retry state rather than
declining to write it, since the count already exists by then.
Extract finish_snapshot() for the send-time stamp and counter clear
that acceptance and abandonment have always shared.
Refs #68120
* Add changelog entry for the WC Tracker snapshot retry bound
* fix(tracker): Describe the abandonment cause the log can prove
The attempt counter is persisted before both the snapshot build and
the POST, so the give-up branch is reached whenever a run escapes
send_tracking_data() without recording a result. A Throwable out of
get_tracking_data() and a process death while blocked inside
wp_safe_remote_post() leave byte-identical persisted state.
The warning nonetheless asserted the snapshot "could not be built".
After three interruptions of the delivery kind it named a build
failure that never happened, pointing whoever reads the log at the
wrong phase.
Say only what the persisted state supports: the attempts ended
before a result was recorded. Less specific, but no longer wrong.
Distinguishing the phases for real would mean storing phase state on
every run, which is more machinery than the message is worth.
Refs #68225
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(tracker): State that last_send marks an attempt, not a delivery
woocommerce_tracker_last_send stopped meaning "a snapshot was
delivered" in #68120, which stamped it when a failed delivery was
abandoned. Bounding unbuildable snapshots widens it again: the stamp
now also advances when an attempt is given up before a snapshot ever
existed, so the timestamp can have no HTTP request behind it.
That value is not internal. get_last_send_time() passes the raw
option through the public woocommerce_tracker_last_send_time filter
(@since 2.3.0), so third-party callbacks receive a timestamp whose
meaning has shifted twice while the docblock still promised a send
time. finish_snapshot() had the same problem in reverse, documenting
a snapshot that does not exist at one of its call sites.
Say what both actually mark. Keeping last_send tied to a delivery
would need a separate close-out marker folded into the interval gate
with max(), which is two options to keep in sync for no behavioural
difference.
Refs #68225
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(tracker): Assert the abandonment warning, not just the path
test_unbuildable_snapshot_is_abandoned_after_max_attempts discarded
the logger from expect_tracker_warning(), so it proved the give-up
branch ran but nothing about what it logged.
That left the failure count in the warning context, $attempts - 1,
with no coverage anywhere in the suite. The off-by-one there is easy
to reintroduce and would misreport how many attempts a store spent
before backing off, which is the number an operator reads out of
Status > Logs when diagnosing a silent tracker.
Capture the logger and assert the warning count and the reported
failure count, following the pattern already used by
test_failed_send_is_retried_on_next_run in the same file. Confirmed
the assertion bites: changing $attempts - 1 to $attempts fails with
"Failed asserting that 4 is identical to 3".
Refs #68225
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/fix-tracker-unbuildable-snapshot-retries b/plugins/woocommerce/changelog/fix-tracker-unbuildable-snapshot-retries
new file mode 100644
index 00000000000..48d6be9cb02
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-tracker-unbuildable-snapshot-retries
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Stop rebuilding a WC Tracker snapshot on every daily run when building it fails.
diff --git a/plugins/woocommerce/includes/class-wc-tracker.php b/plugins/woocommerce/includes/class-wc-tracker.php
index 63e5f4cb46c..e1212884567 100644
--- a/plugins/woocommerce/includes/class-wc-tracker.php
+++ b/plugins/woocommerce/includes/class-wc-tracker.php
@@ -39,9 +39,10 @@ class WC_Tracker {
private static $api_url = 'https://tracking.woocommerce.com/v1/';
/**
- * Consecutive failed deliveries after which the current snapshot is abandoned.
+ * Consecutive failed attempts after which the current snapshot is abandoned.
*
- * Retries happen on the daily tracker action, so this bounds retrying to a few days.
+ * Counts attempts rather than delivery failures, so a snapshot that cannot be built is
+ * bounded too. Retries happen on the daily tracker action, so this is a few days.
*
* @var int
*/
@@ -87,6 +88,26 @@ class WC_Tracker {
// Recorded before building the snapshot so overlapping override sends are still suppressed.
update_option( 'woocommerce_tracker_last_attempt', time(), false );
+ // Count the attempt before the snapshot is built. A fatal or timeout inside
+ // get_tracking_data() never reaches record_send_result(), so a counter that only
+ // moved on delivery outcomes would let an unbuildable snapshot rebuild on every
+ // scheduled run instead of giving up the way a failed delivery does.
+ $attempts = (int) get_option( 'woocommerce_tracker_send_failures', 0 ) + 1;
+
+ if ( self::MAX_CONSECUTIVE_SEND_FAILURES < $attempts ) {
+ self::finish_snapshot();
+ wc_get_logger()->warning(
+ 'WooCommerce tracker snapshot attempts ended before recording a result; giving up until the next interval.',
+ array(
+ 'source' => 'woocommerce-tracker',
+ 'failures' => $attempts - 1,
+ )
+ );
+ return;
+ }
+
+ update_option( 'woocommerce_tracker_send_failures', $attempts, false );
+
$body = wp_json_encode( self::get_tracking_data() );
if ( false === $body ) {
self::record_send_failure( false, 0, 'json_encode_failure', 0 );
@@ -123,8 +144,7 @@ class WC_Tracker {
$status = is_wp_error( $response ) ? 0 : (int) wp_remote_retrieve_response_code( $response );
if ( 200 <= $status && 300 > $status ) {
- update_option( 'woocommerce_tracker_last_send', time() );
- delete_option( 'woocommerce_tracker_send_failures' );
+ self::finish_snapshot();
return;
}
@@ -134,10 +154,10 @@ class WC_Tracker {
/**
* Record a failed delivery attempt.
*
- * Consecutive retryable failures are counted so a persistent outage does not retry forever;
- * a non-retryable failure or the last allowed attempt gives up on the current snapshot.
- * Retry state is only kept while tracking is still enabled, since opting out may have
- * happened while the request was in flight.
+ * The attempt was already counted before the snapshot was built, so this only decides
+ * whether to keep retrying: a non-retryable failure or the last allowed attempt gives up
+ * on the current snapshot. Retry state is discarded when tracking was turned off while
+ * the request was in flight.
*
* @param bool $retryable Whether the next scheduled run should try again.
* @param int $status HTTP status code, 0 when no response was received.
@@ -145,14 +165,14 @@ class WC_Tracker {
* @param int $body_bytes Size of the snapshot.
*/
private static function record_send_failure( $retryable, $status, $error_code, $body_bytes ): void {
- $failures = (int) get_option( 'woocommerce_tracker_send_failures', 0 ) + 1;
+ // The attempt was already counted before the snapshot was built.
+ $failures = (int) get_option( 'woocommerce_tracker_send_failures', 0 );
$give_up = ! $retryable || self::MAX_CONSECUTIVE_SEND_FAILURES <= $failures;
if ( $give_up ) {
- update_option( 'woocommerce_tracker_last_send', time() );
+ self::finish_snapshot();
+ } elseif ( true !== wc_string_to_bool( get_option( 'woocommerce_allow_tracking', 'no' ) ) ) {
delete_option( 'woocommerce_tracker_send_failures' );
- } elseif ( true === wc_string_to_bool( get_option( 'woocommerce_allow_tracking', 'no' ) ) ) {
- update_option( 'woocommerce_tracker_send_failures', $failures, false );
}
if ( 413 === $status ) {
@@ -177,6 +197,18 @@ class WC_Tracker {
);
}
+ /**
+ * Close out the current tracking attempt, whether a snapshot was sent or the
+ * attempt was abandoned before one existed.
+ *
+ * The scheduled gate reads only the send time, so advancing it is what puts the
+ * store back on the weekly interval and stops the daily retries.
+ */
+ private static function finish_snapshot(): void {
+ update_option( 'woocommerce_tracker_last_send', time() );
+ delete_option( 'woocommerce_tracker_send_failures' );
+ }
+
/**
* Whether a delivery failure with the given status is worth retrying.
*
@@ -191,7 +223,11 @@ class WC_Tracker {
}
/**
- * Get the last time tracking data was sent.
+ * Get the time the current tracking attempt was closed out.
+ *
+ * Despite the option name, this is not only set on delivery: it also advances when an
+ * attempt is abandoned, including before a snapshot could be built. The scheduled gate
+ * reads it as "this cycle is done", not "a snapshot was sent".
*
* @return int|bool
*/
@@ -199,6 +235,8 @@ class WC_Tracker {
/**
* Filter the last time tracking data was sent.
*
+ * The timestamp also covers abandoned attempts, not only deliveries.
+ *
* @since 2.3.0
*/
return apply_filters( 'woocommerce_tracker_last_send_time', get_option( 'woocommerce_tracker_last_send', false ) );
diff --git a/plugins/woocommerce/tests/php/includes/class-wc-tracker-test.php b/plugins/woocommerce/tests/php/includes/class-wc-tracker-test.php
index 08a6fb04a03..e6f71de1f2b 100644
--- a/plugins/woocommerce/tests/php/includes/class-wc-tracker-test.php
+++ b/plugins/woocommerce/tests/php/includes/class-wc-tracker-test.php
@@ -348,6 +348,54 @@ class WC_Tracker_Test extends \WC_Unit_Test_Case {
$this->assertFalse( get_option( 'woocommerce_tracker_send_failures' ), 'Retry state must not be created once tracking is off.' );
}
+ /**
+ * @testdox Should stop rebuilding a snapshot that cannot be built after the same number of attempts a failed delivery gets.
+ *
+ * A Throwable escaping get_tracking_data() never reaches record_send_result(), so nothing
+ * downstream records the outcome. Throwing from a woocommerce_tracker_data callback
+ * reproduces that exact path: a real fatal is uncatchable, but both leave send_tracking_data()
+ * before any result is recorded.
+ */
+ public function test_unbuildable_snapshot_is_abandoned_after_max_attempts(): void {
+ update_option( 'woocommerce_allow_tracking', 'yes' );
+ $last_send = strtotime( '-2 weeks' );
+ update_option( 'woocommerce_tracker_last_send', $last_send );
+
+ $builds = 0;
+ $requests = 0;
+ add_filter(
+ 'woocommerce_tracker_data',
+ function () use ( &$builds ) {
+ ++$builds;
+ throw new RuntimeException( 'Snapshot build failed' );
+ }
+ );
+ add_filter(
+ 'pre_http_request',
+ function () use ( &$requests ) {
+ ++$requests;
+ return array( 'response' => array( 'code' => 200 ) );
+ }
+ );
+ $logger = $this->expect_tracker_warning();
+
+ // Four consecutive scheduled runs, on a store where building the snapshot always fails.
+ for ( $run = 0; $run < 4; $run++ ) {
+ try {
+ WC_Tracker::send_tracking_data();
+ } catch ( RuntimeException $e ) {
+ continue;
+ }
+ }
+
+ $this->assertSame( 0, $requests, 'A snapshot that cannot be built is never posted.' );
+ $this->assertSame( 3, $builds, 'The build should be attempted the same number of times a failed delivery is retried, then abandoned.' );
+ $this->assertEqualsWithDelta( time(), (int) get_option( 'woocommerce_tracker_last_send' ), 5, 'Abandoning the snapshot should record the send time so the next attempt waits for the weekly interval.' );
+ $this->assertFalse( get_option( 'woocommerce_tracker_send_failures' ), 'Giving up should clear the failure counter.' );
+ $this->assertCount( 1, $logger->warnings, 'Abandoning the snapshot should be logged once, on the run that gives up.' );
+ $this->assertSame( 3, $logger->warnings[0]['failures'], 'The warning should report the attempts made, not the run that found the limit exceeded.' );
+ }
+
/**
* Fake the tracker HTTP response.
*