docs/solutions/logic-errors/denied-push-permission-rendered-as-retryable-failure.md
The Pro post-checkout activation wizard offers a "real-time alerts" step that
requests browser notification permission. When Notification.requestPermission()
resolved denied, the step fell into the wizard's generic failure state:
"DIDN'T WORK — Try again". Browsers never re-prompt once permission is
denied, so that button was guaranteed to fail identically forever — the user's
only escape was abandoning the step.
Surfaced during the live purchase repro in #5600, filed as #5609, fixed in PR #5615.
denied when the flow opened.Reaching for the error message. subscribeToPush() throws a
denial-specific string ('Notifications are blocked. Enable them in your browser settings to continue.', src/services/push-notifications.ts:139), so
the obvious classification is to match on it. Rejected: error strings are
incidental. They get reworded, localized, or wrapped, and the classification
silently degrades to the buggy behavior with no test failing. The permission
state itself is the authoritative signal and is already exposed.
Widening the existing failed state instead of adding a state. Making the
failed state conditionally hide its retry button would have left failed
meaning two different things depending on a flag, and left the outcome recorded
as a failure — which the exit summary renders as "we couldn't set this up", the
wrong sentence for something the browser refused and we never attempted.
1. Give the result union a way to say "do not retry." The confirm handler
returned a binary 'verified' | 'failed', and failed implicitly promises a
retry is worth taking. A third member carries the distinction
(src/components/ProActivationInterstitial.ts:80):
export type ActivationConfirmResult = 'verified' | 'failed' | 'blocked';
2. Classify from live platform state, not the thrown error. After
subscribeToPush() rejects, re-read the permission
(src/components/ProActivationInterstitial.ts:1171):
} catch (err) {
console.warn('[pro-activation] push subscribe declined/failed', err);
// Read the LIVE permission rather than the thrown message: once it is
// `denied` no browser re-prompts, so the step is blocked, not retryable.
// A dismissed prompt leaves it `default` and stays a retryable failure.
return getPushPermission() === 'denied' ? 'blocked' : 'failed';
}
This also preserves the correct retry: a user who dismisses the prompt
without choosing leaves permission at default, the browser will ask again,
and "Try again" still means something.
3. Fold the runtime override into a single state accessor. The blocked UI
already existed but every render path read the step's declared step.state.
Rather than adding parallel branches, one accessor merges the mid-flow block
into that state, and all reads go through it:
const blockedByConfirm = new Set<ActivationStepId>();
const effectiveState = (step: ActivationStep): ActivationStepState =>
blockedByConfirm.has(step.id) ? 'blocked' : step.state;
A mid-flow block then renders through the exact path a mount-blocked step already used — blocked badge, site-settings note, a single "Continue", no retry CTA — with no second implementation to drift.
4. Keep the outcome honest, and replace the signal it costs. A blocked step
must not resolve as failed, or the summary claims a failure that never
happened. It originally resolved as skipped — but skipped is also what a
user who clicked "Skip for now" produces, so the denial cohort vanished into
voluntary skips. A distinct funnel event kept them separable on the live stream:
stepBlocked: 'pro-activation-step-blocked',
#5617 then finished the job on the durable side: blocked became its own
ActivationStepOutcome and its own blockedSteps bucket on the Convex row,
while still reading as pending (never failed) in the summary and still
counting as pending in the exit event — a record change, not a UX change.
The root cause is a terminal state modelled as a transient one. failed
encodes "this attempt did not work"; the retry affordance is downstream of that
meaning. A browser permission denial is not a failed attempt — it is a standing
refusal that no amount of retrying inside the page can lift, and whose remedy
lives outside the app entirely. Once the result vocabulary can express that, the
right UI was already written.
Reading getPushPermission() rather than the error text works because the
permission is the thing that actually determines whether a retry can succeed.
The spec commits Notification.permission before requestPermission()'s
promise settles, so the read is accurate at the moment of the catch. If some
engine violated that, the read returns a stale default and the code falls back
to today's retryable behavior — it fails safe.
Ask "can a retry ever succeed?" before rendering a retry. Any failure path that surfaces a retry CTA needs an answer. Where the failure comes from a platform permission, a hard quota, a revoked grant, or a policy block, the answer is no, and the UI owes the user the out-of-app remedy instead. This is the mirror of a bug already documented in this repo — see billing-state-cancelled-but-paid-through-misclassified-as-lapsed, where a state that merely sounded terminal was treated as terminal. Both come from inferring terminal-vs-transient from a proxy (a status word, an error string) instead of reading the authoritative live signal.
When a state becomes reachable at a new time, funnel every read through one
accessor. The blocked state was previously only reachable at mount, so reads
of step.state were safe. Making it reachable mid-flow turned each direct read
into a latent bug. Two reviewers independently found two reads that had not been
migrated — inert at the time, because both only branched on already-done, but
they would have failed silently the moment either branched on blocked. Grep
for the raw field after introducing an override and migrate all of them.
When a fix reclassifies an outcome into an existing bucket, check what signal
that erases. Moving denials from failed to skipped was right for the user
and quietly destroyed the ability to size the denial cohort — a browser refusal
would have read as disinterest in the funnel. Adding an event was ~6 lines. The
durable Convex record collapsed the two for another day (skippedSteps with no
marker) until #5617 added a fourth blockedSteps bucket.
Restoring a lost signal is two separate jobs: the live stream and the durable record. The event fix (#5615) answered "how many denials this week"; it could not answer "is re-prompting THIS account worth anything" — a per-account question only the persisted row can serve. Adding the event felt like closing the gap because the dashboards went right. Ask which questions each surface actually answers before calling a telemetry gap closed.
A widened enum finds every else you wrote as a catch-all. Adding
'blocked' to ActivationStepOutcome was safe everywhere the code switched
exhaustively (TypeScript reddened each one) and silent exactly where it did not:
buildActivationOutcomeBuckets ended else failedSteps.push(r.id), so the new
outcome would have been persisted as a failed write — the precise mislabel the
change existed to prevent, reintroduced by the shape of the old code. Route
every case explicitly so the compiler, not a reviewer, catches the next one.
And it finds every hand-copied structural type.
ProActivationFlowDependencies.recordOutcome re-declared the outcome-snapshot
shape inline instead of importing ProActivationOutcomeSnapshot. A duplicated
shape does not fail when the original grows — it just quietly describes less.
Here typecheck happened to catch it; a looser call site would not have.
One state, one write path — count them before adding the state. A denied
alerts step can leave the wizard three ways: Continue, Escape/dismiss, and never
being reached at all. Two of the three ran through separate inline literals.
Writing blocked on only the Continue arm (the shape the issue proposed) would
have produced a blockedSteps bucket sampling denied users who clicked
Continue — a plausible-looking number that silently undercounts the cohort it
exists to size. All three now share one pure selectAdvanceOutcome(state).
A test DOM whose innerHTML getter replays the assigned string will go
vacuous. The unit harness for this fix parses assigned innerHTML so the
component's querySelector wiring works. Its getter originally returned the raw
assigned string, which is sound only while nothing mutates the subtree after
render — and this component already mutates post-render elsewhere (the brief
step's preview swap). The moment coverage extends there, assert.doesNotMatch
starts passing because nothing was parsed rather than because the thing is
absent: green while red. Serialize the live tree instead:
class ParsingElement extends MiniElement {
override get innerHTML(): string {
return this.childNodes.map((child) => serializeNode(child as MiniElement | MiniText)).join('');
}
override set innerHTML(value: string) { /* parse into childNodes */ }
}
The serializer must be attribute-faithful — MiniElement's own outerHTML
drops attributes, which would break every data-action="..." assertion.
Prove the guard with a mutation, not a passing run. Both regressions here
were confirmed by breaking the code and watching exactly one test go red:
disabling the blocked branch reddens the e2e; making finalizeAndShowSummary
record blocked steps as failed reddens the Escape-while-blocked unit test. A
test that has never been observed failing has not been shown to test anything.
blockedSteps as a fourth outcome bucket, so
a browser refusal is queryable per-account after the fact and not just live
on the event stream