Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/socket-retry-state.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog-php": patch
---

Resend the complete batch when the Socket consumer retries a request.
8 changes: 6 additions & 2 deletions lib/Consumer/Socket.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,15 @@ private function createSocket()
*/
private function makeRequest($socket, $req, $retry = true)
{
$bytes_written = 0;
$bytes_total = strlen($req);
$closed = false;

// Retries with exponential backoff until success
$backoff = 100; // Set initial waiting time to 100ms

while (true) {
$bytes_written = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: Bound retries for repeated redirects — Resetting the write offset now sends a complete POST after every 3xx response, but those responses bypass both the backoff-limit check and sleep below. A host/proxy continually returning 301/302 therefore makes flush() (including destructor flushing) send requests indefinitely and block the customer process. Previously the replacement connection received no request, timed out, and reached the retry limit through status 0. Please terminate unsupported responses or enforce the retry bound for every repeated request. Reproduction: reproduced — testRedirectResponsesDoNotBypassRetryBudget observed nine requests with maximum_backoff_duration=200 on the reviewed head, despite that budget permitting only two retryable attempts; the same test passed at the exact merge base.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pauldambra ping to unblock this one

$closed = false;

// Send request to server
while (!$closed && $bytes_written < $bytes_total) {
try {
Expand Down Expand Up @@ -173,6 +174,9 @@ private function makeRequest($socket, $req, $retry = true)
// Retry uploading...
$backoff *= 2;
$socket = $this->createSocket();
if (false === $socket) {
break;
}
}

return false;
Expand Down
12 changes: 8 additions & 4 deletions sdk_compliance_adapter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ not SDK shutdown certification; `/flush` always calls the real public SDK method

## Known failures and applicability

LibCurl is expected to pass all 47 selected tests. Socket and foreground ForkCurl
each expose these 11 existing capture failures:
LibCurl is expected to pass all 47 selected tests. Foreground ForkCurl exposes
these 11 capture failures. The last Socket report also contains these failures;
it predates the Socket write-state repair:

- `capture.retry_behavior.retries_on_503`
- `capture.retry_behavior.retries_on_500`
Expand All @@ -83,8 +84,11 @@ each expose these 11 existing capture failures:
- `capture.deduplication.preserves_uuid_and_timestamp_on_retry`
- `capture.deduplication.preserves_uuid_and_timestamp_on_batch_retry`

Socket does not reset its write state on retry, treats 408 as terminal, and ignores
Retry-After. ForkCurl does not retry HTTP rejection and reports a successful curl
Socket now resends the complete request on each attempt. Native regression tests
cover 503 and 429 followed by 200, stable request bodies, and queue drain. The full
Socket compliance suite was not rerun for that repair. Socket still treats 408 as
terminal and ignores Retry-After. ForkCurl does not retry HTTP rejection and
reports a successful curl
process exit as successful delivery even for HTTP errors. The adapter exposes
these behaviors rather than retrying on the SDK's behalf.

Expand Down
30 changes: 30 additions & 0 deletions test/ConsumerSocketTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PostHog\Test;

use PHPUnit\Framework\Attributes\DataProvider;
use RuntimeException;

class ConsumerSocketTest extends ConsumerTransportTestCase
Expand All @@ -11,6 +12,35 @@ protected function consumer(): string
return 'socket';
}

public static function retryableStatuses(): array
{
return ['server failure' => [503], 'rate limit' => [429]];
}

#[DataProvider('retryableStatuses')]
public function testRetrySendsTheCompleteBatch(int $status): void
{
$this->server->stop();
$this->server = new LocalHttpServer([['status' => $status], ['status' => 200]]);
$client = $this->client(['maximum_backoff_duration' => 200]);
$previousTimeout = ini_set('default_socket_timeout', '1');

try {
self::assertTrue($client->capture(['distinctId' => 'some-user', 'event' => 'retried event']));
self::assertTrue($client->flush());
$requests = $this->server->requests();
self::assertCount(2, $requests);
self::assertSame('POST /batch/ HTTP/1.1', $requests[1]['requestLine']);
self::assertSame($requests[0]['body'], $requests[1]['body']);
$payload = json_decode($requests[1]['body'], true, 512, JSON_THROW_ON_ERROR);
self::assertSame('retried event', $payload['batch'][0]['event']);
self::assertTrue($client->flush());
self::assertCount(2, $this->server->requests());
} finally {
ini_set('default_socket_timeout', $previousTimeout);
}
}

public function testShortTimeout(): void
{
$client = $this->client(['timeout' => 0.01]);
Expand Down
Loading