Skip to content

fix: reset socket retry state and handle failed reconnects - #274

Merged
marandaneto merged 1 commit into
mainfrom
fix/socket-retry-state
Oct 22, 2026
Merged

marandaneto merged 1 commit into
mainfrom
fix/socket-retry-state

Conversation

@marandaneto

Copy link
Copy Markdown
Member

💡 Motivation and Context

The socket consumer keeps its previous write counters when retrying a request. After HTTP 503 or 429, it can open a new connection without sending the request again. If reconnecting fails, it passes false to fread(), causing a TypeError that can escape request-end destruction.

Reset the write state for each attempt and return false if reconnecting fails. This PR is independent of the destructor and timeout fixes.

💚 How did you test it?

  • Added loopback regression tests for successful retries with plain and gzip payloads, and for a 503 followed by connection refusal. They failed on the original code and pass with this fix.
  • Socket tests: 15 passing tests and 160 assertions.
  • Full PHPUnit suite: 1,348 passing tests and 6,812 assertions.
  • Python adapter suite: 16 passing tests.
  • Replayed the failed-reconnect scenario under local PHP-FPM. The completed application request no longer becomes HTTP 500.
  • Changed-file PHP_CodeSniffer, public API snapshot, and whitespace checks pass. Existing repository-wide style warnings and deprecations remain. No local coverage driver is installed.
  • Final branch autoreview against origin/main passed at fc30bc109cfaae80084ab78d294d6cb4a31fc73e, with no actionable findings.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented with Pi using worker/reviewer subagents, file tools, Git/GitHub CLI, and the isolated autoreview helper. The Pi session transcript is local and has not been published. Manoel directed the scope and requested separate, independently mergeable PRs.

These are SDK robustness fixes identified while investigating PHP runtime advisories. They do not fix PHP's runtime vulnerabilities or replace upgrading PHP. Human review is required.

@marandaneto marandaneto self-assigned this Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-10-21T17:09:33.745473+00:00
Duration: 116718ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 30ms
Format Validation.Event Has Uuid ✅ 524ms
Format Validation.Event Has Lib Properties ✅ 525ms
Format Validation.Distinct Id Is String ✅ 525ms
Format Validation.Token Is Present ✅ 525ms
Format Validation.Custom Properties Preserved ✅ 526ms
Format Validation.Event Has Timestamp ✅ 527ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 528ms
Retry Behavior.Retries On 503 ✅ 5833ms
Retry Behavior.Does Not Retry On 400 ✅ 2530ms
Retry Behavior.Does Not Retry On 401 ✅ 2527ms
Retry Behavior.Respects Retry After Header ❌ 6337ms
Retry Behavior.Implements Backoff ✅ 16249ms
Retry Behavior.Retries On 500 ✅ 5632ms
Retry Behavior.Retries On 502 ✅ 5638ms
Retry Behavior.Retries On 504 ✅ 5634ms
Retry Behavior.Max Retries Respected ✅ 17052ms
Deduplication.Generates Unique Uuids ✅ 231ms
Deduplication.Preserves Uuid On Retry ✅ 5633ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10842ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5638ms
Deduplication.No Duplicate Events In Batch ✅ 530ms
Deduplication.Different Events Have Different Uuids ✅ 527ms
Compression.Sends Gzip When Enabled ✅ 526ms
Batch Format.Uses Proper Batch Structure ✅ 525ms
Batch Format.Flush With No Events Sends Nothing ✅ 521ms
Batch Format.Multiple Events Batched Together ✅ 513ms
Error Handling.Does Not Retry On 403 ✅ 2527ms
Error Handling.Does Not Retry On 413 ✅ 2526ms
Error Handling.Retries On 408 ✅ 5635ms

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 ✅ 523ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 522ms
Request Payload.Flags Request Omits Authorization Header ✅ 523ms
Request Payload.Token In Flags Body Matches Init ✅ 524ms
Request Payload.Groups Round Trip ✅ 525ms
Request Payload.Groups Default To Empty Object ✅ 524ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 522ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 522ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 522ms
Request Lifecycle.No Flags Request On Init Alone ✅ 517ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 511ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 527ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 528ms
Retry Behavior.Retries Flags On 502 ✅ 624ms
Retry Behavior.Retries Flags On 504 ✅ 627ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 526ms

@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-10-21T17:09:33.413919+00:00
Duration: 112806ms

⚠️ Some Tests Failed

36/47 tests passed, 11 failed


Capture Tests

⚠️ 19/30 tests passed, 11 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 33ms
Format Validation.Event Has Uuid ✅ 528ms
Format Validation.Event Has Lib Properties ✅ 528ms
Format Validation.Distinct Id Is String ✅ 529ms
Format Validation.Token Is Present ✅ 530ms
Format Validation.Custom Properties Preserved ✅ 529ms
Format Validation.Event Has Timestamp ✅ 529ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 529ms
Retry Behavior.Retries On 503 ❌ 5530ms
Retry Behavior.Does Not Retry On 400 ✅ 2534ms
Retry Behavior.Does Not Retry On 401 ✅ 2531ms
Retry Behavior.Respects Retry After Header ❌ 5533ms
Retry Behavior.Implements Backoff ❌ 15546ms
Retry Behavior.Retries On 500 ❌ 5534ms
Retry Behavior.Retries On 502 ❌ 5537ms
Retry Behavior.Retries On 504 ❌ 5534ms
Retry Behavior.Max Retries Respected ❌ 15544ms
Deduplication.Generates Unique Uuids ✅ 536ms
Deduplication.Preserves Uuid On Retry ❌ 5534ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 10540ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 5535ms
Deduplication.No Duplicate Events In Batch ✅ 537ms
Deduplication.Different Events Have Different Uuids ✅ 530ms
Compression.Sends Gzip When Enabled ✅ 530ms
Batch Format.Uses Proper Batch Structure ✅ 530ms
Batch Format.Flush With No Events Sends Nothing ✅ 516ms
Batch Format.Multiple Events Batched Together ✅ 521ms
Error Handling.Does Not Retry On 403 ✅ 2529ms
Error Handling.Does Not Retry On 413 ✅ 2532ms
Error Handling.Retries On 408 ❌ 5534ms

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 ✅ 524ms
Request Payload.Flags Request Uses V2 Query Param ✅ 521ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 520ms
Request Payload.Flags Request Omits Authorization Header ✅ 522ms
Request Payload.Token In Flags Body Matches Init ✅ 521ms
Request Payload.Groups Round Trip ✅ 520ms
Request Payload.Groups Default To Empty Object ✅ 521ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 520ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 521ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 520ms
Request Lifecycle.No Flags Request On Init Alone ✅ 515ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 517ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 523ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 523ms
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 ✅ 530ms

@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-10-21T17:09:38.687553+00:00
Duration: 115465ms

⚠️ Some Tests Failed

45/47 tests passed, 2 failed


Capture Tests

⚠️ 28/30 tests passed, 2 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 31ms
Format Validation.Event Has Uuid ✅ 525ms
Format Validation.Event Has Lib Properties ✅ 525ms
Format Validation.Distinct Id Is String ✅ 526ms
Format Validation.Token Is Present ✅ 526ms
Format Validation.Custom Properties Preserved ✅ 526ms
Format Validation.Event Has Timestamp ✅ 526ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 527ms
Retry Behavior.Retries On 503 ✅ 5837ms
Retry Behavior.Does Not Retry On 400 ✅ 2528ms
Retry Behavior.Does Not Retry On 401 ✅ 2527ms
Retry Behavior.Respects Retry After Header ❌ 5633ms
Retry Behavior.Implements Backoff ✅ 16253ms
Retry Behavior.Retries On 500 ✅ 5636ms
Retry Behavior.Retries On 502 ✅ 5634ms
Retry Behavior.Retries On 504 ✅ 5635ms
Retry Behavior.Max Retries Respected ✅ 16250ms
Deduplication.Generates Unique Uuids ✅ 534ms
Deduplication.Preserves Uuid On Retry ✅ 5634ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10839ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5643ms
Deduplication.No Duplicate Events In Batch ✅ 533ms
Deduplication.Different Events Have Different Uuids ✅ 529ms
Compression.Sends Gzip When Enabled ✅ 527ms
Batch Format.Uses Proper Batch Structure ✅ 528ms
Batch Format.Flush With No Events Sends Nothing ✅ 521ms
Batch Format.Multiple Events Batched Together ✅ 513ms
Error Handling.Does Not Retry On 403 ✅ 2529ms
Error Handling.Does Not Retry On 413 ✅ 2530ms
Error Handling.Retries On 408 ❌ 5531ms

Failures

retry_behavior.respects_retry_after_header

Retry delay too short: 103ms < 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 ✅ 527ms
Request Payload.Flags Request Uses V2 Query Param ✅ 523ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 523ms
Request Payload.Flags Request Omits Authorization Header ✅ 525ms
Request Payload.Token In Flags Body Matches Init ✅ 525ms
Request Payload.Groups Round Trip ✅ 523ms
Request Payload.Groups Default To Empty Object ✅ 524ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 525ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 523ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 524ms
Request Lifecycle.No Flags Request On Init Alone ✅ 518ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 510ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 528ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 525ms
Retry Behavior.Retries Flags On 502 ✅ 627ms
Retry Behavior.Retries Flags On 504 ✅ 627ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 527ms

@marandaneto
marandaneto marked this pull request as ready for review October 21, 2026 17:12
@marandaneto
marandaneto requested a review from a team as a code owner October 21, 2026 17:12

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PostHog Review · Standard · reviewed fc30bc1: no findings.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

@posthog posthog Bot removed the reviewhog label Oct 21, 2026
@marandaneto
marandaneto merged commit 195fa98 into main Oct 22, 2026
29 checks passed
@marandaneto
marandaneto deleted the fix/socket-retry-state branch October 22, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants