Skip to content

fix: honor configured ingestion transport timeouts - #275

Merged
marandaneto merged 2 commits into
mainfrom
fix/ingestion-transport-timeouts
Oct 22, 2026
Merged

marandaneto merged 2 commits into
mainfrom
fix/ingestion-transport-timeouts

Conversation

@marandaneto

@marandaneto marandaneto commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

💡 Motivation and Context

The libcurl ingestion consumer creates its HTTP client without forwarding the configured timeout. ForkCurl does not set curl's connection or total timeout, so debug-mode requests can block indefinitely and background processes can outlive the intended deadline.

Forward the existing millisecond timeout to LibCurl's HTTP client. Convert it to seconds for ForkCurl's --connect-timeout and --max-time arguments in both execution modes. Round positive fractional milliseconds up to the next whole millisecond before passing them to either transport, so a value such as 0.5 cannot become an unlimited timeout.

The default remains 10 seconds. Explicit timeout=0 remains unlimited for verified requests, and LibCurl's existing non-verifying 1ms behavior is unchanged. This PR does not depend on the socket or destructor fixes.

💚 How did you test it?

  • Added stalled loopback receiver cases for LibCurl, synchronous ForkCurl, and spawned ForkCurl. A generous watchdog checks connection closure without exact elapsed-time assertions.
  • Added checks for zero timeout, the default, fractional milliseconds, millisecond-to-second argument conversion, and non-verifying LibCurl behavior.
  • The original timeout regressions had seven failures before the initial fix. The review follow-up reproduced six additional failures on the initial PR head, including stalled requests with timeout=0.5 in both transports.
  • The final focused suite passed repeatedly: 19 tests and 83 assertions.
  • Full PHPUnit suite: 1,364 passing tests and 6,875 assertions.
  • Python adapter suite: 16 passing tests.
  • 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 441365204771748071eb618792c68271c5e6748e, 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

@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 991f0de: 1 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

Comment thread lib/Consumer/LibCurl.php Outdated
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-10-21T17:27:55.835292+00:00
Duration: 112980ms

⚠️ 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 ✅ 36ms
Format Validation.Event Has Uuid ✅ 531ms
Format Validation.Event Has Lib Properties ✅ 531ms
Format Validation.Distinct Id Is String ✅ 533ms
Format Validation.Token Is Present ✅ 531ms
Format Validation.Custom Properties Preserved ✅ 532ms
Format Validation.Event Has Timestamp ✅ 533ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 532ms
Retry Behavior.Retries On 503 ❌ 5535ms
Retry Behavior.Does Not Retry On 400 ✅ 2535ms
Retry Behavior.Does Not Retry On 401 ✅ 2534ms
Retry Behavior.Respects Retry After Header ❌ 5534ms
Retry Behavior.Implements Backoff ❌ 15550ms
Retry Behavior.Retries On 500 ❌ 5537ms
Retry Behavior.Retries On 502 ❌ 5540ms
Retry Behavior.Retries On 504 ❌ 5538ms
Retry Behavior.Max Retries Respected ❌ 15547ms
Deduplication.Generates Unique Uuids ✅ 541ms
Deduplication.Preserves Uuid On Retry ❌ 5535ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 10543ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 5542ms
Deduplication.No Duplicate Events In Batch ✅ 538ms
Deduplication.Different Events Have Different Uuids ✅ 533ms
Compression.Sends Gzip When Enabled ✅ 534ms
Batch Format.Uses Proper Batch Structure ✅ 532ms
Batch Format.Flush With No Events Sends Nothing ✅ 518ms
Batch Format.Multiple Events Batched Together ✅ 521ms
Error Handling.Does Not Retry On 403 ✅ 2534ms
Error Handling.Does Not Retry On 413 ✅ 2534ms
Error Handling.Retries On 408 ❌ 5536ms

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 ✅ 522ms
Request Payload.Flags Request Uses V2 Query Param ✅ 522ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 523ms
Request Payload.Flags Request Omits Authorization Header ✅ 521ms
Request Payload.Token In Flags Body Matches Init ✅ 522ms
Request Payload.Groups Round Trip ✅ 522ms
Request Payload.Groups Default To Empty Object ✅ 522ms
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 ✅ 518ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 525ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 523ms
Retry Behavior.Retries Flags On 502 ✅ 625ms
Retry Behavior.Retries Flags On 504 ✅ 625ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 533ms

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-10-21T17:28:04.715691+00:00
Duration: 116679ms

⚠️ 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 ✅ 29ms
Format Validation.Event Has Uuid ✅ 522ms
Format Validation.Event Has Lib Properties ✅ 526ms
Format Validation.Distinct Id Is String ✅ 525ms
Format Validation.Token Is Present ✅ 524ms
Format Validation.Custom Properties Preserved ✅ 525ms
Format Validation.Event Has Timestamp ✅ 525ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 527ms
Retry Behavior.Retries On 503 ✅ 5833ms
Retry Behavior.Does Not Retry On 400 ✅ 2526ms
Retry Behavior.Does Not Retry On 401 ✅ 2527ms
Retry Behavior.Respects Retry After Header ❌ 6334ms
Retry Behavior.Implements Backoff ✅ 16248ms
Retry Behavior.Retries On 500 ✅ 5634ms
Retry Behavior.Retries On 502 ✅ 5631ms
Retry Behavior.Retries On 504 ✅ 5633ms
Retry Behavior.Max Retries Respected ✅ 17039ms
Deduplication.Generates Unique Uuids ✅ 245ms
Deduplication.Preserves Uuid On Retry ✅ 5633ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10840ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5637ms
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 ✅ 519ms
Batch Format.Multiple Events Batched Together ✅ 514ms
Error Handling.Does Not Retry On 403 ✅ 2526ms
Error Handling.Does Not Retry On 413 ✅ 2527ms
Error Handling.Retries On 408 ✅ 5631ms

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 ✅ 522ms
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 ✅ 523ms
Request Payload.Groups Round Trip ✅ 522ms
Request Payload.Groups Default To Empty Object ✅ 522ms
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 ✅ 524ms
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 ✅ 526ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 524ms
Retry Behavior.Retries Flags On 502 ✅ 625ms
Retry Behavior.Retries Flags On 504 ✅ 625ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 524ms

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-10-21T17:28:57.118606+00:00
Duration: 149228ms

⚠️ 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 ✅ 28ms
Format Validation.Event Has Uuid ✅ 522ms
Format Validation.Event Has Lib Properties ✅ 524ms
Format Validation.Distinct Id Is String ✅ 525ms
Format Validation.Token Is Present ✅ 524ms
Format Validation.Custom Properties Preserved ✅ 525ms
Format Validation.Event Has Timestamp ✅ 524ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 524ms
Retry Behavior.Retries On 503 ❌ 9233ms
Retry Behavior.Does Not Retry On 400 ✅ 2529ms
Retry Behavior.Does Not Retry On 401 ✅ 2526ms
Retry Behavior.Respects Retry After Header ❌ 9235ms
Retry Behavior.Implements Backoff ❌ 19233ms
Retry Behavior.Retries On 500 ❌ 8749ms
Retry Behavior.Retries On 502 ❌ 9237ms
Retry Behavior.Retries On 504 ❌ 9237ms
Retry Behavior.Max Retries Respected ❌ 19246ms
Deduplication.Generates Unique Uuids ✅ 534ms
Deduplication.Preserves Uuid On Retry ❌ 9235ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 14241ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 9241ms
Deduplication.No Duplicate Events In Batch ✅ 533ms
Deduplication.Different Events Have Different Uuids ✅ 525ms
Compression.Sends Gzip When Enabled ✅ 524ms
Batch Format.Uses Proper Batch Structure ✅ 524ms
Batch Format.Flush With No Events Sends Nothing ✅ 520ms
Batch Format.Multiple Events Batched Together ✅ 514ms
Error Handling.Does Not Retry On 403 ✅ 2527ms
Error Handling.Does Not Retry On 413 ✅ 2527ms
Error Handling.Retries On 408 ❌ 5529ms

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 ✅ 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 ✅ 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 ✅ 527ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 524ms
Retry Behavior.Retries Flags On 502 ✅ 626ms
Retry Behavior.Retries Flags On 504 ✅ 626ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 526ms

@marandaneto
marandaneto marked this pull request as ready for review October 21, 2026 17:15
@marandaneto
marandaneto requested a review from a team as a code owner October 21, 2026 17:15
@marandaneto
marandaneto merged commit 2af3838 into main Oct 22, 2026
28 checks passed
@marandaneto
marandaneto deleted the fix/ingestion-transport-timeouts 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