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

Stop retrying events #103

Merged
merged 8 commits into from
Oct 17, 2024
26 changes: 24 additions & 2 deletions includes/EventManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ public function add_minutely_schedule( $schedules ) {
*/
public function shutdown(): void {

// Due to a bug sending too many events, we are temporarily disabling these.
$disabled_events = array( 'pageview', 'wp_mail', 'plugin_updated' );
foreach ( $this->queue as $index => $event ) {
if ( in_array( $event->key, $disabled_events, true ) ) {
unset( $this->queue[ $index ] );
}
}

// Separate out the async events
$async = array();
foreach ( $this->queue as $index => $event ) {
Expand Down Expand Up @@ -211,6 +219,9 @@ protected function send_request_events( array $events ): void {
*/
$response = $subscriber->notify( $events );

// Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.
continue;

if ( ! ( $subscriber instanceof HiiveConnection ) ) {
continue;
}
Expand Down Expand Up @@ -240,21 +251,29 @@ public function send_saved_events_batch(): void {
*
* @var array<int,Event> $events
*/
$events = $queue->pull( 100 );
$events = $queue->pull( 50 );

// If queue is empty, do nothing.
if ( empty( $events ) ) {
return;
}

$queue->reserve( array_keys( $events ) );
// Reserve the events in the queue so they are not processed by another instance.
if ( ! $queue->reserve( array_keys( $events ) ) ) {
// If the events fail to reserve, they will be repeatedly retried.
// It would be good to log this somewhere.
return;
}

foreach ( $this->get_subscribers() as $subscriber ) {
/**
* @var array{succeededEvents:array,failedEvents:array}|WP_Error $response
*/
$response = $subscriber->notify( $events );

// Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.
continue;

if ( ! ( $subscriber instanceof HiiveConnection ) ) {
continue;
}
Expand All @@ -274,5 +293,8 @@ public function send_saved_events_batch(): void {
$queue->release( array_keys( $response['failedEvents'] ) );
}
}

// Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.
$queue->remove( array_keys( $events ) );
}
}
28 changes: 28 additions & 0 deletions includes/HiiveConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,12 @@ public function notify( $events ) {
*/
public function hiive_request( string $path, ?array $payload = array(), ?array $args = array() ) {

/**
* @see \WP_Http::request()
* @see https://developer.wordpress.org/reference/hooks/http_headers_useragent/
*/
add_filter( 'http_headers_useragent', array( $this, 'add_plugin_name_version_to_user_agent' ), 10, 2 );

// If for some reason we are not connected, bail out now.
// If we are not connected, the throttling logic should eventually reconnect.
if ( ! self::is_connected() ) {
Expand Down Expand Up @@ -363,6 +369,8 @@ public function hiive_request( string $path, ?array $payload = array(), ?array $
}
}

remove_filter( 'http_headers_useragent', array( $this, 'add_plugin_name_version_to_user_agent' ) );

return $request_response;
}

Expand Down Expand Up @@ -405,4 +413,24 @@ public function get_core_data() {

return apply_filters( 'newfold_wp_data_module_core_data_filter', $data );
}

/**
* Add the plugin name and version to the user agent string
*
* @param string $user_agent E.g. "WordPress/6.4.3; https://example.org".
* @param string $url E.g. "https://hiive.cloud/api/sites/v2/events".
*
* @return string E.g. "WordPress/6.4.3; bluehost/1.2.3; https://example.org".
*/
public function add_plugin_name_version_to_user_agent( string $user_agent, string $url ): string {
$container = container();
$plugin_brand = sanitize_title( $container->plugin()->brand );
$plugin_version = $container->plugin()->get( 'version', '0' );

$user_agent_parts = array_map( 'trim', explode( ';', $user_agent ) );

array_splice( $user_agent_parts, 1, 0, "{$plugin_brand}/{$plugin_version}" );

return implode( '; ', $user_agent_parts );
}
}
91 changes: 76 additions & 15 deletions tests/phpunit/includes/EventManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,11 @@ public function test_init(): void {

/**
* @covers ::send_saved_events_batch
* @covers ::send
*/
public function test_send_saved_events_happy_path(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
Expand Down Expand Up @@ -186,7 +187,8 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 15 ) );
->with( array( 15 ) )
->andReturnTrue();

$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );

Expand Down Expand Up @@ -222,10 +224,11 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::send_saved_events_batch
* @covers ::send
*/
public function test_send_saved_events_happy_path_no_failed_events(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
Expand Down Expand Up @@ -254,7 +257,8 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 15 ) );
->with( array( 15 ) )
->andReturnTrue();

$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );

Expand Down Expand Up @@ -289,10 +293,11 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::send_saved_events_batch
* @covers ::send
*/
public function test_send_saved_events_happy_path_no_successful_events(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
Expand All @@ -312,7 +317,7 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'pull' )
->once()
->with( 100 )
->with( 50 )
->andReturn(
array(
15 => $event,
Expand All @@ -321,7 +326,8 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 15 ) );
->with( array( 15 ) )
->andReturnTrue();

$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );

Expand Down Expand Up @@ -356,10 +362,11 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::send_saved_events_batch
* @covers ::send
*/
public function test_send_saved_events_wp_error_from_hiive_connection(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
Expand All @@ -379,7 +386,7 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'pull' )
->once()
->with( 100 )
->with( 50 )
->andReturn(
array(
15 => $event,
Expand All @@ -388,7 +395,8 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 15 ) );
->with( array( 15 ) )
->andReturnTrue();

$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );

Expand Down Expand Up @@ -417,10 +425,11 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::send_saved_events_batch
* @covers ::send
*/
public function test_send_saved_events_failures_from_hiive(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
Expand All @@ -440,7 +449,7 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'pull' )
->once()
->with( 100 )
->with( 50 )
->andReturn(
array(
16 => $event,
Expand All @@ -449,7 +458,8 @@ function () use ( $batch_queue_mock ) {

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 16 ) );
->with( array( 16 ) )
->andReturnTrue();

$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );

Expand Down Expand Up @@ -487,6 +497,8 @@ function () use ( $batch_queue_mock ) {
*/
public function test_shutdown_happy_path_no_failed_events(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$sut = new EventManager();

$event = Mockery::mock( Event::class )->makePartial();
Expand Down Expand Up @@ -537,6 +549,8 @@ function () use ( $batch_queue_mock ) {
*/
public function test_shutdown_happy_path_with_failed_events(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$sut = new EventManager();

$event = Mockery::mock( Event::class )->makePartial();
Expand Down Expand Up @@ -584,10 +598,12 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::shutdown
* @covers ::send
* @covers ::send_request_events
*/
public function test_shutdown_hiive_connection_wp_error(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$sut = new EventManager();

$event = Mockery::mock( Event::class )->makePartial();
Expand Down Expand Up @@ -630,10 +646,12 @@ function () use ( $batch_queue_mock ) {

/**
* @covers ::shutdown
* @covers ::send
* @covers ::send_request_events
*/
public function test_shutdown_hiive_500_error(): void {

$this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' );

$sut = new EventManager();

$event = Mockery::mock( Event::class )->makePartial();
Expand Down Expand Up @@ -678,4 +696,47 @@ function () use ( $batch_queue_mock ) {

$this->assertConditionsMet();
}

/**
* @covers ::send_saved_events_batch
*/
public function test_send_saved_events_reserve_fails(): void {

$batch_queue_mock = Mockery::mock( BatchQueue::class );

\Patchwork\redefine(
array( EventQueue::class, '__construct' ),
function () {}
);
\Patchwork\redefine(
array( EventQueue::class, 'queue' ),
function () use ( $batch_queue_mock ) {
return $batch_queue_mock;
}
);

$sut = Mockery::mock( EventManager::class )->makePartial();

$event = Mockery::mock( Event::class );

$batch_queue_mock->expects( 'pull' )
->once()
->with( 50 )
->andReturn(
array(
15 => $event,
)
);

$batch_queue_mock->expects( 'reserve' )
->once()
->with( array( 15 ) )
->andReturnFalse();

$sut->expects( 'get_subscribers' )->never();

$sut->send_saved_events_batch();

$this->assertConditionsMet();
}
}
Loading
Loading