diff --git a/.github/workflows/build-docs.yml b/.github/workflows/build-docs.yml index e3ba9294ad..609624c0ec 100644 --- a/.github/workflows/build-docs.yml +++ b/.github/workflows/build-docs.yml @@ -81,8 +81,17 @@ jobs: - name: Test C API 404 fallback run: yarn test:not-found + # This build checks the site; it does not publish it. On the main branch + # it gets the feedback widget's fallback endpoint so the check covers the + # configuration the published site runs with. Pull-request builds (whose + # ref is refs/pull/...) get empty values, which disable the fallback; the + # widget still works without it. The published site gets its values from + # the job that publishes it. - name: Build documentation run: yarn build + env: + DOCS_FEEDBACK_ENDPOINT: ${{ github.ref == 'refs/heads/main' && vars.DOCS_FEEDBACK_ENDPOINT || '' }} + DOCS_FEEDBACK_TOKEN: ${{ github.ref == 'refs/heads/main' && secrets.DOCS_FEEDBACK_TOKEN || '' }} # Asserts what the widget actually sends to Algolia: the API reference is # reachable on the served version and absent on frozen legacy versions. diff --git a/.github/workflows/docs-preview.yml b/.github/workflows/docs-preview.yml index f891280f8a..ac044c99e1 100644 --- a/.github/workflows/docs-preview.yml +++ b/.github/workflows/docs-preview.yml @@ -31,6 +31,9 @@ jobs: - name: Install dependencies run: yarn install --frozen-lockfile + # DOCS_FEEDBACK_ENDPOINT / DOCS_FEEDBACK_TOKEN are deliberately NOT set + # here: a preview must not post reader feedback into the production + # feedback sheet. preview_build also forces them off in the config. - name: Build documentation run: yarn build env: diff --git a/docusaurus.config.ts b/docusaurus.config.ts index 9a5ba94934..941784181b 100644 --- a/docusaurus.config.ts +++ b/docusaurus.config.ts @@ -1008,6 +1008,22 @@ const config: Config = { // The search widget's whole view of docusaurus_tag, derived above and passed // through so SearchBar never constructs one of these strings itself. customFields: { + // The Apps Script Web App that receives feedback from readers on whose + // page PostHog is not loaded -- it is loaded behind the cookie banner, so + // their comments reached nothing at all. Empty disables the fallback and + // the widget behaves exactly as it did before. + // + // Set from the environment so a redeploy of the Web App (which changes the + // URL) needs no code change, and so a fork or a preview build does not + // silently post into the production sheet. The job that publishes the + // production site sets it; every other build leaves it unset, and a + // preview build forces it off even if the variable leaks into its + // environment. + feedbackEndpoint: isPreviewBuild ? "" : process.env.DOCS_FEEDBACK_ENDPOINT ?? "", + // Not a secret: it ships in the client bundle. It only filters untargeted + // requests; any limit on abuse has to be enforced by the receiving + // endpoint. + feedbackToken: isPreviewBuild ? "" : process.env.DOCS_FEEDBACK_TOKEN ?? "", // Major typed in a query -> tag of the version a reader is actually served. versionTagByMajor, // A docs version's tag -> the API-reference tag(s) that document it, so a diff --git a/src/components/PageFeedback/direct.ts b/src/components/PageFeedback/direct.ts new file mode 100644 index 0000000000..7a98d34956 --- /dev/null +++ b/src/components/PageFeedback/direct.ts @@ -0,0 +1,71 @@ +/** + * The fallback for a comment PostHog could not take because it is not loaded. + * + * WHY THIS EXISTS. The widget's events go to PostHog, which is loaded BEHIND + * the consent banner: until a reader accepts cookies, `window.posthog` does + * not exist and the capture reports `not-loaded`. So a reader who declines + * and then spends two minutes writing us a paragraph leaves no trace of it + * anywhere. + * + * ONLY WHEN POSTHOG IS NOT LOADED. Never beside a successful capture, so a + * consenting reader's comment is not recorded twice under two timestamps; and + * never when the reader has explicitly opted out of capturing in PostHog (or + * when PostHog threw and that cannot be ruled out). An opt-out is a refusal, + * not a delivery problem to route around. + * + * WHERE IT GOES. An Apps Script Web App deployed to accept anonymous requests, + * which appends the comment to a spreadsheet. The endpoint must accept + * requests from a reader's browser without any sign-in. The widget's note + * tells the reader this before they send, whenever this path applies. + * + * WHAT IT SENDS. The four fields the widget already sends to PostHog, plus + * the token, and nothing else in the body. Like any web request, it also + * carries the browser's IP address and user agent to the receiving server. + */ + +type DirectFeedback = { + url: string; + title: string; + helpful: boolean | null; + comment: string; +}; + +/** + * `no-cors`, and what that costs. + * + * An Apps Script Web App does not answer a cross-origin preflight, so a normal + * JSON POST never leaves the browser. `text/plain` keeps the request "simple" + * and no-cors lets it through β€” at the price of an opaque response we cannot + * read. So this reports whether the request was DISPATCHED, not whether it + * arrived. + * + * An Apps Script that answers with an error, or was redeployed to a new URL, + * still resolves here. So on this path the widget's copy says only that the + * note was sent ("Thanks β€” your note was sent."), never that it reached the + * team. + */ +export async function sendDirectFeedback( + endpoint: string, + token: string, + payload: DirectFeedback, +): Promise { + if (!endpoint) return false; + try { + await fetch(endpoint, { + method: 'POST', + mode: 'no-cors', + // Deliberately text/plain: application/json would trigger a preflight + // the Web App cannot answer. + headers: { 'Content-Type': 'text/plain;charset=utf-8' }, + // `keepalive` so the request is still dispatched if the reader navigates + // away mid-send (no-cors cannot confirm it arrived). + keepalive: true, + body: JSON.stringify({ ...payload, token }), + }); + return true; + } catch { + // Offline, blocked, or refused. The caller tells the reader plainly and + // keeps their text in the box. + return false; + } +} diff --git a/src/components/PageFeedback/index.tsx b/src/components/PageFeedback/index.tsx index 7878341576..dc4c6388ca 100644 --- a/src/components/PageFeedback/index.tsx +++ b/src/components/PageFeedback/index.tsx @@ -1,6 +1,12 @@ import React from 'react'; import BrowserOnly from '@docusaurus/BrowserOnly'; -import { capturePostHogEvent } from '@site/src/components/SkillsCallout/analytics'; +import { + capturePostHogEvent, + capturePostHogEventWithResult, + postHogStatus, +} from '@site/src/components/SkillsCallout/analytics'; +import useDocusaurusContext from '@docusaurus/useDocusaurusContext'; +import { sendDirectFeedback } from './direct'; import styles from './styles.module.css'; /** @@ -29,6 +35,10 @@ import styles from './styles.module.css'; * of thanking them for something that was dropped, and every further * attempt is announced as well as focused, because focusing an element that * already has focus re-reads nothing; + * - a COMMENT on a page where PostHog is NOT LOADED goes to the fallback + * endpoint instead (see ./direct.ts), if one is configured, and the note + * under the box says so before the reader sends. A reader who explicitly + * opted out of PostHog never takes that path; * - a VOTE that did not send is held and retried β€” when the comment is sent, * on a short bounded poll, and once more as the page is being unloaded β€” * so consenting mid-visit does not cost the KPI the votes of exactly the @@ -42,6 +52,19 @@ import styles from './styles.module.css'; * would drop keyboard focus to . */ +type Outcome = + | 'open' + // Accepted by PostHog. + | 'sent' + // Dispatched to the fallback endpoint. `no-cors` hides the response, so this + // is all that can be said - the copy for it never claims delivery. + | 'dispatched' + // PostHog refused (not loaded with no fallback configured, opted out, or + // threw). Nothing left the page. + | 'refused' + // The fallback request itself failed: offline, or blocked. + | 'unreachable'; + const VOTE_KEY = (path: string) => `docs-feedback:${path}`; const COMMENT_KEY = (path: string) => `docs-feedback-comment:${path}`; const COMMENT_MAX = 1000; @@ -76,10 +99,47 @@ function Inner({ url, title }: PageFeedbackProps) { // Match `trailingSlash: true`, which is what $pathname carries. const path = url.endsWith('/') ? url : `${url}/`; + // CONFIGURED, NOT COMPILED IN. The Web App URL changes every time the Apps + // Script is redeployed, and an empty value disables the fallback: submit() + // never calls it, the note keeps its analytics wording, and the widget + // behaves exactly as it did before, telling the reader their note did not + // send. + const { siteConfig } = useDocusaurusContext(); + const feedbackEndpoint = String( + (siteConfig.customFields as Record | undefined)?.feedbackEndpoint ?? '', + ); + // Obfuscation, and named as such. It ships in client JavaScript, so it stops + // scanners rather than people; the real limits are on the receiving end. + const feedbackToken = String( + (siteConfig.customFields as Record | undefined)?.feedbackToken ?? '', + ); + const [helpful, setHelpful] = React.useState(null); const [recorded, setRecorded] = React.useState(false); const [comment, setComment] = React.useState(''); - const [outcome, setOutcome] = React.useState<'open' | 'sent' | 'undelivered'>('open'); + const [outcome, setOutcomeState] = React.useState('open'); + // The current outcome, readable after an await. The `outcome` a pending + // submit() closed over is from the render that created it, and can be stale + // by the time the request settles. + const outcomeRef = React.useRef('open'); + const setOutcome = (next: Outcome) => { + outcomeRef.current = next; + setOutcomeState(next); + }; + // In-flight guard. The ref blocks a second submit synchronously (a + // double-click, or Cmd+Enter then a click, lands before any re-render); the + // state drives the button. + const sendingRef = React.useRef(false); + const [sending, setSending] = React.useState(false); + // A reader can follow an in-site link while the fallback request is in + // flight; nothing may set state on the unmounted widget after that. + const mounted = React.useRef(true); + React.useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; + }; + }, []); const [attempts, setAttempts] = React.useState(0); // Bumped on every vote attempt, so the retry effect re-arms even when the // reader clicks the same button again and no other state changes. @@ -101,6 +161,9 @@ function Inner({ url, title }: PageFeedbackProps) { const confirmation = React.useRef(null); // A vote whose capture was dropped, waiting for PostHog to turn up. const heldVote = React.useRef(null); + // Counts thumb clicks, so code that waited on a request can tell whether the + // reader voted again meanwhile. A ref, not state: read after an await. + const votesCast = React.useRef(0); // Set when the confirmation is about to appear for the first time, i.e. when // the outcome actually changes. A repeat announces instead - see submit(). const focusConfirmation = React.useRef(false); @@ -111,7 +174,8 @@ function Inner({ url, title }: PageFeedbackProps) { setHelpful(prior === 'up'); setRecorded(true); } - if (readSession(COMMENT_KEY(path)) === 'sent') setOutcome('sent'); + const priorComment = readSession(COMMENT_KEY(path)); + if (priorComment === 'sent' || priorComment === 'dispatched') setOutcome(priorComment); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -144,7 +208,11 @@ function Inner({ url, title }: PageFeedbackProps) { }; const vote = (isHelpful: boolean) => { - if (recorded) return; + // Not while a comment is being sent: it already carries the current vote, + // and a different one cast meanwhile would be recorded in two places with + // two values. + if (recorded || sendingRef.current) return; + votesCast.current += 1; setHelpful(isHelpful); setVoteAttempt((n) => n + 1); say( @@ -189,90 +257,148 @@ function Inner({ url, title }: PageFeedbackProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [voteAttempt, recorded]); - const submit = () => { + const submit = async () => { const text = comment.trim(); - if (!text) return; - // Flush the held vote first, so the pair arrives in the right order. - // - // Keep this UNCONDITIONAL and keep it here. - // - // What else could flush a held vote, and why none of it is cover. - // - // The poll stops retrying after RETRY_LIMIT * RETRY_MS, but clearing the - // interval does NOT unregister the `pagehide` listener or the unmount - // `lastChance()` - the effect does not re-run - so both stay armed. They - // are not equivalent to each other, and the difference matters: - // - // - UNMOUNT is reliable. On this SPA the widget unmounts on a client-side - // route change, which is how most readers leave a docs page, and the - // page is fully alive when it runs (see the note on the cleanup above). - // It is a better bet than the flush below, not a worse one - do not - // remove it on the strength of the next bullet. - // - PAGEHIDE is the unreliable one: a real unload, where an in-flight - // request may never leave the browser, and where an accepted event is - // not a delivered one. - // - // So this line has two jobs, depending on whether the poll is still running: - // - // - WHILE THE POLL RUNS, it keeps the vote AHEAD of the comment. The poll - // only fires every RETRY_MS, so if PostHog becomes ready between ticks - - // the reader accepts cookies, then presses Send a second later - the - // vote is still held here, and without this line the comment would go - // first and the vote would follow on the next tick or at unmount, out of - // order. It is not a no-op then, and not a no-op when PostHog is still - // missing either: the send fails and the vote is re-held for the poll. - // - ONCE THE POLL HAS EXHAUSTED, it is the only flush for the reader who - // neither navigates in-site nor unloads cleanly - the tab discarded, the - // process killed - having written a comment. - // - // Neither job depends on the other, so do not gate this line on the poll's - // state in either direction. - // - // Two edits remove it: - // - // - DELETING it as "the poll already covers it". Once exhausted the poll - // has stopped; before that it only fires on a tick, which cannot keep - // the vote ahead of a comment sent between ticks. - // - GATING it on the comment succeeding, `if (delivered && ...)`. The - // argument against this needs no scenario at all: the gate can only ever - // LOSE a vote and can never gain one. Where `delivered` is true the - // vote goes either way, so the gate buys nothing; where it is false the - // vote may still have been accepted - three of capturePostHogEvent's - // four false-exits are shared state read one line apart, but the fourth - // is a throw, and the vote's properties are a subset of the comment's, - // so a payload that throws on the comment need not have thrown on the - // vote. There is no case in which adding the gate helps. - // - // The ORDER of the two calls protects nothing on its own; it only keeps the - // pair in sequence. For the three shared-state exits, swapping them cannot - // change whether the VOTE is accepted. - // - // Why a lost vote matters rests on something NOT checkable from this repo: - // vote counts are read from `docs_page_feedback`. Note that this is a - // strict PREFIX of `docs_page_feedback_comment`, so an insight that matches - // by "contains" or by regex rather than exact equality already counts both - // - in which case this rationale is wrong today, not merely at risk of - // going wrong. Neither event name appears anywhere else in this repo, so - // confirm the insight uses an exact-match filter before relying on any of - // this. - if (heldVote.current !== null) sendVote(heldVote.current); - const delivered = capturePostHogEvent('docs_page_feedback_comment', { - ...base(), - helpful: helpful === true, - comment: text, - }); - if (delivered) writeSession(COMMENT_KEY(path), 'sent'); - const next = delivered ? 'sent' : 'undelivered'; + if (!text || sendingRef.current) return; + sendingRef.current = true; + setSending(true); + let next: Outcome; + try { + // Flush the held vote first, so the pair arrives in the right order. + // + // Keep this UNCONDITIONAL and keep it here. + // + // What else could flush a held vote, and why none of it is cover. + // + // The poll stops retrying after RETRY_LIMIT * RETRY_MS, but clearing the + // interval does NOT unregister the `pagehide` listener or the unmount + // `lastChance()` - the effect does not re-run - so both stay armed. They + // are not equivalent to each other, and the difference matters: + // + // - UNMOUNT is reliable. On this SPA the widget unmounts on a client-side + // route change, which is how most readers leave a docs page, and the + // page is fully alive when it runs (see the note on the cleanup above). + // It is a better bet than the flush below, not a worse one - do not + // remove it on the strength of the next bullet. + // - PAGEHIDE is the unreliable one: a real unload, where an in-flight + // request may never leave the browser, and where an accepted event is + // not a delivered one. + // + // So this line has two jobs, depending on whether the poll is still running: + // + // - WHILE THE POLL RUNS, it keeps the vote AHEAD of the comment. The poll + // only fires every RETRY_MS, so if PostHog becomes ready between ticks - + // the reader accepts cookies, then presses Send a second later - the + // vote is still held here, and without this line the comment would go + // first and the vote would follow on the next tick or at unmount, out of + // order. It is not a no-op then, and not a no-op when PostHog is still + // missing either: the send fails and the vote is re-held for the poll. + // - ONCE THE POLL HAS EXHAUSTED, it is the only flush for the reader who + // neither navigates in-site nor unloads cleanly - the tab discarded, the + // process killed - having written a comment. + // + // Neither job depends on the other, so do not gate this line on the poll's + // state in either direction. + // + // Two edits remove it: + // + // - DELETING it as "the poll already covers it". Once exhausted the poll + // has stopped; before that it only fires on a tick, which cannot keep + // the vote ahead of a comment sent between ticks. + // - GATING it on the comment succeeding, `if (delivered && ...)`. The + // argument against this needs no scenario at all: the gate can only ever + // LOSE a vote and can never gain one. Where `delivered` is true the + // vote goes either way, so the gate buys nothing; where it is false the + // vote may still have been accepted - three of capturePostHogEvent's + // four false-exits are shared state read one line apart, but the fourth + // is a throw, and the vote's properties are a subset of the comment's, + // so a payload that throws on the comment need not have thrown on the + // vote. There is no case in which adding the gate helps. + // + // The ORDER of the two calls protects nothing on its own; it only keeps the + // pair in sequence. For the three shared-state exits, swapping them cannot + // change whether the VOTE is accepted. + // + // Why a lost vote matters rests on something NOT checkable from this repo: + // vote counts are read from `docs_page_feedback`. Note that this is a + // strict PREFIX of `docs_page_feedback_comment`, so an insight that matches + // by "contains" or by regex rather than exact equality already counts both + // - in which case this rationale is wrong today, not merely at risk of + // going wrong. Neither event name appears anywhere else in this repo, so + // confirm the insight uses an exact-match filter before relying on any of + // this. + if (heldVote.current !== null) sendVote(heldVote.current); + const result = capturePostHogEventWithResult('docs_page_feedback_comment', { + ...base(), + helpful: helpful === true, + comment: text, + }); + if (result === 'captured') { + next = 'sent'; + } else if (result === 'not-loaded' && feedbackEndpoint) { + // ONLY WHEN POSTHOG IS NOT LOADED. Never alongside a successful + // capture, or a consenting reader's comment would be recorded twice + // under two timestamps; and never for `opted-out` or `error`, where + // the reader said no (or may have) and the comment must go nowhere. + // The vote goes out with the comment, so stop holding it BEFORE the + // request: otherwise the retry loop (or `pagehide`, or unmount) could + // flush it to PostHog if the reader accepts cookies while the request + // is in flight, and the same vote would be recorded in two places. + // Restored if the request fails, unless the reader voted again. + const held = heldVote.current; + const votesBefore = votesCast.current; + heldVote.current = null; + const dispatched = await sendDirectFeedback(feedbackEndpoint, feedbackToken, { + ...base(), + // THREE-VALUED here, unlike the PostHog call above, which has always + // coerced. A reader can type before clicking either thumb, and a + // missing vote sent as `false` becomes a complaint they did not make. + helpful, + comment: text, + }); + next = dispatched ? 'dispatched' : 'unreachable'; + if (!dispatched && held !== null && votesCast.current === votesBefore) { + if (mounted.current) { + heldVote.current = held; + // The retry poll stops for good once it sees no held vote, which + // it may have while the request was in flight. Start it again. + setVoteAttempt((n) => n + 1); + } else { + // Unmounted during the request: its last chance to flush already + // ran, with nothing held. One try now, or the vote is lost. + if (capturePostHogEvent('docs_page_feedback', { ...base(), helpful: held })) { + writeSession(VOTE_KEY(path), held ? 'up' : 'down'); + } + } + } + if (dispatched) { + if (helpful !== null) { + writeSession(VOTE_KEY(path), helpful ? 'up' : 'down'); + if (mounted.current) setRecorded(true); + } + } + } else { + next = 'refused'; + } + } finally { + sendingRef.current = false; + } + // Written even if the widget has gone, so a reload does not reopen a box + // whose comment already went out. + if (next === 'sent' || next === 'dispatched') writeSession(COMMENT_KEY(path), next); + if (!mounted.current) return; + setSending(false); // Exactly one channel, chosen by whether the message is new. // - // NEW outcome: the paragraph mounts and focus moves to it, which reads it in - // full. Announcing as well would say the same thing twice in two wordings. + // NEW outcome: the paragraph mounts and focus moves to it, which reads it + // in full. Announcing as well would say the same thing twice in two + // wordings. // - // REPEAT (a retry that failed again): nothing new mounts, so the region - // carries it - and focus must NOT move, because the reader is on the Send - // button they just pressed and moving focus would re-read the paragraph - // over the announcement. - if (next === outcome) { + // REPEAT (a retry that failed the same way again): nothing new mounts, so + // the region carries it - and focus must NOT move, because the reader is on + // the Send button they just pressed and moving focus would re-read the + // paragraph over the announcement. + if (next === outcomeRef.current) { say('That did not send. Your note is still in the box - press Send to try again.'); } else { hush(); @@ -291,7 +417,12 @@ function Inner({ url, title }: PageFeedbackProps) { confirmation.current?.focus(); }, [attempts]); - const showBox = helpful !== null && outcome !== 'sent'; + const done = outcome === 'sent' || outcome === 'dispatched'; + const showBox = helpful !== null && !done; + // Which path a comment sent NOW would take, so the note can say where it + // goes before the reader sends it. Re-evaluated on every render (each + // keystroke is one), so it follows a reader who accepts cookies mid-visit. + const usesFallback = Boolean(feedbackEndpoint) && postHogStatus() === 'not-loaded'; const left = COMMENT_MAX - comment.length; return ( @@ -309,7 +440,7 @@ function Inner({ url, title }: PageFeedbackProps) { type="button" className={`${styles.vote} ${helpful === true ? styles.on : ''}`} aria-pressed={helpful === true} - aria-disabled={recorded} + aria-disabled={recorded || sending} onClick={() => vote(true)} > Helpful @@ -318,7 +449,7 @@ function Inner({ url, title }: PageFeedbackProps) { type="button" className={`${styles.vote} ${helpful === false ? styles.bad : ''}`} aria-pressed={helpful === false} - aria-disabled={recorded} + aria-disabled={recorded || sending} onClick={() => vote(false)} > Could be better @@ -334,13 +465,20 @@ function Inner({ url, title }: PageFeedbackProps) { {/* Sits BEFORE the row on purpose. It tells the reader to press Send, and a message placed after the button would have them tab out of the widget to reach it. */} - {outcome === 'undelivered' && ( + {outcome === 'refused' && (

That did not send: analytics is turned off or blocked in this browser, so nothing left this page. Your note is still in the box β€” press Send to try again, or copy it somewhere safe.

)} + {outcome === 'unreachable' && ( +

+ That could not be sent: the connection failed or something in this browser blocked + the request. Your note is still in the box β€” press Send to try again, or copy it + somewhere safe. +

+ )}