Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
bs1180 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/display-survey-manual-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"posthog": minor
"posthog-android": minor
---

Add `PostHog.displaySurvey(surveyId)` to display a survey on demand, bypassing display conditions (targeting flags, event triggers, and seen/wait-period checks). This is the mobile counterpart of the web SDK's `posthog.displaySurvey()` and also enables API-type surveys, which are never auto-displayed.
1 change: 1 addition & 0 deletions posthog-android/api/posthog-android.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public class com/posthog/android/replay/internal/LogcatParser {

public final class com/posthog/android/surveys/PostHogSurveysIntegration : com/posthog/PostHogIntegration, com/posthog/internal/surveys/PostHogSurveysHandler {
public fun <init> (Landroid/content/Context;Lcom/posthog/PostHogConfig;)V
public fun displaySurvey (Ljava/lang/String;)V
public fun install (Lcom/posthog/PostHogInterface;)V
public fun onEvent (Ljava/lang/String;Ljava/util/Map;)V
public fun onRemoteConfig (Z)V
Expand Down
29 changes: 29 additions & 0 deletions posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,35 @@ public class PostHogSurveysIntegration(
}
}

/**
* Displays the survey with the given ID on demand.
*
* Unlike automatic display, this bypasses display conditions (targeting flags,
* event triggers, and the seen/wait-period checks), so it also works for
* API-type surveys, which are never auto-displayed. The survey must be present
* in the surveys loaded from remote config. If another survey is already being
* displayed, this call is ignored.
*
* @param surveyId The ID of the survey to display
*/
public override fun displaySurvey(surveyId: String) {
if (!config.surveys) {
config.logger.log("[Surveys] Cannot display survey $surveyId - surveys are disabled in the config")
return
}
val isIntegrationStarted = synchronized(lifecycleLock) { isStarted }
if (!isIntegrationStarted) {
config.logger.log("[Surveys] Cannot display survey $surveyId - surveys integration is not started")
return
}
val survey = synchronized(surveysLock) { cachedSurveys.firstOrNull { it.id == surveyId } }
if (survey == null) {
config.logger.log("[Surveys] Cannot display survey $surveyId - survey not found")
return
}
showSurvey(survey)
}
Comment on lines +151 to +167

@posthog posthog Bot Jul 20, 2026

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 currently running, despite the documented contract requiring it

should_fix bug

Why we think it's a valid issue
  • Checked: the public contract (PostHogInterface.kt:327-340), the implementation (PostHogSurveysIntegration.kt:151-167), the auto-display filter (getActiveMatchingSurveys, lines 255-306), how cachedSurveys is populated (line 128 ← PostHog.kt:299 ← PostHogRemoteConfig.getSurveys() at PostHogRemoteConfig.kt:1243-1247), the Survey model (Survey.kt:26-29), and the new test fixtures.
  • Found: PostHogInterface.kt:330 explicitly documents "The survey must be running ...; targeting flags, event triggers, and the seen/wait-period checks are bypassed" — enumerating only three bypassed conditions and requiring running. But displaySurvey (line 161) resolves the survey solely by cachedSurveys.firstOrNull { it.id == surveyId } and calls showSurvey with no startDate/endDate guard.
  • Found: getActiveMatchingSurveys applies if (survey.startDate == null || survey.endDate != null) return@filter false as its very first check (line 260, commented "must have start date and no end date") against the same cachedSurveys list. That guard existing on the auto-display path is direct in-repo evidence that the raw cache is expected to contain non-running (not-yet-started or ended) surveys — so the premise is not speculative.
  • Found: cachedSurveys (line 128) is assigned the unfiltered list from onSurveysLoaded, which receives remoteConfig.getSurveys() verbatim; no date filtering happens anywhere on that path.
  • Impact: displaySurvey("ended-or-draft-id") for a survey still present in the cache will render it and fire survey shown/sent/dismissed events, collecting responses for a stopped/unlaunched campaign — contradicting the method's own documented "must be running" contract and polluting that survey's response data past its intended end date. This is a real correctness/contract gap on newly added code with a trivial one-line fix (mirror the getActiveMatchingSurveys running check), and the new test suite (createSurvey always uses startDate = Date(), endDate = null) never exercises it. It clears the keep bar: concrete trigger and concrete consequence, not overengineering, style, or defensive paranoia.
Issue description

The public contract in PostHogInterface.kt (lines 327-341) explicitly states: "The survey must be running and returned by the project's remote config; targeting flags, event triggers, and the seen/wait-period checks are bypassed." The PR description and changeset also scope the bypass to only "targeting flags, event triggers, and seen/wait-period checks" — the active-window (running) requirement is not listed among the bypassed conditions.

However, the actual implementation of displaySurvey only checks that the survey ID exists in cachedSurveys (cachedSurveys.firstOrNull { it.id == surveyId }, line 161) — it never checks survey.startDate/survey.endDate. Compare this with getActiveMatchingSurveys() (lines 255-306), which explicitly filters if (survey.startDate == null || survey.endDate != null) return@filter false as its very first "is this survey running" check before any of the other display-condition checks that displaySurvey intentionally bypasses. cachedSurveys itself is the raw, unfiltered list pushed straight from PostHogRemoteConfig.getSurveys() (confirmed in PostHog.kt line 299 and PostHogRemoteConfig.kt line 1243-1247), so it can legitimately contain surveys that haven't started yet (no startDate) or have already ended (endDate set) — the existence of the dedicated filter in getActiveMatchingSurveys is proof that such surveys are present in the cache.

As written, calling PostHog.displaySurvey("ended-survey-id") for a survey whose endDate has already passed (e.g. a closed campaign) will still render it and fire survey shown/survey sent/survey dismissed events for it, contradicting the documented "must be running" requirement and potentially polluting the closed survey's response data after its intended end date. The new PostHogSurveysDisplaySurveyTest doesn't cover this: every survey fixture in that test file is constructed with startDate = Date() and endDate = null (always running), so this gap has no test coverage.

Suggested fix

Add the same "is running" check that getActiveMatchingSurveys() already uses before calling showSurvey(survey), e.g.:

val survey = synchronized(surveysLock) { cachedSurveys.firstOrNull { it.id == surveyId } }
if (survey == null || survey.startDate == null || survey.endDate != null) {
    config.logger.log("[Surveys] Cannot display survey $surveyId - survey not found or not running")
    return
}

Alternatively, if showing not-yet-started/ended surveys on demand is actually intended (e.g. to preview drafts), update the docstring in PostHogInterface.kt (and the changeset) to drop the "must be running" claim so the documented contract matches the real behavior, and add a test exercising an ended/draft survey ID to lock in the intended behavior either way.

Prompt to fix with AI (copy-paste) 
## Context
@posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt#L151-167

<issue_description>
The public contract in PostHogInterface.kt (lines 327-341) explicitly states: "The survey must be running and returned by the project's remote config; targeting flags, event triggers, and the seen/wait-period checks are bypassed." The PR description and changeset also scope the bypass to only "targeting flags, event triggers, and seen/wait-period checks" — the active-window (running) requirement is not listed among the bypassed conditions.

However, the actual implementation of `displaySurvey` only checks that the survey ID exists in `cachedSurveys` (`cachedSurveys.firstOrNull { it.id == surveyId }`, line 161) — it never checks `survey.startDate`/`survey.endDate`. Compare this with `getActiveMatchingSurveys()` (lines 255-306), which explicitly filters `if (survey.startDate == null || survey.endDate != null) return@filter false` as its very first "is this survey running" check before any of the other display-condition checks that `displaySurvey` intentionally bypasses. `cachedSurveys` itself is the raw, unfiltered list pushed straight from `PostHogRemoteConfig.getSurveys()` (confirmed in `PostHog.kt` line 299 and `PostHogRemoteConfig.kt` line 1243-1247), so it can legitimately contain surveys that haven't started yet (no `startDate`) or have already ended (`endDate` set) — the existence of the dedicated filter in `getActiveMatchingSurveys` is proof that such surveys are present in the cache. 

As written, calling `PostHog.displaySurvey("ended-survey-id")` for a survey whose `endDate` has already passed (e.g. a closed campaign) will still render it and fire `survey shown`/`survey sent`/`survey dismissed` events for it, contradicting the documented "must be running" requirement and potentially polluting the closed survey's response data after its intended end date. The new `PostHogSurveysDisplaySurveyTest` doesn't cover this: every survey fixture in that test file is constructed with `startDate = Date()` and `endDate = null` (always running), so this gap has no test coverage.
</issue_description>

<issue_validation>
- **Checked:** the public contract (`PostHogInterface.kt:327-340`), the implementation (`PostHogSurveysIntegration.kt:151-167`), the auto-display filter (`getActiveMatchingSurveys`, lines 255-306), how `cachedSurveys` is populated (line 128 ← `PostHog.kt:299` ← `PostHogRemoteConfig.getSurveys()` at `PostHogRemoteConfig.kt:1243-1247`), the `Survey` model (`Survey.kt:26-29`), and the new test fixtures.
- **Found:** `PostHogInterface.kt:330` explicitly documents "The survey must be running ...; targeting flags, event triggers, and the seen/wait-period checks are bypassed" — enumerating only three bypassed conditions and requiring running. But `displaySurvey` (line 161) resolves the survey solely by `cachedSurveys.firstOrNull { it.id == surveyId }` and calls `showSurvey` with no `startDate`/`endDate` guard.
- **Found:** `getActiveMatchingSurveys` applies `if (survey.startDate == null || survey.endDate != null) return@filter false` as its very first check (line 260, commented "must have start date and no end date") against the *same* `cachedSurveys` list. That guard existing on the auto-display path is direct in-repo evidence that the raw cache is expected to contain non-running (not-yet-started or ended) surveys — so the premise is not speculative.
- **Found:** `cachedSurveys` (line 128) is assigned the unfiltered list from `onSurveysLoaded`, which receives `remoteConfig.getSurveys()` verbatim; no date filtering happens anywhere on that path.
- **Impact:** `displaySurvey("ended-or-draft-id")` for a survey still present in the cache will render it and fire `survey shown`/`sent`/`dismissed` events, collecting responses for a stopped/unlaunched campaign — contradicting the method's own documented "must be running" contract and polluting that survey's response data past its intended end date. This is a real correctness/contract gap on newly added code with a trivial one-line fix (mirror the `getActiveMatchingSurveys` running check), and the new test suite (`createSurvey` always uses `startDate = Date(), endDate = null`) never exercises it. It clears the keep bar: concrete trigger and concrete consequence, not overengineering, style, or defensive paranoia.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Add the same "is running" check that `getActiveMatchingSurveys()` already uses before calling `showSurvey(survey)`, e.g.:

```kotlin
val survey = synchronized(surveysLock) { cachedSurveys.firstOrNull { it.id == surveyId } }
if (survey == null || survey.startDate == null || survey.endDate != null) {
    config.logger.log("[Surveys] Cannot display survey $surveyId - survey not found or not running")
    return
}

Alternatively, if showing not-yet-started/ended surveys on demand is actually intended (e.g. to preview drafts), update the docstring in PostHogInterface.kt (and the changeset) to drop the "must be running" claim so the documented contract matches the real behavior, and add a test exercising an ended/draft survey ID to lock in the intended behavior either way.
</potential_solution>


</details>

Comment on lines +151 to +167

@posthog posthog Bot Jul 20, 2026

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()'s documented 'ignored if another survey is active' guarantee is not enforced atomically

should_fix bug

Why we think it's a valid issue
  • Checked: the guard/claim ordering across showSurvey (line 318-323, 437), canShowNextSurvey (line 683-685), the activeSurvey write in the onSurveyShown callback (lines 341-361), the default delegate (PostHogSurveysDefaultDelegate.renderSurvey), and the surveyPopupDelaySeconds field in the public appearance model.
  • Found (non-atomic claim): showSurvey only reads activeSurvey via canShowNextSurvey() (line 320 → 683-685) and then calls getSurveysDelegate().renderSurvey(...) (line 437). The corresponding write activeSurvey = originalSurvey happens at line 347, but only inside the onSurveyShown lambda that the delegate invokes — never during showSurvey itself. So between renderSurvey being called and the delegate later firing onSurveyShown, activeSurvey stays null and any second showSurvey passes the check. This is a genuine TOCTOU, confirmed by the code, not the snippet.
  • Found (async is the norm, not the exception): onSurveyShown is a "survey became visible" callback fired by delegate UI code after renderSurvey returns; the public surveyPopupDelaySeconds field (PostHogDisplaySurveyAppearance.kt:40, SurveyAppearance.kt:27) explicitly encodes a delay between trigger and display, so the window is architecturally expected. The bundled PostHogSurveysDefaultDelegate renders nothing and never calls onSurveyShown, and its own log says "Implement your own PostHogSurveysDelegate to render surveys" — i.e. every functional deployment uses a custom delegate subject to this window.
  • Found (spurious events): in onSurveyShown, sendSurveyShownEvent(originalSurvey, ...) (line 354) runs unconditionally, outside the if (activeSurvey == null) block (lines 346-351), so two overlapping renders each emit a survey shown (and later dismissed) event even though only one wins the active slot.
  • Impact: displaySurvey is a public API built for uncoordinated app-code invocation (button tap, double-tap, or racing an automatic onEvent/onSurveysLoaded trigger). Two calls landing in the render window both reach renderSurvey, breaking the docstring's "If another survey is already being displayed, this call is ignored" guarantee and polluting analytics with events for surveys the user never saw. Concrete trigger + concrete consequence on newly added code; the suggested fix (claim activeSurvey in the same activeSurvey Lock section as the check) is the standard correct pattern. Clears the keep bar — a race corrupting a documented shared-state invariant, not speculation or paranoia.
  • Priority: keeping should_fix — real invariant break with data-quality/UX impact, but it needs an overlapping-trigger window and yields duplicate events rather than a crash, data loss, or security hole, so must_fix would overstate it.
Issue description

The docstring for displaySurvey (lines 140-150) promises: "If another survey is already being displayed, this call is ignored." That guarantee relies entirely on showSurvey()'s canShowNextSurvey() check (synchronized(activeSurveyLock) { activeSurvey == null }), but activeSurvey is only ever set later, inside the onSurveyShown callback that the delegate invokes asynchronously — potentially after a configurable popup delay (e.g. surveyPopupDelaySeconds in the bundled Compose delegate), or on whatever schedule a custom PostHogSurveysDelegate implementation chooses. Between showSurvey(survey) being called and the delegate eventually invoking onSurveyShown, activeSurvey remains null, so a concurrent second call — another displaySurvey(otherId), or an automatic trigger via onEvent/onSurveysLoaded firing in that same window — also passes canShowNextSurvey() and reaches getSurveysDelegate().renderSurvey(...). Before this PR, only automatic triggers could race each other; displaySurvey is designed to be called from arbitrary, uncoordinated app code (e.g. a button's onClick), which meaningfully raises the odds of hitting this window in production and produces two concurrent renderSurvey() calls, spurious survey shown/survey dismissed events for a survey the user never actually saw, and a violated 'single active survey' invariant.

Suggested fix

Mark the survey as claimed atomically with the decision to render, inside the same activeSurveyLock critical section that performs the canShowNextSurvey() check, instead of deferring that write to the async onSurveyShown callback, e.g.:

internal fun showSurvey(survey: Survey) {
    synchronized(activeSurveyLock) {
        if (activeSurvey != null) {
            config.logger.log("Cannot show survey - another survey is already active")
            return
        }
        activeSurvey = survey
        activeSurveyCompleted = false
        currentSurveyResponses.clear()
    }
    // ... existing setup, with onSurveyShown now just sending the 'survey shown' event
    // instead of also setting activeSurvey
}
Prompt to fix with AI (copy-paste) 
## Context
@posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt#L151-167

<issue_description>
The docstring for `displaySurvey` (lines 140-150) promises: "If another survey is already being displayed, this call is ignored." That guarantee relies entirely on `showSurvey()`'s `canShowNextSurvey()` check (`synchronized(activeSurveyLock) { activeSurvey == null }`), but `activeSurvey` is only ever *set* later, inside the `onSurveyShown` callback that the delegate invokes asynchronously — potentially after a configurable popup delay (e.g. `surveyPopupDelaySeconds` in the bundled Compose delegate), or on whatever schedule a custom `PostHogSurveysDelegate` implementation chooses. Between `showSurvey(survey)` being called and the delegate eventually invoking `onSurveyShown`, `activeSurvey` remains `null`, so a concurrent second call — another `displaySurvey(otherId)`, or an automatic trigger via `onEvent`/`onSurveysLoaded` firing in that same window — also passes `canShowNextSurvey()` and reaches `getSurveysDelegate().renderSurvey(...)`. Before this PR, only automatic triggers could race each other; `displaySurvey` is designed to be called from arbitrary, uncoordinated app code (e.g. a button's onClick), which meaningfully raises the odds of hitting this window in production and produces two concurrent `renderSurvey()` calls, spurious `survey shown`/`survey dismissed` events for a survey the user never actually saw, and a violated 'single active survey' invariant.
</issue_description>

<issue_validation>
- **Checked:** the guard/claim ordering across `showSurvey` (line 318-323, 437), `canShowNextSurvey` (line 683-685), the `activeSurvey` write in the `onSurveyShown` callback (lines 341-361), the default delegate (`PostHogSurveysDefaultDelegate.renderSurvey`), and the `surveyPopupDelaySeconds` field in the public appearance model.
- **Found (non-atomic claim):** `showSurvey` only *reads* `activeSurvey` via `canShowNextSurvey()` (line 320 → 683-685) and then calls `getSurveysDelegate().renderSurvey(...)` (line 437). The corresponding *write* `activeSurvey = originalSurvey` happens at line 347, but only inside the `onSurveyShown` lambda that the delegate invokes — never during `showSurvey` itself. So between `renderSurvey` being called and the delegate later firing `onSurveyShown`, `activeSurvey` stays `null` and any second `showSurvey` passes the check. This is a genuine TOCTOU, confirmed by the code, not the snippet.
- **Found (async is the norm, not the exception):** `onSurveyShown` is a "survey became visible" callback fired by delegate UI code after `renderSurvey` returns; the public `surveyPopupDelaySeconds` field (`PostHogDisplaySurveyAppearance.kt:40`, `SurveyAppearance.kt:27`) explicitly encodes a delay between trigger and display, so the window is architecturally expected. The bundled `PostHogSurveysDefaultDelegate` renders nothing and never calls `onSurveyShown`, and its own log says "Implement your own PostHogSurveysDelegate to render surveys" — i.e. every functional deployment uses a custom delegate subject to this window.
- **Found (spurious events):** in `onSurveyShown`, `sendSurveyShownEvent(originalSurvey, ...)` (line 354) runs unconditionally, outside the `if (activeSurvey == null)` block (lines 346-351), so two overlapping renders each emit a `survey shown` (and later `dismissed`) event even though only one wins the active slot.
- **Impact:** `displaySurvey` is a public API built for uncoordinated app-code invocation (button tap, double-tap, or racing an automatic `onEvent`/`onSurveysLoaded` trigger). Two calls landing in the render window both reach `renderSurvey`, breaking the docstring's "If another survey is already being displayed, this call is ignored" guarantee and polluting analytics with events for surveys the user never saw. Concrete trigger + concrete consequence on newly added code; the suggested fix (claim `activeSurvey` in the same `activeSurvey Lock` section as the check) is the standard correct pattern. Clears the keep bar — a race corrupting a documented shared-state invariant, not speculation or paranoia.
- **Priority:** keeping `should_fix` — real invariant break with data-quality/UX impact, but it needs an overlapping-trigger window and yields duplicate events rather than a crash, data loss, or security hole, so must_fix would overstate it.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Mark the survey as claimed atomically with the decision to render, inside the same `activeSurveyLock` critical section that performs the `canShowNextSurvey()` check, instead of deferring that write to the async `onSurveyShown` callback, e.g.:
```kotlin
internal fun showSurvey(survey: Survey) {
    synchronized(activeSurveyLock) {
        if (activeSurvey != null) {
            config.logger.log("Cannot show survey - another survey is already active")
            return
        }
        activeSurvey = survey
        activeSurveyCompleted = false
        currentSurveyResponses.clear()
    }
    // ... existing setup, with onSurveyShown now just sending the 'survey shown' event
    // instead of also setting activeSurvey
}

</potential_solution>


</details>


/**
* Resolves the surveys delegate.
*
Expand Down
177 changes: 177 additions & 0 deletions posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDisplaySurveyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
package com.posthog.android.surveys

import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.posthog.PostHogConfig
import com.posthog.PostHogInterface
import com.posthog.surveys.OnPostHogSurveyClosed
import com.posthog.surveys.OnPostHogSurveyResponse
import com.posthog.surveys.OnPostHogSurveyShown
import com.posthog.surveys.PostHogDisplaySurvey
import com.posthog.surveys.PostHogSurveysDelegate
import com.posthog.surveys.Survey
import com.posthog.surveys.SurveyConditions
import com.posthog.surveys.SurveyEventCondition
import com.posthog.surveys.SurveyEventConditions
import com.posthog.surveys.SurveyType
import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

/**
* Tests for the manual displaySurvey() API, which displays a survey by ID on demand,
* bypassing display conditions (event triggers, seen checks) — the mobile counterpart
* of the web SDK's posthog.displaySurvey().
*/
@RunWith(AndroidJUnit4::class)
internal class PostHogSurveysDisplaySurveyTest {
private val context = ApplicationProvider.getApplicationContext<android.content.Context>()

/**
* A delegate that records rendered survey IDs and reports each survey as shown,
* so the integration tracks it as the active survey (like the real UI delegates do).
*/
private class RecordingDelegate : PostHogSurveysDelegate {
val renderedSurveyIds = mutableListOf<String>()

override fun renderSurvey(
survey: PostHogDisplaySurvey,
onSurveyShown: OnPostHogSurveyShown,
onSurveyResponse: OnPostHogSurveyResponse,
onSurveyClosed: OnPostHogSurveyClosed,
) {
renderedSurveyIds.add(survey.id)
onSurveyShown(survey)
}

override fun cleanupSurveys() {}
}

private fun createIntegration(
delegate: RecordingDelegate,
surveysEnabled: Boolean = true,
): PostHogSurveysIntegration {
val config =
PostHogConfig("test-api-key").apply {
surveys = surveysEnabled
surveysConfig.surveysDelegate = delegate
}
val integration = PostHogSurveysIntegration(context, config)
val fake = mock<PostHogInterface>()
whenever(fake.isFeatureEnabled(any(), any(), any())).thenReturn(true)
integration.install(fake)
return integration
}

private fun createSurvey(
id: String,
type: SurveyType,
conditions: SurveyConditions? = null,
): Survey {
return Survey(
id = id,
name = "Test Survey $id",
type = type,
questions = emptyList(),
description = null,
featureFlagKeys = null,
linkedFlagKey = null,
targetingFlagKey = null,
internalTargetingFlagKey = null,
conditions = conditions,
appearance = null,
currentIteration = null,
currentIterationStartDate = null,
startDate = java.util.Date(),
endDate = null,
schedule = null,
)
}

private fun eventConditions(eventName: String): SurveyConditions {
return SurveyConditions(
url = null,
urlMatchType = null,
selector = null,
deviceTypes = null,
deviceTypesMatchType = null,
seenSurveyWaitPeriodInDays = null,
events = SurveyEventConditions(repeatedActivation = null, values = listOf(SurveyEventCondition(name = eventName))),
)
}

@Test
fun `displaySurvey renders an API-type survey that is never auto-displayed`() {
val delegate = RecordingDelegate()
val integration = createIntegration(delegate)
integration.onSurveysLoaded(listOf(createSurvey("api-1", SurveyType.API)))
assertFalse(delegate.renderedSurveyIds.contains("api-1"), "API survey should not be auto-displayed")

integration.displaySurvey("api-1")

assertTrue(delegate.renderedSurveyIds.contains("api-1"), "displaySurvey should render the API survey")
}

@Test
fun `displaySurvey bypasses event trigger conditions`() {
val delegate = RecordingDelegate()
val integration = createIntegration(delegate)
val survey = createSurvey("popover-1", SurveyType.POPOVER, conditions = eventConditions("some_event"))
integration.onSurveysLoaded(listOf(survey))
assertFalse(
delegate.renderedSurveyIds.contains("popover-1"),
"Survey with an unfired event trigger should not be auto-displayed",
)

integration.displaySurvey("popover-1")

assertTrue(
delegate.renderedSurveyIds.contains("popover-1"),
"displaySurvey should render the survey even though its event trigger never fired",
)
}

@Test
fun `displaySurvey does nothing when the survey ID is unknown`() {
val delegate = RecordingDelegate()
val integration = createIntegration(delegate)
integration.onSurveysLoaded(listOf(createSurvey("api-1", SurveyType.API)))

integration.displaySurvey("unknown-id")

assertEquals(emptyList(), delegate.renderedSurveyIds)
}

@Test
fun `displaySurvey is ignored while another survey is active`() {
val delegate = RecordingDelegate()
val integration = createIntegration(delegate)
val surveys =
listOf(
createSurvey("popover-1", SurveyType.POPOVER),
createSurvey("api-1", SurveyType.API),
)
integration.onSurveysLoaded(surveys)
assertEquals(listOf("popover-1"), delegate.renderedSurveyIds)

integration.displaySurvey("api-1")

assertEquals(listOf("popover-1"), delegate.renderedSurveyIds)
}

@Test
fun `displaySurvey does nothing when surveys are disabled in config`() {
val delegate = RecordingDelegate()
val integration = createIntegration(delegate, surveysEnabled = false)
integration.onSurveysLoaded(listOf(createSurvey("api-1", SurveyType.API)))

integration.displaySurvey("api-1")

assertEquals(emptyList(), delegate.renderedSurveyIds)
}
}
4 changes: 4 additions & 0 deletions posthog/api/posthog.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public final class com/posthog/PostHog : com/posthog/PostHogStateless, com/posth
public fun captureFeatureView (Ljava/lang/String;Ljava/lang/String;)V
public fun captureLog (Ljava/lang/String;Lcom/posthog/logs/PostHogLogSeverity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;)V
public fun close ()V
public fun displaySurvey (Ljava/lang/String;)V
public fun distinctId ()Ljava/lang/String;
public fun endSession ()V
public fun flush ()V
Expand Down Expand Up @@ -77,6 +78,7 @@ public final class com/posthog/PostHog$Companion : com/posthog/PostHogInterface
public fun captureLog (Ljava/lang/String;Lcom/posthog/logs/PostHogLogSeverity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;)V
public fun close ()V
public fun debug (Z)V
public fun displaySurvey (Ljava/lang/String;)V
public fun distinctId ()Ljava/lang/String;
public fun endSession ()V
public fun flush ()V
Expand Down Expand Up @@ -352,6 +354,7 @@ public abstract interface class com/posthog/PostHogInterface : com/posthog/PostH
public abstract fun captureFeatureInteraction (Ljava/lang/String;Ljava/lang/String;)V
public abstract fun captureFeatureView (Ljava/lang/String;Ljava/lang/String;)V
public abstract fun captureLog (Ljava/lang/String;Lcom/posthog/logs/PostHogLogSeverity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;)V
public abstract fun displaySurvey (Ljava/lang/String;)V
public abstract fun distinctId ()Ljava/lang/String;
public abstract fun endSession ()V
public abstract fun getAllFeatureFlags ()Ljava/util/List;
Expand Down Expand Up @@ -1428,6 +1431,7 @@ public final class com/posthog/internal/replay/RRWireframe {
}

public abstract interface class com/posthog/internal/surveys/PostHogSurveysHandler {
public abstract fun displaySurvey (Ljava/lang/String;)V
public abstract fun onEvent (Ljava/lang/String;Ljava/util/Map;)V
public abstract fun onSurveysLoaded (Ljava/util/List;)V
}
Expand Down
14 changes: 14 additions & 0 deletions posthog/src/main/java/com/posthog/PostHog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,6 +2016,16 @@ public class PostHog private constructor(
}
}

override fun displaySurvey(surveyId: String) {
if (!isEnabled()) {
return
}

surveysHandler?.displaySurvey(surveyId) ?: run {
config?.logger?.log("Cannot display survey $surveyId - surveys integration isn't installed.")
}
}
Comment on lines +2019 to +2027

@posthog posthog Bot Jul 20, 2026

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.

New displaySurvey() entry point lacks the exception guard every other survey-triggering call site uses

must_fix bug

Why we think it's a valid issue
  • Checked: the new displaySurvey entry point (PostHog.kt:2019-2027), the three sibling survey-triggering call sites it is compared against, and whether the guard is instead applied inside the integration (PostHogSurveysIntegration.displaySurvey/showSurvey).
  • Found (siblings all guarded): PostHog.kt:193-198 wraps surveysHandler?.onSurveysLoaded(surveys) in try/catch (Throwable); PostHog.kt:298-303 wraps the install-time it.onSurveysLoaded(surveys); and PostHog.kt:816 surveysHandler?.onEvent(...) sits inside capture()'s top-level try { ... } catch (e: Throwable) (lines 713-822). All three reach showSurvey() (via showNextSurvey()), so the SDK deliberately guards this exact chain at the PostHog.kt boundary.
  • Found (new path unguarded, synchronous): PostHog.kt:2024 calls surveysHandler?.displaySurvey(surveyId) with no guard. The integration's displaySurvey (PostHogSurveysIntegration.kt:151-166) calls showSurvey(survey) directly and synchronously, and showSurvey runs resolveSurveyTranslations, PostHogDisplaySurvey.toDisplaySurvey, then getSurveysDelegate().renderSurvey(...) at line 437 — none wrapped in try/catch. So a throw from the customer-supplied PostHogSurveysDelegate or from translation/model conversion propagates synchronously back out through the public PostHog.displaySurvey().
  • Impact: displaySurvey is a public, synchronous API invoked from app code (e.g. a button tap, typically the main thread) and is the only path that renders API-type/condition-bypassing surveys — the shapes least exercised elsewhere — and the delegate is an explicit customer extension point. An uncaught exception there crashes the host app, a failure mode an analytics SDK is expected to absorb and that every other survey call site in this file already absorbs. Concrete trigger + concrete consequence, directly on the new code, matching an established unanimous convention — clears the keep bar; not defensive paranoia (the delegate is real external code that can throw) or style.
  • Priority: Keeping must_fix — the defect is a host-app crash escaping a public SDK entry point, and 3/3 sibling call sites treat this chain as requiring the guard, so the omission is a clear crash-safety regression rather than a minor inconsistency.
Issue description

Every other place in this file that can trigger PostHogSurveysIntegration.showSurvey() (and thus delegate renderSurvey(), translation resolution, and PostHogDisplaySurvey.toDisplaySurvey conversion) wraps the call in try { } catch (e: Throwable): the onRemoteConfigLoaded callback's surveysHandler?.onSurveysLoaded(surveys) (lines 193-198), the install-time cached-surveys push it.onSurveysLoaded(surveys) (lines 298-303), and the event-driven surveysHandler?.onEvent(event, mergedProperties) inside capture()'s own top-level try/catch (lines 704-822). The new override fun displaySurvey(surveyId: String) calls surveysHandler?.displaySurvey(surveyId) with no such guard. displaySurvey is a public, synchronous, on-demand API explicitly meant to be invoked directly from app code (e.g. a button tap) and, per the PR description, is also the only path that renders API-type surveys and any survey configuration that bypasses the normal display-condition filtering — i.e. it is the first path likely to hit malformed/unexercised survey shapes or a buggy custom PostHogSurveysDelegate (delegates are an explicit customer extension point). If showSurvey() or the delegate throws, that exception now propagates uncaught straight out of the SDK's public API into the host app, unlike every other survey code path in this file, risking a host-app crash from what is supposed to be a resilient analytics SDK call.

Suggested fix

Wrap the delegate call the same way the other call sites in this file do:

override fun displaySurvey(surveyId: String) {
    if (!isEnabled()) {
        return
    }

    try {
        surveysHandler?.displaySurvey(surveyId) ?: run {
            config?.logger?.log("Cannot display survey $surveyId - surveys integration isn't installed.")
        }
    } catch (e: Throwable) {
        config?.logger?.log("Failed to display survey $surveyId: $e.")
    }
}
Prompt to fix with AI (copy-paste) 
## Context
@posthog/src/main/java/com/posthog/PostHog.kt#L2019-2027

<issue_description>
Every other place in this file that can trigger `PostHogSurveysIntegration.showSurvey()` (and thus delegate `renderSurvey()`, translation resolution, and `PostHogDisplaySurvey.toDisplaySurvey` conversion) wraps the call in `try { } catch (e: Throwable)`: the `onRemoteConfigLoaded` callback's `surveysHandler?.onSurveysLoaded(surveys)` (lines 193-198), the install-time cached-surveys push `it.onSurveysLoaded(surveys)` (lines 298-303), and the event-driven `surveysHandler?.onEvent(event, mergedProperties)` inside `capture()`'s own top-level try/catch (lines 704-822). The new `override fun displaySurvey(surveyId: String)` calls `surveysHandler?.displaySurvey(surveyId)` with no such guard. `displaySurvey` is a public, synchronous, on-demand API explicitly meant to be invoked directly from app code (e.g. a button tap) and, per the PR description, is also the only path that renders API-type surveys and any survey configuration that bypasses the normal display-condition filtering — i.e. it is the first path likely to hit malformed/unexercised survey shapes or a buggy custom `PostHogSurveysDelegate` (delegates are an explicit customer extension point). If `showSurvey()` or the delegate throws, that exception now propagates uncaught straight out of the SDK's public API into the host app, unlike every other survey code path in this file, risking a host-app crash from what is supposed to be a resilient analytics SDK call.
</issue_description>

<issue_validation>
- **Checked:** the new `displaySurvey` entry point (`PostHog.kt:2019-2027`), the three sibling survey-triggering call sites it is compared against, and whether the guard is instead applied inside the integration (`PostHogSurveysIntegration.displaySurvey`/`showSurvey`).
- **Found (siblings all guarded):** `PostHog.kt:193-198` wraps `surveysHandler?.onSurveysLoaded(surveys)` in `try/catch (Throwable)`; `PostHog.kt:298-303` wraps the install-time `it.onSurveysLoaded(surveys)`; and `PostHog.kt:816` `surveysHandler?.onEvent(...)` sits inside `capture()`'s top-level `try { ... } catch (e: Throwable)` (lines 713-822). All three reach `showSurvey()` (via `showNextSurvey()`), so the SDK deliberately guards this exact chain at the `PostHog.kt` boundary.
- **Found (new path unguarded, synchronous):** `PostHog.kt:2024` calls `surveysHandler?.displaySurvey(surveyId)` with no guard. The integration's `displaySurvey` (`PostHogSurveysIntegration.kt:151-166`) calls `showSurvey(survey)` directly and synchronously, and `showSurvey` runs `resolveSurveyTranslations`, `PostHogDisplaySurvey.toDisplaySurvey`, then `getSurveysDelegate().renderSurvey(...)` at line 437 — none wrapped in try/catch. So a throw from the customer-supplied `PostHogSurveysDelegate` or from translation/model conversion propagates synchronously back out through the public `PostHog.displaySurvey()`.
- **Impact:** `displaySurvey` is a public, synchronous API invoked from app code (e.g. a button tap, typically the main thread) and is the only path that renders API-type/condition-bypassing surveys — the shapes least exercised elsewhere — and the delegate is an explicit customer extension point. An uncaught exception there crashes the host app, a failure mode an analytics SDK is expected to absorb and that every other survey call site in this file already absorbs. Concrete trigger + concrete consequence, directly on the new code, matching an established unanimous convention — clears the keep bar; not defensive paranoia (the delegate is real external code that can throw) or style.
- **Priority:** Keeping `must_fix` — the defect is a host-app crash escaping a public SDK entry point, and 3/3 sibling call sites treat this chain as requiring the guard, so the omission is a clear crash-safety regression rather than a minor inconsistency.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Wrap the delegate call the same way the other call sites in this file do:
```kotlin
override fun displaySurvey(surveyId: String) {
    if (!isEnabled()) {
        return
    }

    try {
        surveysHandler?.displaySurvey(surveyId) ?: run {
            config?.logger?.log("Cannot display survey $surveyId - surveys integration isn't installed.")
        }
    } catch (e: Throwable) {
        config?.logger?.log("Failed to display survey $surveyId: $e.")
    }
}

</potential_solution>


</details>


override fun getSessionId(): UUID? {
if (!isEnabled()) {
return null
Expand Down Expand Up @@ -2329,6 +2339,10 @@ public class PostHog private constructor(
shared.stopSessionReplay()
}

override fun displaySurvey(surveyId: String) {
shared.displaySurvey(surveyId)
}

override fun getSessionId(): UUID? {
return shared.getSessionId()
}
Expand Down
15 changes: 15 additions & 0 deletions posthog/src/main/java/com/posthog/PostHogInterface.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,21 @@ public interface PostHogInterface : PostHogCoreInterface {
*/
public fun stopSessionReplay()

/**
* Displays the survey with the given ID on demand, regardless of its display conditions.
*
* The survey must be running and returned by the project's remote config; targeting flags,
* event triggers, and the seen/wait-period checks are bypassed. Use this to display surveys
* of type API, or to trigger a popover survey from your own logic. If another survey is
* already being displayed, this call is ignored.
*
* Requires the surveys integration to be enabled via `PostHogConfig.surveys`.
* Android only.
*
* @param surveyId The ID of the survey to display
*/
public fun displaySurvey(surveyId: String)

/**
* Returns the session Id if a session is active
* @return The current session ID, or null if no session is active.
Expand Down
8 changes: 8 additions & 0 deletions posthog/src/main/java/com/posthog/internal/surveys/PostHogSurveysHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,12 @@ public interface PostHogSurveysHandler {
* @param surveys List of surveys loaded from remote config (may be empty)
*/
public fun onSurveysLoaded(surveys: List<Survey>)

/**
* Displays the survey with the given ID on demand, bypassing display conditions
* such as targeting flags, event triggers, and the seen/wait-period checks.
*
* @param surveyId The ID of the survey to display
*/
public fun displaySurvey(surveyId: String)
}
5 changes: 5 additions & 0 deletions posthog/src/testFixtures/java/com/posthog/PostHogFake.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class PostHogFake : PostHogInterface {
public var sessionReplayActive: Boolean = false
public var startSessionReplayCalls: Int = 0
public var stopSessionReplayCalls: Int = 0
public val displaySurveyCalls: MutableList<String> = mutableListOf()

// `PostHogLogger`'s constructor is `internal`. Test fixtures live in the
// same module, so we can construct a silent no-op here without exposing
Expand Down Expand Up @@ -232,6 +233,10 @@ public class PostHogFake : PostHogInterface {
sessionReplayActive = false
}

override fun displaySurvey(surveyId: String) {
displaySurveyCalls.add(surveyId)
}

override fun getSessionId(): UUID? {
return null
}
Expand Down
Loading