fix(start-session-recording): require replay setup crash containment - #55
Draft
posthog-local-dev[bot] wants to merge 3 commits into
Draft
Conversation
Add a canonical requirement and public acceptance scenarios that hold every replay-capable SDK to the same crash-containment bar. When the replay configuration is invalid, or the replay integration is unavailable or fails to initialize, start-session-recording must no-op or log and must not throw into host application code, and session recording must stay inactive. The contract existed only in prose and no scenario tested it. The direct crash fixes land in posthog-android and posthog-ios. Generated-By: PostHog Desktop Task-Id: 0422fe0c-2fe9-4a36-91a4-60bf2b7cd40f
Author
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 2 should fix, 0 consider. Published 2 findings (view the review). Resolved comments: 2 fixed |
Author
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
The crash-containment requirement names three failure inputs — invalid configuration, unavailable integration, and an integration that fails to initialize — but only the first two had scenarios. An SDK that validates the configuration before calling the replay initializer, or that never builds the integration, passes both existing scenarios while still throwing from an initializer that fails at runtime (native library load failure, missing OS API, permission refusal) — the exact path the audited crash report describes. Add a third scenario where replay is configured and eligible to start but the integration fails to initialize, so the initializer is actually reached, and assert the untested promise clause alongside "does not throw" and "recording stays inactive". Mirrored into the public acceptance feature and the archived change delta. Generated-By: PostHog Code Task-Id: 9f7f8b74-ca9d-4eb2-960a-4aece6bc51fc
Tasks 2.1 and 2.3 claimed strict validation without saying what ran, and the PR description said the OpenSpec CLI was unavailable — so the archived record was unsupported. The CLI is available: it is @fission-ai/openspec, the package the repo's own validation workflow installs. Strict validation now genuinely passes — `validate --all --strict --no-interactive` reports 62/62 specs, and the change itself passes `validate --changes --strict` when its archived directory is staged back as an active change in a scratch copy. Note both commands next to the tasks so the archive records how the boxes were earned rather than only that they were checked. Generated-By: PostHog Code Task-Id: 9f7f8b74-ca9d-4eb2-960a-4aece6bc51fc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
start-session-recordingsays the call "should not throw" and unavailable integrations "no-op or log";session-replay-privacysays capture must "fail closed ... rather than crash the app".Changes
start-session-recording: when the replay configuration is invalid, or the replay integration is unavailable or fails to initialize, the SDK MUST no-op or log and MUST NOT throw into host application code. Recording stays inactive; promise variants resolve rather than reject.start-session-recordingError handling prose to the new requirement.The non-negotiable outcome tested is "does not throw" + "recording stays inactive". Logging stays optional to match the canonical "no-op or log" wording and not over-constrain SDKs that legitimately stay silent.
Scope
openspec/changes/archive/2026-09-01-require-replay-start-crash-containment/, canonical spec synced).Agent context
openspec validate --specs --strict: the OpenSpec CLI is not installed and is not the public npmopenspecpackage. Format was matched by hand against the most recent archived changes and canonical specs.Created with PostHog Desktop from this inbox report.