Skip to content

feat(surveys): add displaySurvey to show a survey on demand#730

Draft
bs1180 wants to merge 1 commit into
mainfrom
posthog-code/display-survey-manual-api
Draft

feat(surveys): add displaySurvey to show a survey on demand#730
bs1180 wants to merge 1 commit into
mainfrom
posthog-code/display-survey-manual-api

Conversation

@bs1180

@bs1180 bs1180 commented Jul 20, 2026

Copy link
Copy Markdown

💡 Motivation and Context

Closes #648

The web SDK exposes posthog.displaySurvey() to display a survey on demand (see implementing custom surveys), but there is no mobile equivalent, so surveys can only be shown when the automatic display conditions are met, and API-type surveys can't be shown at all. This adds the iOS counterpart:

PostHogSDK.shared.displaySurvey("survey-id")
  • Looks the survey up in the loaded surveys and routes it through the existing display path (translations, active-survey tracking, survey shown/sent/dismissed events), so all downstream behavior is unchanged.
  • Bypasses display conditions (targeting flags, event triggers, seen/wait-period checks), matching the web SDK's default behavior.
  • Also enables API-type surveys, which showNextSurvey() intentionally never auto-displays.
  • No-op (with a log) when the survey isn't found or another survey is already being displayed.

showNextSurvey() was refactored to share the new presentSurvey(_:) helper; its behavior is unchanged.

Sibling PRs: posthog-android and posthog-flutter (Flutter delegates survey logic to the native SDKs, so it needs this API first). RN is tracked in PostHog/posthog-js#3817.

💚 How did you test it?

Added PostHogSurveyDisplaySurveyTest covering: displaying an API-type survey by ID, bypassing an unfired event trigger, unknown ID as a no-op, and ignoring the call while another survey is active. Written to match the existing survey test harness, but not executed locally (authored in a Linux environment without an Apple toolchain) — relying on CI here.

📝 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 changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Authored with PostHog Code (Claude Code) from a request to bring the JS SDK's displaySurvey to the Flutter SDK; since Flutter's survey pipeline lives in the native SDKs, this iOS API is the prerequisite. Design choice: reuse the internal display path via a shared presentSurvey(_:) helper rather than a parallel code path, and keep v1 to a single surveyId parameter (no ignoreConditions/prefill options yet) to match the tracking issue's scope. Tests assert on integration state via the existing TESTING extension instead of rendering UI.


Created with PostHog Code

Adds PostHogSDK.shared.displaySurvey(surveyId), the mobile counterpart of the web SDK's posthog.displaySurvey(). It looks the survey up in the loaded surveys and routes it through the existing display path (translations, active-survey tracking, shown/sent/dismissed events), bypassing display conditions such as targeting flags, event triggers, and the seen/wait-period checks. This also enables API-type surveys, which are never auto-displayed.

Generated-By: PostHog Code
Task-Id: 96def33a-2460-42b7-9690-0a75ae8383fe
@bs1180
bs1180 requested review from ioannisj and turnipdabeets July 20, 2026 13:20

bs1180 commented Jul 20, 2026

Copy link
Copy Markdown
Author

Hi @ioannisj @turnipdabeets 👋 — tagging you both since you maintain the surveys code here. This adds the manual displaySurvey API tracked in #648, as the first piece needed for Flutter support (PostHog/posthog-flutter#225). No rush at all, and happy to adjust the API shape (e.g. add an ignoreConditions option or restrict allowed survey types) if you'd prefer something different. Thanks for taking a look!

@github-actions

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-07-20 13:31:18 UTC
Duration: 228555ms

⚠️ Some Tests Failed

44/45 tests passed, 1 failed


Capture Tests

⚠️ 28/29 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 4364ms
Format Validation.Event Has Uuid 2823ms
Format Validation.Event Has Lib Properties 3109ms
Format Validation.Distinct Id Is String 3332ms
Format Validation.Token Is Present 3228ms
Format Validation.Custom Properties Preserved 2814ms
Format Validation.Event Has Timestamp 443ms
Retry Behavior.Retries On 503 11790ms
Retry Behavior.Does Not Retry On 400 4892ms
Retry Behavior.Does Not Retry On 401 5003ms
Retry Behavior.Respects Retry After Header 7825ms
Retry Behavior.Implements Backoff 15566ms
Retry Behavior.Retries On 500 8835ms
Retry Behavior.Retries On 502 8866ms
Retry Behavior.Retries On 504 9305ms
Retry Behavior.Max Retries Respected 21768ms
Deduplication.Generates Unique Uuids 2962ms
Deduplication.Preserves Uuid On Retry 9270ms
Deduplication.Preserves Uuid And Timestamp On Retry 16538ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 8013ms
Deduplication.No Duplicate Events In Batch 3349ms
Deduplication.Different Events Have Different Uuids 546ms
Compression.Sends Gzip When Enabled 663ms
Batch Format.Uses Proper Batch Structure 2960ms
Batch Format.Flush With No Events Sends Nothing 437ms
Batch Format.Multiple Events Batched Together 3253ms
Error Handling.Does Not Retry On 403 4900ms
Error Handling.Does Not Retry On 413 4866ms
Error Handling.Retries On 408 9207ms

Failures

deduplication.different_events_have_different_uuids

Need at least 2 events

Feature_Flags Tests

16/16 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 2848ms
Request Payload.Flags Request Uses V2 Query Param 2832ms
Request Payload.Flags Request Hits Flags Path Not Decide 2783ms
Request Payload.Flags Request Omits Authorization Header 2782ms
Request Payload.Token In Flags Body Matches Init 2850ms
Request Payload.Groups Round Trip 2911ms
Request Payload.Groups Default To Empty Object 2829ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It 2770ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 2764ms
Request Payload.Disable Geoip Omitted Defaults To False 2684ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 2822ms
Request Lifecycle.No Flags Request On Init Alone 67ms
Request Lifecycle.No Flags Request On Normal Capture 2887ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 5371ms
Request Lifecycle.Mock Response Value Is Returned To Caller 2935ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 2913ms

@posthog

posthog Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🦔 ReviewHog reviewed this pull request

Found 2 must fix, 0 should fix, 0 consider.

Published 2 findings (view the review).

@posthog

posthog Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

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

ReviewHog Report

Changes

Issues: 2 issues

Files (5)
  • .changeset/display-survey-manual-api.md
  • PostHog.xcodeproj/project.pbxproj
  • PostHog/PostHogSDK.swift
  • PostHog/Surveys/PostHogSurveyIntegration.swift
  • PostHogTests/PostHogSurveyDisplaySurveyTest.swift

Comment on lines +310 to +331
func displaySurvey(surveyId: String) {
#if os(iOS)
guard #available(iOS 15.0, *) else {
hedgeLog("[Surveys] Surveys can be rendered only on iOS 15+")
return
}
#endif

guard canShowNextSurvey() else {
hedgeLog("[Surveys] Cannot display survey \(surveyId) - another survey is already being displayed")
return
}

getSurveys { [weak self] surveys in
guard let self else { return }
guard let survey = surveys.first(where: { $0.id == surveyId }) else {
hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey not found")
return
}
self.presentSurvey(survey)
}
}

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.

displaySurvey doesn't verify the survey is running, contradicting its documented contract

must_fix bug

Why we think it's a valid issue
  • Checked: getSurveys (PostHogSurveyIntegration.swift:166-189) and its terminal decodeAndSetSurveys (231-252) — they call back with the raw decoded loadedSurveys and apply no active/running filter. Compared against getActiveMatchingSurveys (98-143) and displaySurvey (310-331).
  • Found: getActiveMatchingSurveys applies .filter(\.isActive) at line 110; displaySurvey looks the survey up with surveys.first(where: { $0.id == surveyId }) at line 325 and calls presentSurvey directly at 329, never checking isActive. isActive is startDate != nil && endDate == nil (987-989). The public doc for displaySurvey states "The survey must be running and present in the project's surveys" (PostHogSDK.swift:2474) — the "present" half is enforced by the not-found guard (326), the "running" half is not enforced at all.
  • Found: The surveys pipeline reference confirms the backend (SurveyViewSet) returns ALL non-archived surveys and does no server-side eligibility filtering — stopping a survey sets end_date without archiving, so stopped surveys stay in the cached payload. isSurveyRunning (has start_date, no end_date) is the web SDK's first eligibility gate, a baseline check distinct from the display conditions (targeting_flag, event triggers, seen/wait) that displaySurvey intentionally bypasses. isActive here is the exact iOS equivalent.
  • Impact: Confirmed reachable, not speculative: calling displaySurvey(surveyId:) with the ID of a survey that has been stopped on the dashboard (end_date set) renders it and collects survey shown/sent responses, violating the method's own documented "must be running" precondition and diverging from the running/active baseline every other SDK enforces. Contract break + correctness defect on a brand-new public API, with a one-line fix that mirrors the existing automatic path.
Issue description

The public doc comment on PostHogSDK.displaySurvey (PostHog/PostHogSDK.swift:2474) explicitly states "The survey must be running", and the PR description enumerates exactly which conditions are bypassed — targeting flags, event triggers, and seen/wait-period checks — deliberately not including the survey's running/active state. However, PostHogSurveyIntegration.displaySurvey(surveyId:) looks the survey up directly in the raw, unfiltered getSurveys result (line 325: surveys.first(where: { $0.id == surveyId })) and never applies the isActive check (startDate != nil && endDate == nil, defined at line 987) that getActiveMatchingSurveys applies for automatic display (line 110: .filter(\.isActive)). That client-side filter only exists because the remote config payload can contain surveys that have already ended (endDate set) — otherwise there would be no need to filter it out for the automatic path. As written, calling displaySurvey(surveyId:) with the ID of a survey that has been stopped/archived on the PostHog dashboard (e.g. pulled for compliance, legal, or content-accuracy reasons) will still render it and collect responses, directly contradicting the documented contract and the intent of the automatic-display pipeline it otherwise mirrors.

Suggested fix

Apply the same survey.isActive check used by getActiveMatchingSurveys before presenting in displaySurvey, e.g.:

guard let survey = surveys.first(where: { $0.id == surveyId }) else {
    hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey not found")
    return
}
guard survey.isActive else {
    hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey is not running")
    return
}
self.presentSurvey(survey)

This keeps the intentionally-bypassed conditions (targeting flags, event triggers, wait period, seen state) bypassed while still honoring the documented "must be running" precondition, and add a test case asserting that a survey with an endDate set is rejected by displaySurvey.

Prompt to fix with AI (copy-paste)
## Context
@PostHog/Surveys/PostHogSurveyIntegration.swift#L310-331

<issue_description>
The public doc comment on `PostHogSDK.displaySurvey` (PostHog/PostHogSDK.swift:2474) explicitly states "The survey must be running", and the PR description enumerates exactly which conditions are bypassed — targeting flags, event triggers, and seen/wait-period checks — deliberately not including the survey's running/active state. However, `PostHogSurveyIntegration.displaySurvey(surveyId:)` looks the survey up directly in the raw, unfiltered `getSurveys` result (line 325: `surveys.first(where: { $0.id == surveyId })`) and never applies the `isActive` check (`startDate != nil && endDate == nil`, defined at line 987) that `getActiveMatchingSurveys` applies for automatic display (line 110: `.filter(\.isActive)`). That client-side filter only exists because the remote config payload can contain surveys that have already ended (endDate set) — otherwise there would be no need to filter it out for the automatic path. As written, calling `displaySurvey(surveyId:)` with the ID of a survey that has been stopped/archived on the PostHog dashboard (e.g. pulled for compliance, legal, or content-accuracy reasons) will still render it and collect responses, directly contradicting the documented contract and the intent of the automatic-display pipeline it otherwise mirrors.
</issue_description>

<issue_validation>
- **Checked:** `getSurveys` (PostHogSurveyIntegration.swift:166-189) and its terminal `decodeAndSetSurveys` (231-252) — they call back with the raw decoded `loadedSurveys` and apply no active/running filter. Compared against `getActiveMatchingSurveys` (98-143) and `displaySurvey` (310-331).
- **Found:** `getActiveMatchingSurveys` applies `.filter(\.isActive)` at line 110; `displaySurvey` looks the survey up with `surveys.first(where: { $0.id == surveyId })` at line 325 and calls `presentSurvey` directly at 329, never checking `isActive`. `isActive` is `startDate != nil && endDate == nil` (987-989). The public doc for `displaySurvey` states "The survey must be running and present in the project's surveys" (PostHogSDK.swift:2474) — the "present" half is enforced by the not-found guard (326), the "running" half is not enforced at all.
- **Found:** The surveys pipeline reference confirms the backend (`SurveyViewSet`) returns ALL non-archived surveys and does no server-side eligibility filtering — stopping a survey sets `end_date` without archiving, so stopped surveys stay in the cached payload. `isSurveyRunning` (has `start_date`, no `end_date`) is the web SDK's *first* eligibility gate, a baseline check distinct from the display conditions (`targeting_flag`, event triggers, seen/wait) that `displaySurvey` intentionally bypasses. `isActive` here is the exact iOS equivalent.
- **Impact:** Confirmed reachable, not speculative: calling `displaySurvey(surveyId:)` with the ID of a survey that has been stopped on the dashboard (end_date set) renders it and collects `survey shown`/`sent` responses, violating the method's own documented "must be running" precondition and diverging from the running/active baseline every other SDK enforces. Contract break + correctness defect on a brand-new public API, with a one-line fix that mirrors the existing automatic path.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Apply the same `survey.isActive` check used by `getActiveMatchingSurveys` before presenting in `displaySurvey`, e.g.:
```swift
guard let survey = surveys.first(where: { $0.id == surveyId }) else {
    hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey not found")
    return
}
guard survey.isActive else {
    hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey is not running")
    return
}
self.presentSurvey(survey)

This keeps the intentionally-bypassed conditions (targeting flags, event triggers, wait period, seen state) bypassed while still honoring the documented "must be running" precondition, and add a test case asserting that a survey with an endDate set is rejected by displaySurvey.
</potential_solution>


</details>

Comment on lines +310 to +331
func displaySurvey(surveyId: String) {
#if os(iOS)
guard #available(iOS 15.0, *) else {
hedgeLog("[Surveys] Surveys can be rendered only on iOS 15+")
return
}
#endif

guard canShowNextSurvey() else {
hedgeLog("[Surveys] Cannot display survey \(surveyId) - another survey is already being displayed")
return
}

getSurveys { [weak self] surveys in
guard let self else { return }
guard let survey = surveys.first(where: { $0.id == surveyId }) else {
hedgeLog("[Surveys] Cannot display survey \(surveyId) - survey not found")
return
}
self.presentSurvey(survey)
}
}

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.

TOCTOU race between the active-survey check and claim lets a rendered survey diverge from tracked activeSurvey, permanently blocking all future surveys

must_fix bug

Why we think it's a valid issue
  • Checked: setActiveSurvey (751-762), presentSurvey (334-356), canShowNextSurvey (970-972), the three handle* rejection guards (482/509/576), clearActiveSurvey callers (grep: only 595 in production + a TESTING helper at 1111), the default delegate renderSurvey (PostHogSurveysDefaultDelegate.swift:23-50), and the trigger/threading model.
  • Found: setActiveSurvey writes only if activeSurvey == nil (753) under the lock, but presentSurvey calls it at 337 and then unconditionally calls renderSurvey at 344 — the return of the claim is never inspected. The pre-check canShowNextSurvey() in both displaySurvey (318) and showNextSurvey (293) is a separate lock acquisition, with an async getSurveys hop before the claim, so it is a genuine check-then-act across two locks.
  • Found: the divergence is not self-healing: handleSurveyClosed only reaches clearActiveSurvey() (595) when survey.id == activeSurvey.id (576); the default delegate overwrites its close handler (46) and re-renders without guarding, so the loser's survey ends up on screen while activeSurvey holds the winner, whose close event never arrives → activeSurvey never cleared → canShowNextSurvey() returns false for the rest of the process (in-memory state, no timeout, cleanupSurveys doesn't reset it).
  • Impact: Confirmed reachable and severe: a background-thread @objc displaySurvey call (the project mandates any-thread public APIs) overlapping a main-thread automatic trigger, or a startup double-load where two getSurveys cache-miss callbacks land on network threads, lets both flows pass the nil-check before either claims — one renders an untracked survey and permanently, silently disables ALL surveys until app restart. Race corrupting shared state with a silent, unrecoverable, hard-to-diagnose consequence and a cheap, clean fix (render only on a successful atomic claim) — squarely in the keep bar. The window is narrow and the most common usage (main-thread call with surveys already cached) is race-free, but at scale this will occur for some users; the reviewer named both the concrete trigger and the concrete consequence accurately.
Issue description

displaySurvey(surveyId:) calls canShowNextSurvey() (a synchronous read of activeSurvey == nil) and only later, after the async getSurveys round trip, calls presentSurvey(_:), which calls setActiveSurvey(...). setActiveSurvey (line 751) is a 'set only if nil' operation - if another call already claimed the slot, it silently no-ops - but presentSurvey never checks whether its own claim succeeded before unconditionally proceeding to invoke self.postHog?.config.surveysConfig.surveysDelegate.renderSurvey(survey.toDisplaySurvey(...), ...) on line 344. The automatic path showNextSurvey() (line 286) has the exact same check-then-act shape and calls the same shared presentSurvey. Since displaySurvey is a new public, @objc entry point that can be invoked from any thread (per this project's 'all public APIs should be callable from any thread' requirement) while showNextSurvey() is independently triggered from view-layout throttling, app-become-active, and captured-event callbacks, two calls can now race: both pass canShowNextSurvey() before either calls setActiveSurvey, the first call's claim wins, but the second call still renders its survey via the delegate. From then on, handleSurveyShown/handleSurveyResponse/handleSurveyClosed all compare the survey that's actually on screen against the wrong tracked activeSurvey and reject it ('Received a show/response/close event for a non-active survey'), so clearActiveSurvey() is never called for the survey that actually holds the slot. canShowNextSurvey() then returns false for the remainder of the process lifetime, silently disabling all surveys (both automatic and via displaySurvey) until the app is restarted. This is a genuine reliability regression introduced by promoting the previously-inline, single-caller logic into a shared presentSurvey now reachable from two independent, potentially concurrent triggers, and it is not covered by the new test suite (displaySurveyIgnoredWhileAnotherSurveyActive only exercises the case where a survey is already active before the call, not the async race).

Suggested fix

Make the claim-and-render atomic instead of check-then-act-later: have setActiveSurvey (or a new tryClaimActiveSurvey) return a Bool indicating whether it actually won the slot, and only call the delegate's renderSurvey when the claim succeeded; otherwise log and return, mirroring the existing 'another survey is already being displayed' log. For example:

private func presentSurvey(_ survey: PostHogSurvey) {
    let translations = ...
    guard claimActiveSurvey(survey: survey, language: translations.matchedKey, questionTranslations: translations.questions) else {
        hedgeLog("[Surveys] Cannot display survey \(survey.id) - another survey is already being displayed")
        return
    }
    // proceed to render only on a successful claim
}

where claimActiveSurvey performs the nil-check and assignment under activeSurveyLock and returns whether it wrote. This removes the redundant, racy canShowNextSurvey() pre-check in both displaySurvey and showNextSurvey and closes the window entirely.

Prompt to fix with AI (copy-paste)
## Context
@PostHog/Surveys/PostHogSurveyIntegration.swift#L310-331
@PostHog/Surveys/PostHogSurveyIntegration.swift#L333-356

<issue_description>
`displaySurvey(surveyId:)` calls `canShowNextSurvey()` (a synchronous read of `activeSurvey == nil`) and only later, after the async `getSurveys` round trip, calls `presentSurvey(_:)`, which calls `setActiveSurvey(...)`. `setActiveSurvey` (line 751) is a 'set only if nil' operation - if another call already claimed the slot, it silently no-ops - but `presentSurvey` never checks whether its own claim succeeded before unconditionally proceeding to invoke `self.postHog?.config.surveysConfig.surveysDelegate.renderSurvey(survey.toDisplaySurvey(...), ...)` on line 344. The automatic path `showNextSurvey()` (line 286) has the exact same check-then-act shape and calls the same shared `presentSurvey`. Since `displaySurvey` is a new public, `@objc` entry point that can be invoked from any thread (per this project's 'all public APIs should be callable from any thread' requirement) while `showNextSurvey()` is independently triggered from view-layout throttling, app-become-active, and captured-event callbacks, two calls can now race: both pass `canShowNextSurvey()` before either calls `setActiveSurvey`, the first call's claim wins, but the second call still renders its survey via the delegate. From then on, `handleSurveyShown`/`handleSurveyResponse`/`handleSurveyClosed` all compare the survey that's actually on screen against the wrong tracked `activeSurvey` and reject it ('Received a show/response/close event for a non-active survey'), so `clearActiveSurvey()` is never called for the survey that actually holds the slot. `canShowNextSurvey()` then returns `false` for the remainder of the process lifetime, silently disabling all surveys (both automatic and via `displaySurvey`) until the app is restarted. This is a genuine reliability regression introduced by promoting the previously-inline, single-caller logic into a shared `presentSurvey` now reachable from two independent, potentially concurrent triggers, and it is not covered by the new test suite (`displaySurveyIgnoredWhileAnotherSurveyActive` only exercises the case where a survey is already active *before* the call, not the async race).
</issue_description>

<issue_validation>
- **Checked:** `setActiveSurvey` (751-762), `presentSurvey` (334-356), `canShowNextSurvey` (970-972), the three `handle*` rejection guards (482/509/576), `clearActiveSurvey` callers (grep: only 595 in production + a TESTING helper at 1111), the default delegate `renderSurvey` (PostHogSurveysDefaultDelegate.swift:23-50), and the trigger/threading model.
- **Found:** `setActiveSurvey` writes only `if activeSurvey == nil` (753) under the lock, but `presentSurvey` calls it at 337 and then unconditionally calls `renderSurvey` at 344 — the return of the claim is never inspected. The pre-check `canShowNextSurvey()` in both `displaySurvey` (318) and `showNextSurvey` (293) is a separate lock acquisition, with an async `getSurveys` hop before the claim, so it is a genuine check-then-act across two locks.
- **Found:** the divergence is not self-healing: `handleSurveyClosed` only reaches `clearActiveSurvey()` (595) when `survey.id == activeSurvey.id` (576); the default delegate overwrites its close handler (46) and re-renders without guarding, so the loser's survey ends up on screen while `activeSurvey` holds the winner, whose close event never arrives → `activeSurvey` never cleared → `canShowNextSurvey()` returns false for the rest of the process (in-memory state, no timeout, `cleanupSurveys` doesn't reset it).
- **Impact:** Confirmed reachable and severe: a background-thread `@objc displaySurvey` call (the project mandates any-thread public APIs) overlapping a main-thread automatic trigger, or a startup double-load where two `getSurveys` cache-miss callbacks land on network threads, lets both flows pass the nil-check before either claims — one renders an untracked survey and permanently, silently disables ALL surveys until app restart. Race corrupting shared state with a silent, unrecoverable, hard-to-diagnose consequence and a cheap, clean fix (render only on a successful atomic claim) — squarely in the keep bar. The window is narrow and the most common usage (main-thread call with surveys already cached) is race-free, but at scale this will occur for some users; the reviewer named both the concrete trigger and the concrete consequence accurately.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Make the claim-and-render atomic instead of check-then-act-later: have `setActiveSurvey` (or a new `tryClaimActiveSurvey`) return a Bool indicating whether it actually won the slot, and only call the delegate's `renderSurvey` when the claim succeeded; otherwise log and return, mirroring the existing 'another survey is already being displayed' log. For example:
```swift
private func presentSurvey(_ survey: PostHogSurvey) {
    let translations = ...
    guard claimActiveSurvey(survey: survey, language: translations.matchedKey, questionTranslations: translations.questions) else {
        hedgeLog("[Surveys] Cannot display survey \(survey.id) - another survey is already being displayed")
        return
    }
    // proceed to render only on a successful claim
}

where claimActiveSurvey performs the nil-check and assignment under activeSurveyLock and returns whether it wrote. This removes the redundant, racy canShowNextSurvey() pre-check in both displaySurvey and showNextSurvey and closes the window entirely.
</potential_solution>


</details>

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.

Support manual API displaySurvey

1 participant