Repository navigation
fix(surveys): dismiss a survey when a cancel event is captured - #1042
Conversation
e23fada to
fa11f60
Compare
Commit signatures resolvedGitHub now verifies every commit signature in this PR. No further action is needed. |
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review. Reproduced blocking findings from the requested review.
|
@marandaneto survey dismissal now runs on the main thread, and the cancel test is in the Xcode target. Can you review again? |
and force push |
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory follow-up review. Both earlier findings are fixed; two new reproduced blockers remain.
Capture can invoke cancel handlers off the main queue, and the new regression test was missing from the Xcode test target.
The main-queue dismissal is asynchronous, and it leaves a survey alone when reset has already started a new attempt.
|
@marandaneto rewrote the branch onto main so the unsigned merge is gone. The three commits on the branch are signed. |
|
Unrelated to this PR, so feel free to ignore it here. I applied for the Technical Support Engineer role today. The careers form returned an application-limit error, so I sent the application to careers@posthog.com instead. Working through the mobile SDKs over the last two months, I kept running into the same kinds of gaps: surveys behaving differently on Flutter, iOS and Android, opt-out not fully stopping replay or integrations, and beforeSend not covering every capture path on Flutter web. I have ideas for dealing with the patterns behind those and not only the individual bugs, and I'd like the chance to work on them properly. Would you be open to a short chat about it, or to pointing me to the right person on the talent team? Happy to do it async, wherever suits you. |
marandaneto
left a comment
There was a problem hiding this comment.
Agent-drafted and reviewed by Manoel.
Posting the new reproduced selection/reservation cancellation race. The existing deadlock and stale-attempt findings also still reproduce on this head; their inline comments are not duplicated here:
|
@PostHog/team-client-libraries could you please re-review this PR? New changes have been pushed to address the survey cancellation feedback. |
Recheck activation atomically during reservation and reuse attempt-checked close handling for pending and shown surveys. Preserve the queued selection filter and cover cancellation after a response before the shown callback.
|
Pushed the agent-assisted follow-up in a92adcb on top of The previous head passed the three original regressions, but a further simulator test exposed a lifecycle-state inconsistency: a renderer can report a response before calling The retained This also keeps the latest queued-selection filter and rechecks activation under the reservation lock before committing an attempt. The asynchronous main-queue handoff and stale-attempt protection remain in place. Validation passed:
Simulator runs used iPhone 17 / iOS 27.2 with a validation-only iOS 15 build override for the installed Xcode. Repository deployment minima are unchanged; the multi-platform SDK builds ran without that override. |
turnipdabeets
left a comment
There was a problem hiding this comment.
[nit] The description says web dismisses an on-screen survey on a cancel event. It doesn't: cancelSurvey in surveys-renderer.tsx only acts on surveys still waiting out the delay, and the dashboard describes cancel events as "During your N second delay…".
dustinbyrne
left a comment
There was a problem hiding this comment.
The earlier threading, registration, stale-attempt and queued-selection findings are resolved. The authoritative-state repair also passes an independent SDK-core regression.
The existing custom-delegate ownership concern remains: matching cancellation releases the attempt without cancelling the host’s UI or deferred work. Base/head SDK-core tests reproduce both variants; unrelated-event controls pass. Pending-only behavior must also protect custom deferred handoff.
AI-assisted review.
Preserve shown surveys and custom delegate ownership, including deferred presentations. Reuse the built-in pending-close path, document the contract, and classify the new capability as a minor release.
💡 Motivation and Context
Fixes #450.
Add support for survey
cancel_events, matching the web SDK's pending-only cancellation behavior.A matching cancel event clears unconsumed event activation. For the built-in UI, it also cancels a survey that is still waiting out its popup delay and releases the reservation without sending
survey shownorsurvey dismissed, marking it seen, or updating the last-seen date. It does not dismiss a survey that is already visible.Once
renderSurveyhands a survey to a customPostHogSurveysDelegate, that delegate retains ownership, including while it defers display. Cancel events do not invalidate the attempt or its callbacks, so the host can still report it shown, submit answers, and close it normally without losing responses or admitting another survey on top.No public API change. The delegate documentation explains this ownership boundary. The changeset is classified as minor because this adds support for a previously unsupported remote capability.
💚 How did you test it?
make lint,make checkTestRegistration, andgit diff --checkmake test: 1,186 tests passed in a fresh sequential runmake buildSdk: all supported platform build lanes passedmake testSurveyUI: all 7 mounted UI tests passedmake testPresentationMasks: all 31 hosted presentation/privacy tests passedSimulator runs used iPhone 17 / iOS 27.2 with a validation-only iOS 15 build override for the installed Xcode. Repository deployment minima are unchanged; the cross-platform SDK builds used Xcode 26.6 without that override.
An initial SPM run alongside simulator validation failed in queue/storage tests. The fresh sequential full rerun passed without source changes; both results were retained locally.
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted), directed by @marandaneto.
Pi and the GitHub CLI were used for test-first follow-up fixes and review replies. The implementation deliberately follows pending-only web behavior and preserves custom-renderer ownership rather than introducing a new delegate cancellation API.