Skip to content

fix: accept plain closure arrays in setBeforeSend without a runtime trap - #974

Merged
turnipdabeets merged 1 commit into
chore/v4-remove-deprecatedfrom
fix/v4-setbeforesend-plain-array
Oct 27, 2026
Merged

turnipdabeets merged 1 commit into
chore/v4-remove-deprecatedfrom
fix/v4-setbeforesend-plain-array

Conversation

@turnipdabeets

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Addresses ioannisj's blocking review on #873 (#873).

Making BeforeSendBlock and PostHogBeforeSendLogBlock @Sendable broke Swift 5 apps that pass a stored array typed with the plain function type to setBeforeSend(_:):

let blocks: [(PostHogEvent) -> PostHogEvent?] = [...]
config.setBeforeSend(blocks) // compiles with a "may introduce data races" warning, then traps

The array is force-cast to the @Sendable element type at runtime and traps with Could not cast value of type '(PostHogEvent) -> Optional<PostHogEvent>' to '@Sendable …'.

This adds a @_disfavoredOverload setBeforeSend(_:) taking [(PostHogEvent) -> PostHogEvent?] on PostHogConfig and [(PostHogLogRecord) -> PostHogLogRecord?] on PostHogLogsConfig. Each element is wrapped in a small internal @unchecked Sendable box and forwarded to the existing @Sendable overload, so no array cast happens. The new overloads are Swift-only (not @objc); the Objective-C surface is unchanged.

The 4.0 changeset no longer tells users to retype their arrays as [BeforeSendBlock], since that workaround isn't needed anymore.

💚 How did you test it?

  • Reproduced the trap first: the new regression test crashed make test with signal 6 and the cast message above before the fix.
  • Added regression tests for the plain-array case on both configs (PostHogConfigTest, PostHogLogsQueueTest); they pass after the fix and the data-race warning is gone.
  • Checked overload resolution in a scratch package (library in Swift 5 mode, one client in Swift 5 mode and one in Swift 6 mode):
Call Swift 5 client Swift 6 client
Literal setBeforeSend([{ $0 }]) @Sendable overload @Sendable overload
[BeforeSendBlock] variable @Sendable overload @Sendable overload
Variadic setBeforeSend({ $0 }, { $0 }) @Sendable overload @Sendable overload
[(Event) -> Event?] variable, before fix warning, then runtime trap compile error
[(Event) -> Event?] variable, after fix new overload, no warning, no trap new overload, no diagnostic
Literal capturing non-Sendable state, after fix n/a still a compile error (doesn't fall back to the new overload)
  • make lint, make test and make buildSdk (see Agent context for a worktree caveat).

📝 Checklist

  • All commits are signed (see Commit signing)
  • 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 changeset to generate a changeset file

Edited the existing .changeset/v4-major-release.md instead of adding a new changeset.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Written with Claude Code (Opus 5.5). Followed the reviewer's suggested approach as-is: @_disfavoredOverload plus a per-element box. The box is a private generic class next to BeforeSendChain, shared by events and logs.
  • In Swift 6 mode the new overload accepts an explicitly typed non-Sendable array without a diagnostic. That's the intended escape hatch; closure literals are still checked.
  • api/posthog-ios.public-api.txt: only the two new method lines were added. Running make apiUpdate locally with Xcode 26.4.1 also rewrote unrelated lines to show @Sendable, while CI's latest-stable Xcode passes on the current rendering, so those lines were left alone. make apiUpdate and make buildSdk had to run from a copy of the worktree in a directory named posthog-ios, because the example projects reference ../../posthog-ios.
  • Local results: make lint and make buildSdk pass, and the focused beforeSend tests pass (20 tests). Full make test ran 1155 tests and reported 14 issues, none in code this PR touches. 13 are PostHogStorage app-group tests that fail on this machine because macOS denies writes under ~/Library/Group Containers; they fail the same way on the base branch. The other is the timing-sensitive PostHogLogsQueue "add() racing with flush()" test, which also fails on the base branch on this loaded machine.

🤖 Generated with Claude Code

Passing a [(PostHogEvent) -> PostHogEvent?] (or the logs equivalent) to setBeforeSend(_:) force-cast the array to the @sendable element type and trapped at runtime in Swift 5 mode. Add disfavored overloads for the plain function types that box each element and forward, so no cast happens. Literals, variadic calls and [BeforeSendBlock] arrays still resolve to the @sendable overloads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@turnipdabeets turnipdabeets self-assigned this Oct 27, 2026
@turnipdabeets turnipdabeets mentioned this pull request Oct 27, 2026
2 of 15 tasks
@github-actions

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-10-27T17:10:00.033497+00:00
Duration: 243056ms

⚠️ 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 ✅ 3106ms
Format Validation.Event Has Uuid ✅ 3001ms
Format Validation.Event Has Lib Properties ✅ 2909ms
Format Validation.Distinct Id Is String ✅ 3090ms
Format Validation.Token Is Present ✅ 3008ms
Format Validation.Custom Properties Preserved ✅ 2987ms
Format Validation.Event Has Timestamp ✅ 3002ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 2865ms
Retry Behavior.Retries On 503 ✅ 11889ms
Retry Behavior.Does Not Retry On 400 ✅ 5160ms
Retry Behavior.Does Not Retry On 401 ✅ 5022ms
Retry Behavior.Respects Retry After Header ✅ 8124ms
Retry Behavior.Implements Backoff ✅ 21886ms
Retry Behavior.Retries On 500 ✅ 9307ms
Retry Behavior.Retries On 502 ✅ 9443ms
Retry Behavior.Retries On 504 ✅ 5406ms
Retry Behavior.Max Retries Respected ❌ 21849ms
Deduplication.Generates Unique Uuids ✅ 3162ms
Deduplication.Preserves Uuid On Retry ✅ 9261ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 16858ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 7928ms
Deduplication.No Duplicate Events In Batch ✅ 3088ms
Deduplication.Different Events Have Different Uuids ✅ 2933ms
Compression.Sends Gzip When Enabled ✅ 2979ms
Batch Format.Uses Proper Batch Structure ✅ 2878ms
Batch Format.Flush With No Events Sends Nothing ✅ 341ms
Batch Format.Multiple Events Batched Together ✅ 3061ms
Error Handling.Does Not Retry On 403 ✅ 2429ms
Error Handling.Does Not Retry On 413 ✅ 4882ms
Error Handling.Retries On 408 ✅ 5575ms

Failures

retry_behavior.max_retries_respected

Expected 4 requests, got 6

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 2853ms
Request Payload.Flags Request Uses V2 Query Param ✅ 2967ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 2895ms
Request Payload.Flags Request Omits Authorization Header ✅ 2961ms
Request Payload.Token In Flags Body Matches Init ✅ 3116ms
Request Payload.Groups Round Trip ✅ 3002ms
Request Payload.Groups Default To Empty Object ✅ 3003ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 3000ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 2937ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 2951ms
Request Lifecycle.No Flags Request On Init Alone ✅ 111ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 2906ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 5816ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 2895ms
Retry Behavior.Retries Flags On 502 ✅ 2887ms
Retry Behavior.Retries Flags On 504 ✅ 2948ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 3217ms

@turnipdabeets
turnipdabeets marked this pull request as ready for review October 27, 2026 17:20
@turnipdabeets
turnipdabeets requested a review from a team as a code owner October 27, 2026 17:20
@turnipdabeets turnipdabeets mentioned this pull request Oct 27, 2026
4 of 6 tasks

@dustinbyrne dustinbyrne 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.

The element-wise adaptation fixes stored plain closure arrays while preserving preferred Sendable overloads and existing hook behavior. No actionable findings.

AI-assisted review.

@turnipdabeets
turnipdabeets merged commit 9a89c32 into chore/v4-remove-deprecated Oct 27, 2026
50 checks passed
@turnipdabeets
turnipdabeets deleted the fix/v4-setbeforesend-plain-array branch October 27, 2026 22:06
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