Skip to content

Commit

Permalink
Track “failed attempts” rather than failed retries
Browse files Browse the repository at this point in the history
  • Loading branch information
@WPprodigy
WPprodigy committed Dec 13, 2023
1 parent f1f41b2 commit 5ec681e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 16 deletions.
21 changes: 11 additions & 10 deletions tests/vip-jetpack/connection-pilot/test-class-jetpack-connection-pilot.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

/**
* @requires function Jetpack_Options::get_option
* @group wtf
*/
class Connection_Pilot_Test extends WP_UnitTestCase {
protected static function getMethod( $name ) {
Expand All @@ -22,13 +23,13 @@ protected static function getMethod( $name ) {
* @preserveGlobalState disabled
* @dataProvider get_test_data__update_heartbeat_on_failure
*/
public function test__update_heartbeat_on_failure( ?int $backoff_factor, int $expected_backoff, int $retry_count ) {
public function test__update_heartbeat_on_failure( ?int $backoff_factor, int $expected_backoff, int $failed_attempts ) {
$last_failure = time() - 1000;

$this->set_heartbeat( [
'site_url' => 'example.com',
'backoff_factor' => $backoff_factor,
'retry_count' => $retry_count,
'failed_attempts' => $failed_attempts,
'failure_timestamp' => $last_failure,
] );

Expand All @@ -38,7 +39,7 @@ public function test__update_heartbeat_on_failure( ?int $backoff_factor, int $ex

$option = $this->get_heartbeat();
$this->assertEquals( $expected_backoff, $option['backoff_factor'] );
$this->assertEquals( $retry_count + 1, $option['retry_count'] );
$this->assertEquals( $failed_attempts + 1, $option['failed_attempts'] );
$this->assertTrue( $option['failure_timestamp'] > $last_failure );

// Doesn't change.
Expand All @@ -54,7 +55,7 @@ public function test__update_heartbeat_on_success() {
'site_url' => 'example.com',
'cache_site_id' => 22,
'backoff_factor' => 24,
'retry_count' => 2,
'failed_attempts' => 2,
'failure_timestamp' => time() - 1000,
] );

Expand All @@ -69,7 +70,7 @@ public function test__update_heartbeat_on_success() {

// Resets.
$this->assertEquals( 0, $option['backoff_factor'] );
$this->assertEquals( 0, $option['retry_count'] );
$this->assertEquals( 0, $option['failed_attempts'] );
$this->assertEquals( 0, $option['failure_timestamp'] );
}

Expand All @@ -78,11 +79,11 @@ public function test__update_heartbeat_on_success() {
* @preserveGlobalState disabled
* @dataProvider get_test_data__should_back_off
*/
public function test__should_back_off( ?int $backoff_factor, ?DateTime $failure_time, ?DateTime $legacy_time, int $retry_count, bool $expected ) {
public function test__should_back_off( ?int $backoff_factor, ?DateTime $failure_time, ?DateTime $legacy_time, int $failed_attempts, bool $expected ) {
if ( null !== $backoff_factor ) {
$this->set_heartbeat( [
'backoff_factor' => $backoff_factor,
'retry_count' => $retry_count,
'failed_attempts' => $failed_attempts,
'failure_timestamp' => null === $failure_time ? $legacy_time->getTimestamp() : $failure_time->getTimestamp(),
] );
}
Expand Down Expand Up @@ -117,7 +118,7 @@ public function get_test_data__should_back_off() {
$connection_pilot = Connection_Pilot::instance();
$max_retries = $connection_pilot::MAX_RETRIES;

// [ current backoff factor, last failure's timestamp, legacy timestamp, retry count, expected result ]
// [ current backoff factor, last failure's timestamp, legacy timestamp, failed attempts, expected result ]
return [
'null' => [ null, new DateTime(), null, 0, false ],
'zero' => [ 0, new DateTime(), null, 0, false ],
Expand All @@ -127,7 +128,7 @@ public function get_test_data__should_back_off() {
'eight-hours-false' => [ 8, ( new DateTime() )->modify( '-9 hours' ), null, 0, false ],
'eight-hours-legacy-true' => [ 8, null, new DateTime(), 0, true ],
'eight-hours-legacy-false' => [ 8, null, ( new DateTime() )->modify( '-9 hours' ), 0, false ],
'exceeds-retry-limit-true' => [ 8, ( new DateTime() )->modify( '-9 hours' ), null, $max_retries, true ],
'exceeds-retry-limit-true' => [ 8, ( new DateTime() )->modify( '-9 hours' ), null, $max_retries + 1, true ],
];
}

Expand All @@ -142,7 +143,7 @@ private function set_heartbeat( $values = [] ) {
'cache_site_id' => $values['cache_site_id'] ?? (int) \Jetpack_Options::get_option( 'id', - 1 ),
'success_timestamp' => $values['success_timestamp'] ?? time(),
'backoff_factor' => $values['backoff_factor'] ?? 0,
'retry_count' => $values['retry_count'] ?? 0,
'failed_attempts' => $values['failed_attempts'] ?? 0,
'failure_timestamp' => $values['failure_timestamp'] ?? 0,
], false );

Expand Down
12 changes: 6 additions & 6 deletions vip-jetpack/connection-pilot/class-jetpack-connection-pilot.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ private function should_back_off(): bool {
return false;
}

$retry_count = $this->last_heartbeat['retry_count'] ?? 0;
if ( $retry_count >= self::MAX_RETRIES ) {
$failed_attempts = $this->last_heartbeat['failed_attempts'] ?? 0;
if ( $failed_attempts >= self::MAX_RETRIES + 1 ) {
return true;
}

Expand Down Expand Up @@ -234,7 +234,7 @@ private function update_heartbeat_on_failure(): void {

// Just want to update some values, not overwrite them all.
$new_heartbeat['backoff_factor'] = $new_backoff_factor;
$new_heartbeat['retry_count'] = isset( $new_heartbeat['retry_count'] ) ? $new_heartbeat['retry_count'] + 1 : 0;
$new_heartbeat['failed_attempts'] = isset( $new_heartbeat['failed_attempts'] ) ? $new_heartbeat['failed_attempts'] + 1 : 1;
$new_heartbeat['failure_timestamp'] = time();

$update = update_option( self::HEARTBEAT_OPTION_NAME, $new_heartbeat, false );
Expand All @@ -253,7 +253,7 @@ private function update_heartbeat_on_success(): void {

// Reset these, as we're now successfully connected.
$option['backoff_factor'] = 0;
$option['retry_count'] = 0;
$option['failed_attempts'] = 0;
$option['failure_timestamp'] = 0;

$update = update_option( self::HEARTBEAT_OPTION_NAME, $option, false );
Expand Down Expand Up @@ -340,8 +340,8 @@ protected function send_alert( $message = '', $wp_error = null ) {
$message .= sprintf( ' Backoff Factor: %s hours.', $last_heartbeat['backoff_factor'] );
}

if ( isset( $last_heartbeat['retry_count'] ) && $last_heartbeat['retry_count'] > 0 ) {
$message .= sprintf( ' Retry Count: %s.', $last_heartbeat['retry_count'] );
if ( isset( $last_heartbeat['failed_attempts'] ) && $last_heartbeat['failed_attempts'] > 0 ) {
$message .= sprintf( ' Failed Attempts: %s.', $last_heartbeat['failed_attempts'] );
}

if ( is_wp_error( $wp_error ) ) {
Expand Down

0 comments on commit 5ec681e

Please sign in to comment.