Repository navigation
fix: bound socket retries of redirects and unexpected responses - #264
Conversation
posthog-php-fork_curl Compliance ReportDate: 2026-10-28T13:26:00.923370+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 31ms |
| Format Validation.Event Has Uuid | ✅ | 526ms |
| Format Validation.Event Has Lib Properties | ✅ | 526ms |
| Format Validation.Distinct Id Is String | ✅ | 527ms |
| Format Validation.Token Is Present | ✅ | 528ms |
| Format Validation.Custom Properties Preserved | ✅ | 528ms |
| Format Validation.Event Has Timestamp | ✅ | 527ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 527ms |
| Retry Behavior.Retries On 503 | ❌ | 5530ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2531ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2529ms |
| Retry Behavior.Respects Retry After Header | ❌ | 5535ms |
| Retry Behavior.Implements Backoff | ❌ | 15542ms |
| Retry Behavior.Retries On 500 | ❌ | 5532ms |
| Retry Behavior.Retries On 502 | ❌ | 5533ms |
| Retry Behavior.Retries On 504 | ❌ | 5533ms |
| Retry Behavior.Max Retries Respected | ❌ | 15543ms |
| Deduplication.Generates Unique Uuids | ✅ | 532ms |
| Deduplication.Preserves Uuid On Retry | ❌ | 5533ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ❌ | 10536ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ❌ | 5535ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 530ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 528ms |
| Compression.Sends Gzip When Enabled | ✅ | 529ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 525ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 516ms |
| Batch Format.Multiple Events Batched Together | ✅ | 518ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2529ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2529ms |
| Error Handling.Retries On 408 | ❌ | 5532ms |
Failures
retry_behavior.retries_on_503
Expected at least 3 requests, got 1
retry_behavior.respects_retry_after_header
Expected at least 2 requests, got 1
retry_behavior.implements_backoff
Expected at least 3 requests, got 1
retry_behavior.retries_on_500
Expected at least 2 requests, got 1
retry_behavior.retries_on_502
Expected at least 2 requests, got 1
retry_behavior.retries_on_504
Expected at least 2 requests, got 1
retry_behavior.max_retries_respected
Expected 4 requests, got 1
deduplication.preserves_uuid_on_retry
Need at least 2 requests to check retry
deduplication.preserves_uuid_and_timestamp_on_retry
Expected at least 3 requests, got 1
deduplication.preserves_uuid_and_timestamp_on_batch_retry
Expected at least 2 requests, got 1
error_handling.retries_on_408
Expected at least 2 requests, got 1
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 520ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 518ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 519ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 519ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 519ms |
| Request Payload.Groups Round Trip | ✅ | 518ms |
| Request Payload.Groups Default To Empty Object | ✅ | 518ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 519ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 519ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 518ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 515ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 514ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 523ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 521ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 621ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 621ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 527ms |
posthog-php-lib_curl Compliance ReportDate: 2026-10-28T13:25:36.103815+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 27ms |
| Format Validation.Event Has Uuid | ✅ | 522ms |
| Format Validation.Event Has Lib Properties | ✅ | 523ms |
| Format Validation.Distinct Id Is String | ✅ | 522ms |
| Format Validation.Token Is Present | ✅ | 524ms |
| Format Validation.Custom Properties Preserved | ✅ | 523ms |
| Format Validation.Event Has Timestamp | ✅ | 523ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 524ms |
| Retry Behavior.Retries On 503 | ✅ | 5833ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2527ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2525ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6332ms |
| Retry Behavior.Implements Backoff | ✅ | 16248ms |
| Retry Behavior.Retries On 500 | ✅ | 5634ms |
| Retry Behavior.Retries On 502 | ✅ | 5629ms |
| Retry Behavior.Retries On 504 | ✅ | 5634ms |
| Retry Behavior.Max Retries Respected | ✅ | 17047ms |
| Deduplication.Generates Unique Uuids | ✅ | 231ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5632ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10839ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5635ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 530ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 525ms |
| Compression.Sends Gzip When Enabled | ✅ | 523ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 524ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 519ms |
| Batch Format.Multiple Events Batched Together | ✅ | 512ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2524ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2528ms |
| Error Handling.Retries On 408 | ✅ | 5629ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 804ms < 2500ms
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 525ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 521ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 521ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 522ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 522ms |
| Request Payload.Groups Round Trip | ✅ | 521ms |
| Request Payload.Groups Default To Empty Object | ✅ | 522ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 521ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 521ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 522ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 516ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 510ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 525ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 524ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 624ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 623ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 524ms |
posthog-php-socket Compliance ReportDate: 2026-10-28T13:25:23.914740+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 28ms |
| Format Validation.Event Has Uuid | ✅ | 523ms |
| Format Validation.Event Has Lib Properties | ✅ | 524ms |
| Format Validation.Distinct Id Is String | ✅ | 526ms |
| Format Validation.Token Is Present | ✅ | 524ms |
| Format Validation.Custom Properties Preserved | ✅ | 525ms |
| Format Validation.Event Has Timestamp | ✅ | 526ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 524ms |
| Retry Behavior.Retries On 503 | ✅ | 5834ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2527ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2527ms |
| Retry Behavior.Respects Retry After Header | ❌ | 5631ms |
| Retry Behavior.Implements Backoff | ✅ | 16249ms |
| Retry Behavior.Retries On 500 | ✅ | 5633ms |
| Retry Behavior.Retries On 502 | ✅ | 5633ms |
| Retry Behavior.Retries On 504 | ✅ | 5634ms |
| Retry Behavior.Max Retries Respected | ✅ | 16248ms |
| Deduplication.Generates Unique Uuids | ✅ | 532ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5632ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10839ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5638ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 530ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 526ms |
| Compression.Sends Gzip When Enabled | ✅ | 525ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 525ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 521ms |
| Batch Format.Multiple Events Batched Together | ✅ | 512ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2526ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2527ms |
| Error Handling.Retries On 408 | ❌ | 5530ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 102ms < 2500ms
error_handling.retries_on_408
Expected at least 2 requests, got 1
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 526ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 523ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 522ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 522ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 522ms |
| Request Payload.Groups Round Trip | ✅ | 523ms |
| Request Payload.Groups Default To Empty Object | ✅ | 522ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 523ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 522ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 523ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 518ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 509ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 528ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 524ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 625ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 626ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 525ms |
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review. Verdict: needs attention.
f36b4d5 to
e08cb50
Compare
dustinbyrne
left a comment
There was a problem hiding this comment.
approving, as this looks to solve an issue, though i think it's worth @PostHog/team-client-libraries circling back on this and revisiting PHP delivery in general
| } else { | ||
| if ($this->debug()) { | ||
| $this->handleError($res["status"], $res["message"]); | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| // Retry uploading... | ||
| $backoff *= 2; | ||
| $socket = $this->createSocket(); | ||
| if (!$socket) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
our other SDKs would allow the retry with backoff here, instead of dropping the data outright.
generally it seems preferable for us to separate delivery strategy from buffering/retention to ensure we're dropping the least amount of data possible.
unexpected status codes or network responses should be transient, so it seems reasonable to continue retrying
There was a problem hiding this comment.
🤖 Automated reply from Paul's agent:
Good point, thanks. Changed in 4c5f707. Socket now retries redirects and other unexpected responses with backoff, and stops at the backoff limit. Only 4xx client errors (other than 429) drop the batch.
The stacked PR #265 (628fb24) uses the same rule for Socket, ForkCurl, and LibCurl. When retries run out, the batch stays in the queue for a later flush, so it is not dropped. HttpClient's own retry loop does not change.
I agree with your bigger point about keeping data storage separate from how we send it. That is for @PostHog/team-client-libraries to plan. #265 moves part of the way, because a batch that fails with a retryable error stays queued.
The SDKs do not agree on 3xx today: Python retries them and Go drops them. The spec does not say. I noted this in the #265 description.
Main already resets the Socket write state on retry (#274), and its tests cover the same cases. Keep the compliance README note. Generated-By: PostHog Desktop Task-Id: e73c8b25-9501-4f04-9fd8-e82415e1f9f9
Socket now sends the full request again on each retry. A 3xx or other response that is not 200, not retryable and below 400 skipped both the retry limit and the sleep, so a redirect loop never ended. Treat every such response as final and drop the batch, as 4xx already did. Generated-By: PostHog Desktop Task-Id: e73c8b25-9501-4f04-9fd8-e82415e1f9f9
Redirects and other unexpected responses may be transient, so Socket now retries them within the backoff limit instead of dropping the batch. Only 4xx responses other than 429 are final. Generated-By: PostHog Desktop Task-Id: e73c8b25-9501-4f04-9fd8-e82415e1f9f9
4c5f707 to
f382a22
Compare
🤖 Automated PR prepared with PostHog Desktop.
💡 Motivation and Context
The Socket consumer resent a batch forever when the server answered with a redirect (3xx) or another unexpected response, such as 204. These retries skipped the backoff limit.
This PR was smaller before main merged #274. That PR shipped the original fix: Socket now sends the complete request on each retry. This PR now holds only the redirect fix and the README note.
💚 How did you test it?
Parameterized tests cover 301, 302, 307, 308, and 204. One test checks that Socket retries the full request and succeeds on the next 200. The other checks that repeated redirects stop at the backoff limit. Both failed before the fix and pass after it.
The full PHPUnit suite,
composer validate,composer api:check, the Python adapter tests, and phpcs pass. I did not run the full compliance harness again.📝 Checklist
If releasing new changes
pnpm changeto generate a change intent file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
PostHog Desktop agents wrote and rebased the change. Human review is required.
Created with PostHog Desktop