From afda191939496d474a1cacb139ddaf744f76a643 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Wed, 5 Aug 2026 11:08:19 -0700 Subject: [PATCH 01/10] docs: replace the concurrent mode note with a Suspense section (#778) The "Extra Experimental concurrent mode features" section claimed these features "will not be stable until sometime after React 18 is released". React 18 shipped in 2022 and concurrent mode was abandoned as a concept rather than stabilised, so the section pointed readers at a setup guide for a React feature that does not exist. All three of its reactjs.org/docs/concurrent-mode-* links were dead pages. Replaces it with a plain "## Suspense" section, adds a Suspense bullet to "What is ReactFire?", and documents that suspense is off by default, which the old text never stated (verified against src/firebaseApp.tsx, suspense ?? false). Also corrects the SuspenseWithPerf description. The old text said it instruments load times with Firebase Performance Monitoring and linked those docs, but src/performance.tsx uses only the browser User Timing API and still carries a "Should this import firebase/performance?" TODO. Caught by Armando in review. One concurrent-mode link remains in a comment in example/index.tsx, left for a separate follow-up. Closes #756. --- README.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 880b18ed..ae630e37 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ Firebase. auth state, realtime data, and all other Firebase SDK events. Plus, they automatically unsubscribe when your component unmounts. - **Access Firebase libraries from any component** - Need the Firestore SDK? `useFirestore`. Remote Config? `useRemoteConfig`. - **Safely configure Firebase libraries** - Libraries like Firestore and Remote Config require settings like `enablePersistence` to be set before any data fetches are made. This can be tough to support in React's world of re-renders. ReactFire gives you `useInitFirestore` and `useInitRemoteConfig` hooks that guarantee they're set before anything else. +- **Optional `` support** - Hand loading states to React instead of checking a status yourself. Off by default, opt in with the `suspense` prop. See [Suspense](#suspense) below. ## Platform support @@ -93,19 +94,16 @@ render( This repository is maintained by Googlers but is not a supported Firebase product. Issues here are answered by maintainers and other community members on GitHub on a best-effort basis. -### Extra Experimental [concurrent mode](https://reactjs.org/docs/concurrent-mode-suspense.html) features +## Suspense -These features are marked as *extra experimental* because they use experimental React features that [will not be stable until sometime after React 18 is released](https://github.com/reactwg/react-18/discussions/47#:~:text=Likely%20after%20React%2018.0%3A%20Suspense%20for%20Data%20Fetching). +ReactFire's hooks can throw promises for [``](https://react.dev/reference/react/Suspense) to catch, so React handles loading states for you instead of you checking `status` on each result. -- **Loading states handled by ``** - ReactFire's hooks throw promises - that Suspense can catch. Let React - [handle loading states for you](https://reactjs.org/docs/concurrent-mode-suspense.html). -- **Automatically instrument your `Suspense` load times** - Need to automatically instrument your `Suspense` load times with [RUM](https://firebase.google.com/docs/perf-mon)? Use ``. - -Enable concurrent mode features by following the [concurrent mode setup guide](https://reactjs.org/docs/concurrent-mode-adoption.html#installation) and then setting the `suspense` prop in `FirebaseAppProvider`: +This is **off by default**. Opt in with the `suspense` prop on `FirebaseAppProvider`: ```jsx ``` -See concurrent mode code samples in [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense) +`` does the same and also measures how long the fallback was shown, using the browser's [User Timing API](https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/User_timing). + +See [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense) for full samples. From f3223ed091a07aacd93c7455915243c8500fbb63 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Fri, 7 Aug 2026 10:58:06 -0700 Subject: [PATCH 02/10] ci: add a manual firestore flake probe for #776 (#780) * ci: add a manual firestore flake probe for #776 Every measurement of the #776 flake so far has been local, where the failure is a plain waitFor timeout with no gRPC error. In CI it arrives alongside a gRPC framing desync (RESOURCE_EXHAUSTED: Received message larger than max), which raises the possibility that the local repro and the CI failure are not the same bug. That matters, because the @grpc/grpc-js override proposed as the fix was measured only against the local one. This runs the firestore suite N times per arm, across both Node versions and both grpc-js versions, under CI conditions, so the comparison happens where the failure actually occurs. Notes on the design: - workflow_dispatch only. It never runs on a push, a PR or a schedule, so it costs nothing until someone asks for it. - A fresh emulator per iteration, matching how npm test runs in CI. Reusing one emulator across iterations would measure something else. - Failures are classified, not counted. Only the #776 assertion signature counts toward the rate; emulator start failures are reported separately, because folding them in previously inflated a local rate estimate by roughly 50%. - The job reports rather than fails. A red run here means the probe broke, not that the flake reproduced. - Inputs reach the script through env rather than interpolation, and iterations is validated before it reaches the loop. Classifier dry-run against synthetic logs covering pass, flake, flake-with-gRPC-error and infra-failure returns the expected counts and excludes infra failures from the rate. zizmor 1.25.2 reports no findings beyond the cache-poisoning rule CI suppresses. Refs #776 * ci: address review feedback on the firestore flake probe Six fixes from Armando's review on #780: - Pull `arm` out of the matrix. Both arms now run sequentially in one job on one runner, so the machine is held constant. That was the control the local measurements had and the workflow dropped, and the premise of the probe is that the machine matters. Arm order is forced baseline-then-override because applying the override mutates node_modules for everything after it. - Note in the job summary that this runs one emulator and one file, while CI runs five emulators and the whole suite in parallel, so a clean table is not a verdict on CI. - Count the flake/RESOURCE_EXHAUSTED overlap. Two independent totals could not answer whether the gRPC desync and the #776 assertion co-occur, which is the question the probe exists for. - Give hangs their own bucket. A test timeout produces no assertion line, so it was landing in `infra` and dropping out of the rate entirely. #776 reports a 120s hang. - Persist counts per arm and render the summary in an `always()` step. A run that hits `timeout-minutes` now still reports the arms that finished, instead of losing every count. Default iterations 20 -> 30, matching #776's power note. - Surface unrecognized failures. The flake match is a literal vitest assertion string, so a reword would have quietly turned every real flake into an infra failure. Unmatched assertion lines are now collected and shown as a warning in the summary. Also drops a `set +e` / `set -e` pair that turned errexit on partway through a script that never had it enabled. Verified: classifier dry-run over synthetic logs covering pass, flake, flake-with-gRPC, hang, infra and a reworded assertion returns the expected counts and routes the reword to the unmatched warning; arms input validation rejects empty, unknown and non-JSON values; the summary renders correctly from a partial counts file, which is the timeout-recovery path; zizmor 1.25.2 reports no findings beyond the suppressed cache-poisoning rule. --- .github/workflows/flake-probe.yaml | 281 +++++++++++++++++++++++++++++ 1 file changed, 281 insertions(+) create mode 100644 .github/workflows/flake-probe.yaml diff --git a/.github/workflows/flake-probe.yaml b/.github/workflows/flake-probe.yaml new file mode 100644 index 00000000..8d20ed64 --- /dev/null +++ b/.github/workflows/flake-probe.yaml @@ -0,0 +1,281 @@ +# Measures the `test/firestore.test.tsx` flake rate (#776) in CI rather than locally. +# +# Every measurement of this flake so far has been on a laptop, where the failure looks +# like a plain `waitFor` timeout. In CI it comes with a gRPC framing desync +# (`RESOURCE_EXHAUSTED: Received message larger than max`), which may mean the two are +# not the same bug. This runs both `@grpc/grpc-js` arms on both Node versions under CI +# conditions so the comparison is made where the failure actually happens. +# +# Both arms run inside a single job, sequentially on the same runner. They are +# deliberately NOT a matrix dimension: the whole premise is that the machine matters, +# so splitting the arms across two runners would reintroduce the variable being tested. +# +# Manual only. It never runs on a push, a PR or a schedule, so it costs nothing until +# someone asks for it. +name: Firestore flake probe + +on: + workflow_dispatch: + inputs: + iterations: + description: "Test runs per arm (each is a full emulator start/stop, roughly 25s)" + required: false + default: "30" + node_versions: + description: "JSON array of Node majors to probe" + required: false + default: '["22", "24"]' + arms: + description: "JSON array of grpc-js arms: baseline, override, or both" + required: false + default: '["baseline", "override"]' + +# Least privilege. This workflow reads the repo and writes nothing back. +permissions: + contents: read + +jobs: + probe: + runs-on: ubuntu-latest + timeout-minutes: 60 + strategy: + matrix: + node: ${{ fromJSON(inputs.node_versions) }} + fail-fast: false + name: Probe Node ${{ matrix.node }} + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false + + - name: Setup node + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: ${{ matrix.node }} + check-latest: true + cache: 'npm' + + - name: Setup Java + uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0 + with: + distribution: 'temurin' + java-version: '21' + + - name: Firebase emulator cache + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/firebase/emulators + key: firebase_emulators + + - name: Install deps + run: npm ci + + # Inputs and matrix values are passed through `env` rather than interpolated into + # the script body, so nothing from the dispatch form can be executed as shell. + # + # Counts are appended to `probe-counts.tsv` as each arm finishes, and the summary + # is rendered by a separate `always()` step. If the job hits its timeout partway + # through, whatever was measured before the wall still gets reported. + - name: Run the probe + env: + ITERATIONS: ${{ inputs.iterations }} + ARMS: ${{ inputs.arms }} + NODE_MAJOR: ${{ matrix.node }} + run: | + set -uo pipefail + + # Guard against a non-numeric or absurd `iterations` before it reaches the loop. + case "$ITERATIONS" in + ''|*[!0-9]*) echo "iterations must be a positive integer, got '$ITERATIONS'"; exit 1 ;; + esac + if [ "$ITERATIONS" -lt 1 ] || [ "$ITERATIONS" -gt 200 ]; then + echo "iterations must be between 1 and 200, got '$ITERATIONS'" + exit 1 + fi + + # Validate the arm list rather than trusting the dispatch form, and normalize + # it to a space-separated list. Order is forced baseline-then-override because + # applying the override mutates node_modules for everything after it. + ARM_LIST="$(node -e ' + const arms = JSON.parse(process.env.ARMS); + if (!Array.isArray(arms) || arms.length === 0) throw new Error("arms must be a non-empty JSON array"); + const allowed = ["baseline", "override"]; + for (const a of arms) if (!allowed.includes(a)) throw new Error("unknown arm: " + a); + process.stdout.write(allowed.filter((a) => arms.includes(a)).join(" ")); + ')" || exit 1 + + mkdir -p probe-logs + : > probe-counts.tsv + : > probe-unmatched.txt + + for arm in $ARM_LIST; do + echo "::group::Arm: $arm" + + # `npm pkg set` mangles keys containing a slash, so edit package.json directly. + # `npm install` (not `npm ci`) is required here because applying an override + # necessarily changes the lockfile. + if [ "$arm" = "override" ]; then + node -e ' + const fs = require("fs"); + const pkg = JSON.parse(fs.readFileSync("package.json", "utf8")); + pkg.overrides = { ...pkg.overrides, "@grpc/grpc-js": "^1.14.0" }; + fs.writeFileSync("package.json", JSON.stringify(pkg, null, 2) + "\n"); + ' + npm install --no-audit --no-fund + fi + + resolved="$(node -p "require('@grpc/grpc-js/package.json').version")" + echo "Arm $arm resolved @grpc/grpc-js: $resolved" + + pass=0 + flake=0 + infra=0 + hang=0 + grpc_err=0 + flake_with_grpc=0 + + for i in $(seq 1 "$ITERATIONS"); do + log="probe-logs/$arm-run-$i.log" + + # A fresh emulator per iteration, matching how `npm test` runs in CI. Reusing + # one emulator across iterations would measure a different thing. + npx firebase emulators:exec --only firestore --project=rxfire-525a3 \ + "npx vitest run firestore" > "$log" 2>&1 + rc=$? + + saw_grpc=0 + if grep -q "RESOURCE_EXHAUSTED: Received message larger than max" "$log"; then + grpc_err=$((grpc_err + 1)) + saw_grpc=1 + fi + + if [ "$rc" -eq 0 ]; then + pass=$((pass + 1)) + echo "run $i: PASS" + elif grep -q "expected 'loading' to deeply equal 'success'" "$log"; then + # The #776 signature specifically, rather than "the job went red". + flake=$((flake + 1)) + # Whether the gRPC desync and the #776 assertion co-occur is the whole + # question, so count the overlap rather than two independent totals. + if [ "$saw_grpc" -eq 1 ]; then + flake_with_grpc=$((flake_with_grpc + 1)) + fi + echo "run $i: FLAKE (rc=$rc)" + elif grep -qE "Test timed out in [0-9]+ms|Hook timed out in [0-9]+ms" "$log"; then + # #776 also reports a ~120s hang. A hang produces no assertion line, so + # without this bucket it would land in `infra` and vanish from the rate. + hang=$((hang + 1)) + echo "run $i: HANG (rc=$rc)" + else + # Emulator start failures and the like. Counted separately because folding + # them in previously inflated a local flake-rate estimate by ~50%. + infra=$((infra + 1)) + echo "run $i: INFRA FAILURE (rc=$rc), excluded from the rate" + # The flake match is a literal vitest assertion string. If vitest ever + # rewords it, every real flake would quietly become an infra failure, so + # surface the assertion line of anything unrecognized instead of hiding it. + if line="$(grep -m1 -E "AssertionError|expected .* to " "$log")"; then + printf '%s run %s: %s\n' "$arm" "$i" "$line" >> probe-unmatched.txt + fi + tail -20 "$log" + fi + done + + printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + "$NODE_MAJOR" "$arm" "$resolved" \ + "$pass" "$flake" "$infra" "$hang" "$grpc_err" "$flake_with_grpc" \ + >> probe-counts.tsv + + echo "arm=$arm node=$NODE_MAJOR pass=$pass flake=$flake infra=$infra hang=$hang grpc_err=$grpc_err overlap=$flake_with_grpc" + echo "::endgroup::" + done + + # Rendered separately, and on `always()`, so a job killed by `timeout-minutes` + # still reports every arm that finished before the wall. + - name: Summarize + if: ${{ always() }} + env: + NODE_MAJOR: ${{ matrix.node }} + run: | + set -uo pipefail + + if [ ! -s probe-counts.tsv ]; then + echo "No arm completed; nothing to summarize." >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + + while IFS=$'\t' read -r node arm resolved pass flake infra hang grpc_err overlap; do + counted=$((pass + flake)) + if [ "$counted" -gt 0 ]; then + # `< /dev/null` so the subshell cannot consume the loop's stdin. + rate="$(node -e "process.stdout.write(((${flake}/${counted})*100).toFixed(1))" < /dev/null)" + else + rate="n/a" + fi + + { + echo "### Node ${node} / ${arm} (@grpc/grpc-js ${resolved})" + echo "" + echo "| Outcome | Count |" + echo "| --- | --- |" + echo "| Pass | ${pass} |" + echo "| Flake (#776 signature) | ${flake} |" + echo "| ...of which also showed RESOURCE_EXHAUSTED | ${overlap} |" + echo "| Hang (test timeout, no assertion) | ${hang} |" + echo "| Infra failure (excluded) | ${infra} |" + echo "| Runs showing RESOURCE_EXHAUSTED | ${grpc_err} |" + echo "" + echo "**Flake rate: ${rate}% of ${counted} counted runs.**" + echo "" + if [ "$hang" -gt 0 ] || [ "$infra" -gt 0 ]; then + echo "> ${hang} hang(s) and ${infra} infra failure(s) are excluded from the rate." + echo "" + fi + } >> "$GITHUB_STEP_SUMMARY" + done < probe-counts.tsv + + { + echo "---" + echo "" + echo "**A clean table here is not a verdict on CI.** This probe runs one emulator" + echo "and one test file; \`npm run test\` in CI starts five emulators and runs the" + echo "whole suite with parallel workers. \`RESOURCE_EXHAUSTED\` has never appeared in" + echo "a firestore-only run, so this configuration may reproduce the local failure" + echo "while never reaching the CI one." + echo "" + } >> "$GITHUB_STEP_SUMMARY" + + if [ -s probe-unmatched.txt ]; then + { + echo "### ⚠️ Unrecognized failures" + echo "" + echo "These runs failed with an assertion the classifier does not know, so they" + echo "were counted as infra. If vitest reworded the #776 message, the flake counts" + echo "above are wrong and the pattern needs updating." + echo "" + echo '```' + cat probe-unmatched.txt + echo '```' + echo "" + } >> "$GITHUB_STEP_SUMMARY" + fi + + # The probe reports; it does not fail. A red job here would mean the probe + # broke, not that the flake reproduced. + total_counted="$(awk -F'\t' '{ s += $4 + $5 } END { print s + 0 }' probe-counts.tsv)" + if [ "$total_counted" -eq 0 ]; then + echo "Every run failed for infrastructure reasons; the probe measured nothing." + exit 1 + fi + + - name: Upload probe logs + if: ${{ always() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: probe-logs-node${{ matrix.node }} + path: | + probe-logs/ + probe-counts.tsv + probe-unmatched.txt + retention-days: 7 From 028909de0e3e1b51fc1bf77170da9ce1084ab889 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Fri, 7 Aug 2026 11:27:28 -0700 Subject: [PATCH 03/10] docs(example): drop the concurrent mode framing from the demo entry (#781) * docs(example): drop the concurrent mode framing from the demo entry Follow-up to #778, which removed the same obsolete premise from the README but left this copy of it. The comment told readers they need "an experimental build of React to use Concurrent mode" and linked reactjs.org/docs/concurrent-mode-adoption.html, a dead page. React 18 shipped in 2022 and concurrent mode was abandoned as a concept rather than stabilised, so the instruction could not be followed. The two commented-out react/experimental and react-dom/experimental imports existed only to serve that premise and go with it. Comments only. No active code changes, so the demo behaves identically. This removes the last reactjs.org reference in the repository. Refs #756 * docs(example): note that the Suspense path does not run as checked in Armando installed the example's pinned versions and confirmed the uncomment instruction cannot be followed: react-dom is pinned at 17.0.2, where createRoot is undefined at runtime and absent from the root react-dom types, and withSuspense/Firestore.tsx imports useTransition, which is also undefined on 17. Uncommenting the block alongside the existing ReactDOM.render call would also put two roots on one element. The previous wording named a precondition; the replacement read as a complete two-step procedure, which promised more than the file can deliver. This says what is missing instead. Still comments only. --- example/index.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/example/index.tsx b/example/index.tsx index 536f0242..dc047c14 100644 --- a/example/index.tsx +++ b/example/index.tsx @@ -2,13 +2,16 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; /** - * Use this instead of NonConcurrentModeApp to see a ReactFire demo with Suspense/Concurrent mode enabled + * This demo renders without Suspense. The Suspense version is the commented-out import + * below plus the render block at the bottom of this file. * - * You'll need to use an experimental build of React to use Concurrent mode - * https://reactjs.org/docs/concurrent-mode-adoption.html#installation + * That path does not run as checked in: it needs react and react-dom on 18 or later, which + * this example is not yet on, and the `ReactDOM.render` call below has to be replaced + * rather than left alongside it. See #781 for the details. + * + * Suspense is off by default in ReactFire and is opted into with the `suspense` prop on + * `FirebaseAppProvider`. See the Suspense section of the README. */ -// import {} from 'react/experimental' // make TS aware of experimental features -// import {} from 'react-dom/experimental' // make TS aware of experimental features // import { App as ConcurrentModeApp } from './withSuspense/App'; import { App as NonConcurrentModeApp } from './withoutSuspense/App'; import './index.css'; @@ -37,7 +40,7 @@ ReactDOM.render( ); /** - * FOR CONCURRENT MODE + * FOR THE SUSPENSE VERSION */ // ReactDOM.createRoot(rootElement).render( // From af33ca462e1e531ea5da1d6d41952a6257ff22e9 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Fri, 7 Aug 2026 15:15:24 -0700 Subject: [PATCH 04/10] ci(flake-probe): add a full-suite workload mode (#785) * ci(flake-probe): add a full-suite workload mode The first run of the probe (2026-08-07, run 31204989659) came back 120/120 clean in firestore-only mode with zero RESOURCE_EXHAUSTED. That configuration runs one emulator and one test file, while the #776 failures come from `npm run test`: five emulators and the whole suite. So the isolated suite does not reproduce either failure and cannot serve as a control for the @grpc/grpc-js comparison. Adds a `workload` input, defaulting to `full-suite`, which runs the same command CI runs. `firestore-only` is kept because it isolates the Firestore client and is roughly 3x faster per iteration. Also: - Install the functions deps in full-suite mode. `test.yaml` does this before `npm run test` and the functions emulator does not start without it. - Raise timeout-minutes to 180. Measured per-iteration cost is ~23s full-suite and ~8s firestore-only, so the default 30 per arm is ~28 minutes; the raise is headroom for the 200 cap, which is ~153 minutes full-suite. - Classify a failing `double check - emulator is running` as infra, ahead of the hang check. auth, firestore and database each open with that health check and it fails by timing out, so without this an emulator that never came up was counted as the #776 120s hang. Found by running the real suite on a machine where the RTDB emulator was unreachable, not by inspection. - Make the summary footer workload-aware, so a firestore-only table carries a warning pointing at the 120/120 result. Verified: one full-suite iteration run locally end to end (8 of 9 test files passing, the failure being the unreachable local RTDB emulator), and that real log fed through the classifier, which is what surfaced the health-check misclassification; regression cases for flake, hang, infra and a log where the health check passes alongside a real #776 assertion all classify unchanged; workload input rejects unknown values; zizmor 1.25.2 clean. Refs #776, #783. * ci(flake-probe): fix the errexit abort and scope counting to firestore Three fixes from Armando's review on #785. 1. Restore the `set +e` / `set -e` pair around `emulators:exec`. I removed it in 43f39e5 on the reasoning that the script never enables errexit itself. That is true and irrelevant: GitHub runs an undeclared `run:` step as `bash -e {0}`, so errexit is on from the invocation and `set -uo pipefail` does not clear it. The pair was load-bearing. Effect on main today: the first failing iteration kills the step before `rc=$?` is read. Nothing is classified, the arm's tally is never written, and probe-counts.tsv is empty or half written, so the summarize step reports nothing. The probe can only ever produce a clean table, and the first run that genuinely reproduces the flake is the one that reports least. Verified under `bash -e` before and after: without the guard not even the first iteration prints; with it, both arms tally with a failing iteration in each. The comment now says why it exists, because the reasoning that removed it was superficially sound. 2. Scope the flake, hang and health-check searches to firestore's output. This one comes from the widening. In firestore-only mode anything in the log was necessarily about #776. In full-suite mode it is not: `expected 'loading' to deeply equal 'success'` is just what vitest prints when a data hook's status assertion fails, and it appears in 6 of the 9 test files. Three separate miscounts followed, all confirmed against logs rather than argued: - a failure in another test file counted as a #776 flake - a timeout in any file counted as the #776 120s hang - a non-firestore emulator's health check outranked a genuine firestore flake in the same run, filing it as infra and dropping it from the rate entirely The FAIL line names the file and the assertion or timeout sits on the next line, verified against #781's real overnight failure, so -A1 is the right window. Captured into a variable rather than piped into `grep -q`, so an early-exit SIGPIPE cannot combine with pipefail and read as a silent no-match. The health check is now firestore-specific, so a non-firestore health failure falls through to the final branch and is recorded in probe-unmatched.txt rather than silently miscounted. That branch also captures the first FAIL line now, not only an assertion, so a genuine failure in another test file is visible too. 3. Raise timeout-minutes 180 -> 240. The 180 was sized on a flat 23s per iteration. Across recent CI runs that step ranges 20 to 31s, and a timeout cares about the slow tail: at 31s the 200 cap wants ~207 minutes. The default 30 per arm is safe under any reading; the ceiling exists for the cap, so it is now set past the pessimistic figure. Comment records the range and the reason. Verified: a 7-log corpus, one real (#781's overnight #776 failure) and six synthetic, classified under both the old and new logic. The three miscounts above are fixed with no regressions, and the real #776 failure still classifies as a flake. YAML parses; zizmor 1.25.2 clean. Refs #776, #783. --- .github/workflows/flake-probe.yaml | 156 +++++++++++++++++++++++++---- 1 file changed, 135 insertions(+), 21 deletions(-) diff --git a/.github/workflows/flake-probe.yaml b/.github/workflows/flake-probe.yaml index 8d20ed64..a9af29cc 100644 --- a/.github/workflows/flake-probe.yaml +++ b/.github/workflows/flake-probe.yaml @@ -10,6 +10,12 @@ # deliberately NOT a matrix dimension: the whole premise is that the machine matters, # so splitting the arms across two runners would reintroduce the variable being tested. # +# WORKLOAD. The first run of this probe (2026-08-07) came back 120/120 clean in +# `firestore-only` mode, with zero `RESOURCE_EXHAUSTED`. That configuration runs one +# emulator and one test file, while the failures in #776 come from `npm run test`: five +# emulators and the whole suite. So the isolated suite does not reproduce either failure +# and cannot serve as a control. `full-suite` runs the actual CI workload instead. +# # Manual only. It never runs on a push, a PR or a schedule, so it costs nothing until # someone asks for it. name: Firestore flake probe @@ -17,8 +23,16 @@ name: Firestore flake probe on: workflow_dispatch: inputs: + workload: + description: "Which workload to run each iteration" + required: false + type: choice + default: "full-suite" + options: + - "full-suite" + - "firestore-only" iterations: - description: "Test runs per arm (each is a full emulator start/stop, roughly 25s)" + description: "Test runs per arm (full-suite ~23s each, firestore-only ~8s each)" required: false default: "30" node_versions: @@ -37,7 +51,20 @@ permissions: jobs: probe: runs-on: ubuntu-latest - timeout-minutes: 60 + # Sized 2026-08-07 on the `Run tests` step of the real CI job (not the whole job, + # which is ~57s including install and setup). + # + # ⚠️ Do NOT size this on a single figure. Across recent CI runs that step lands + # anywhere from 20 to 31 seconds, and a timeout cares about the slow tail, not the + # median. At 31s the 200 cap wants ~207 minutes, which a flat-23s estimate (~153) + # would have put comfortably inside 180. And if #776's reported ~120s hang ever + # reproduces, those iterations cost far more than any of this. + # + # The default 30 per arm is ~28-38 minutes full-suite and is safe under any reading. + # This ceiling exists for the 200 cap, so it is set past the pessimistic figure + # rather than the average one. The `always()` summary step means a run that does hit + # the wall still reports the arms that finished. + timeout-minutes: 240 strategy: matrix: node: ${{ fromJSON(inputs.node_versions) }} @@ -71,6 +98,14 @@ jobs: - name: Install deps run: npm ci + # The full suite starts the functions emulator, which will not come up without + # these. `test.yaml` does the same thing before `npm run test`. Skipped in + # firestore-only mode, where no functions emulator is started. + - name: Install deps for functions + if: ${{ inputs.workload == 'full-suite' }} + run: npm install --no-audit --no-fund + working-directory: ./functions + # Inputs and matrix values are passed through `env` rather than interpolated into # the script body, so nothing from the dispatch form can be executed as shell. # @@ -81,8 +116,15 @@ jobs: env: ITERATIONS: ${{ inputs.iterations }} ARMS: ${{ inputs.arms }} + WORKLOAD: ${{ inputs.workload }} NODE_MAJOR: ${{ matrix.node }} run: | + # ⚠️ errexit is ON here even though nothing below turns it on: GitHub runs an + # undeclared `run:` step as `bash -e {0}`, and `set -uo pipefail` does not + # disable it. Every command that is ALLOWED to fail therefore has to say so. + # Getting this wrong means the step dies on the first failing iteration and + # the probe can only ever report a clean table, which is the one failure mode + # that makes the whole workflow useless. See the loop below. set -uo pipefail # Guard against a non-numeric or absurd `iterations` before it reaches the loop. @@ -94,6 +136,25 @@ jobs: exit 1 fi + # `full-suite` reproduces what `npm run test` does in CI: every emulator in + # firebase.json, every test file. `firestore-only` is the narrower original, + # kept because it isolates the Firestore client and is ~3x faster per run. + case "$WORKLOAD" in + full-suite) + EMULATOR_ARGS="" + VITEST_ARGS="" + ;; + firestore-only) + EMULATOR_ARGS="--only firestore" + VITEST_ARGS="firestore" + ;; + *) + echo "workload must be full-suite or firestore-only, got '$WORKLOAD'" + exit 1 + ;; + esac + echo "Workload: $WORKLOAD" + # Validate the arm list rather than trusting the dispatch form, and normalize # it to a space-separated list. Order is forced baseline-then-override because # applying the override mutates node_modules for everything after it. @@ -138,11 +199,22 @@ jobs: for i in $(seq 1 "$ITERATIONS"); do log="probe-logs/$arm-run-$i.log" - # A fresh emulator per iteration, matching how `npm test` runs in CI. Reusing - # one emulator across iterations would measure a different thing. - npx firebase emulators:exec --only firestore --project=rxfire-525a3 \ - "npx vitest run firestore" > "$log" 2>&1 + # A fresh emulator start per iteration, matching how `npm test` runs in CI. + # Reusing one emulator across iterations would measure a different thing. + # Unquoted on purpose: both are either empty or a fixed literal set above, + # never user input. + # + # ⚠️ `set +e` is LOAD-BEARING, do not remove it. This command failing is the + # entire point of the probe, but the step runs under `bash -e`, so without + # this the first flake kills the step before `rc` is even read: no + # classification, no tally for the arm, and an empty or half-written + # probe-counts.tsv. It was removed once on the reasoning that the script + # never sets `-e` itself, which is true and irrelevant. + set +e + npx firebase emulators:exec $EMULATOR_ARGS --project=rxfire-525a3 \ + "npx vitest run $VITEST_ARGS" > "$log" 2>&1 rc=$? + set -e saw_grpc=0 if grep -q "RESOURCE_EXHAUSTED: Received message larger than max" "$log"; then @@ -150,11 +222,41 @@ jobs: saw_grpc=1 fi + # ⚠️ EVERY QUESTION BELOW IS SCOPED TO FIRESTORE'S OWN OUTPUT, and it has to + # be. In firestore-only mode anything in the log was necessarily about #776. + # In full-suite mode that is false: `expected 'loading' to deeply equal + # 'success'` is just what vitest prints when a data hook's status assertion + # fails, and it appears in 6 of the 9 test files, 40 times over. An unscoped + # search counts a slow storage upload or a functions failure as a #776 flake. + # + # In a vitest log the FAIL line names the file and the assertion or timeout + # sits on the NEXT line, verified against #781's real overnight failure, so + # -A1 is the correct window. Captured into a variable rather than piped into + # `grep -q`, because an early-exiting `grep -q` can SIGPIPE its producer and + # `pipefail` would turn that 141 into a silent "no match". + fs_fails="$(grep -A1 -E "FAIL.*test/firestore\.test\.tsx" "$log" || true)" + if [ "$rc" -eq 0 ]; then pass=$((pass + 1)) echo "run $i: PASS" - elif grep -q "expected 'loading' to deeply equal 'success'" "$log"; then - # The #776 signature specifically, rather than "the job went red". + elif grep -qE "FAIL.*test/firestore\.test\.tsx.*double check - emulator is running" "$log"; then + # `test/{auth,firestore,database}.test.tsx` each open with an emulator + # health check. If FIRESTORE's fails, its emulator did not come up and no + # firestore result this iteration means anything, so the run is void. + # + # ⚠️ Scoped to firestore deliberately. An unscoped check let ANY emulator's + # health failure outrank a real firestore flake in the same run, filing it + # as infra and dropping it from the rate. A non-firestore health failure + # now falls through to the final `else`, where it is still excluded but is + # recorded in probe-unmatched.txt instead of being silently miscounted. + # + # This must precede the hang check either way: a health check fails BY + # timing out, so it would otherwise read as the #776 120s hang. + infra=$((infra + 1)) + echo "run $i: INFRA FAILURE (rc=$rc), firestore emulator health check failed, excluded from the rate" + tail -20 "$log" + elif grep -q "expected 'loading' to deeply equal 'success'" <<< "$fs_fails"; then + # The #776 signature, in firestore's output specifically. flake=$((flake + 1)) # Whether the gRPC desync and the #776 assertion co-occur is the whole # question, so count the overlap rather than two independent totals. @@ -162,20 +264,24 @@ jobs: flake_with_grpc=$((flake_with_grpc + 1)) fi echo "run $i: FLAKE (rc=$rc)" - elif grep -qE "Test timed out in [0-9]+ms|Hook timed out in [0-9]+ms" "$log"; then + elif grep -qE "Test timed out in [0-9]+ms|Hook timed out in [0-9]+ms" <<< "$fs_fails"; then # #776 also reports a ~120s hang. A hang produces no assertion line, so # without this bucket it would land in `infra` and vanish from the rate. + # Scoped like the flake check: a timeout in any other test file is not + # the #776 hang and must not be presented as one. hang=$((hang + 1)) echo "run $i: HANG (rc=$rc)" else - # Emulator start failures and the like. Counted separately because folding - # them in previously inflated a local flake-rate estimate by ~50%. + # Everything else: emulator start failures, a non-firestore health check, + # a failure in another test file. Counted separately because folding them + # in previously inflated a local flake-rate estimate by ~50%. infra=$((infra + 1)) echo "run $i: INFRA FAILURE (rc=$rc), excluded from the rate" - # The flake match is a literal vitest assertion string. If vitest ever - # rewords it, every real flake would quietly become an infra failure, so - # surface the assertion line of anything unrecognized instead of hiding it. - if line="$(grep -m1 -E "AssertionError|expected .* to " "$log")"; then + # Two ways to land here that must not be silent: vitest rewording the #776 + # assertion (which would turn every real flake into an infra failure), and + # a genuine failure in another test file. Record the first FAIL line from + # anywhere in the log, not just firestore's, so both are visible. + if line="$(grep -m1 -E "FAIL |AssertionError|expected .* to " "$log")"; then printf '%s run %s: %s\n' "$arm" "$i" "$line" >> probe-unmatched.txt fi tail -20 "$log" @@ -197,6 +303,7 @@ jobs: if: ${{ always() }} env: NODE_MAJOR: ${{ matrix.node }} + WORKLOAD: ${{ inputs.workload }} run: | set -uo pipefail @@ -215,7 +322,7 @@ jobs: fi { - echo "### Node ${node} / ${arm} (@grpc/grpc-js ${resolved})" + echo "### Node ${node} / ${arm} / ${WORKLOAD} (@grpc/grpc-js ${resolved})" echo "" echo "| Outcome | Count |" echo "| --- | --- |" @@ -238,11 +345,18 @@ jobs: { echo "---" echo "" - echo "**A clean table here is not a verdict on CI.** This probe runs one emulator" - echo "and one test file; \`npm run test\` in CI starts five emulators and runs the" - echo "whole suite with parallel workers. \`RESOURCE_EXHAUSTED\` has never appeared in" - echo "a firestore-only run, so this configuration may reproduce the local failure" - echo "while never reaching the CI one." + if [ "$WORKLOAD" = "firestore-only" ]; then + echo "⚠️ **A clean table here is not a verdict on CI.** This ran one emulator and" + echo "one test file; \`npm run test\` in CI starts five emulators and runs the whole" + echo "suite with parallel workers. The 2026-08-07 run of this mode came back 120/120" + echo "clean with zero \`RESOURCE_EXHAUSTED\`, so this configuration is not known to" + echo "reproduce either the local or the CI failure. Prefer \`full-suite\`." + else + echo "This ran the same workload as CI: every emulator in \`firebase.json\` and the" + echo "whole test suite, one fresh emulator start per iteration. A failure in any" + echo "test file counts, but only the #776 assertion signature counts toward the" + echo "flake rate; anything else is reported separately and listed below." + fi echo "" } >> "$GITHUB_STEP_SUMMARY" From e60b7ac3b34e401a7068c83e17a299d9ff5f5920 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:32:55 -0700 Subject: [PATCH 05/10] chore(deps-dev): bump hono from 4.12.27 to 4.13.1 (#786) Bumps [hono](https://github.com/honojs/hono) from 4.12.27 to 4.13.1. - [Release notes](https://github.com/honojs/hono/releases) - [Commits](https://github.com/honojs/hono/compare/v4.12.27...v4.13.1) --- updated-dependencies: - dependency-name: hono dependency-version: 4.13.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5e7c64cb..818f2eac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9118,9 +9118,9 @@ } }, "node_modules/hono": { - "version": "4.12.27", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.27.tgz", - "integrity": "sha512-1yrb/+w6HWQJrUCLkJ2IF5jNIPvvFkblV5RNOYl6bV+OA6p9GLcMpHFFGTosSvHvcAUibuUukRqhlYI4z32C7Q==", + "version": "4.13.1", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.1.tgz", + "integrity": "sha512-kdJoFVv2xmayw6cY09H7AbMJMt8Jn5jdlEdXsP7AGBdF2DIptVlKlOLKXP41yPip4/a3yQPv9gVcJYI8YY04dw==", "dev": true, "license": "MIT", "engines": { From bad1433b6cb16fcb30375b31d6f0f6f1223b55f5 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Tue, 11 Aug 2026 15:01:23 -0700 Subject: [PATCH 06/10] ci(probe): record emulator health-check durations per iteration (#787) The grpc-js override was measured and does not fix the flake (baseline 5/60 vs override 4/60, p = 1.0), so the default arm list drops to baseline alone. The arm stays available as a dispatch input. Each iteration records the firestore, auth and database health-check durations into probe-iterations.tsv, and the summary compares distributions across outcomes: each arm reported separately, every cell carrying its own count of measured iterations, mean of the two middle values at even n. The timing hypothesis this was built to test did not survive it: the recovered durations show no separation between passing and failing runs (Armando's recovery, #776), and the flake mechanism is a Listen-stream desync answered with a 60-second maximum backoff (#776). The probe's remaining job is proving the long-polling mitigation, which baseline-only dispatch makes a single 30-iteration run. Durations come from vitest's json reporter rather than the log: the default reporter prints a per-test line only over the 300ms slow threshold and for every test in a failing file, so the log drops auth entirely. Extraction is scoped by file path. Refs #776 --- .github/workflows/flake-probe.yaml | 129 +++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 5 deletions(-) diff --git a/.github/workflows/flake-probe.yaml b/.github/workflows/flake-probe.yaml index a9af29cc..7ada538a 100644 --- a/.github/workflows/flake-probe.yaml +++ b/.github/workflows/flake-probe.yaml @@ -16,6 +16,10 @@ # emulators and the whole suite. So the isolated suite does not reproduce either failure # and cannot serve as a control. `full-suite` runs the actual CI workload instead. # +# WHAT IT MEASURES NOW (2026-08-10). The grpc-js override was tested and does not fix the +# flake, so the open question is timing: whether the iterations that fail are the ones +# where the emulators were slow. Each iteration records its health-check durations. +# # Manual only. It never runs on a push, a PR or a schedule, so it costs nothing until # someone asks for it. name: Firestore flake probe @@ -42,7 +46,9 @@ on: arms: description: "JSON array of grpc-js arms: baseline, override, or both" required: false - default: '["baseline", "override"]' + # Baseline alone by default since 2026-08-10: the override was measured and does + # not fix the flake (#776). Kept as an option rather than deleted. + default: '["baseline"]' # Least privilege. This workflow reads the repo and writes nothing back. permissions: @@ -169,6 +175,7 @@ jobs: mkdir -p probe-logs : > probe-counts.tsv : > probe-unmatched.txt + : > probe-iterations.tsv for arm in $ARM_LIST; do echo "::group::Arm: $arm" @@ -198,6 +205,7 @@ jobs: for i in $(seq 1 "$ITERATIONS"); do log="probe-logs/$arm-run-$i.log" + json="probe-logs/$arm-run-$i.json" # A fresh emulator start per iteration, matching how `npm test` runs in CI. # Reusing one emulator across iterations would measure a different thing. @@ -210,12 +218,50 @@ jobs: # classification, no tally for the arm, and an empty or half-written # probe-counts.tsv. It was removed once on the reasoning that the script # never sets `-e` itself, which is true and irrelevant. + # ⚠️ The json reporter is what makes the timing usable; do not read durations + # out of the human log instead. The default reporter prints a per-test line + # only above its 300ms slow threshold, pass or fail, so the log drops auth + # entirely and censors the fast end of firestore and database. A comparison + # across outcomes needs every iteration, not the slow ones. set +e npx firebase emulators:exec $EMULATOR_ARGS --project=rxfire-525a3 \ - "npx vitest run $VITEST_ARGS" > "$log" 2>&1 + "npx vitest run $VITEST_ARGS --reporter=default --reporter=json --outputFile.json=$json" > "$log" 2>&1 rc=$? set -e + # Health-check duration per emulator, in ms: firestore, auth, database. All + # three so a slow firestore round trip can be told apart from a slow runner. + # + # ⚠️ Scoped by FILE, not by test title. All three files open with a test named + # `double check - emulator is running`, so matching the title alone records + # whichever one vitest emitted first. + # + # `na` when the json is missing or unparseable, and for auth and database in + # firestore-only mode. Never read it as a fast run. + health_line="$(node -e ' + const fs = require("fs"); + const p = process.argv[1]; + const files = ["test/firestore.test.tsx", "test/auth.test.tsx", "test/database.test.tsx"]; + const out = files.map(() => "na"); + if (!fs.existsSync(p)) { process.stdout.write(out.join("\t")); process.exit(0); } + let report; + try { report = JSON.parse(fs.readFileSync(p, "utf8")); } + catch { process.stdout.write(out.join("\t")); process.exit(0); } + for (const file of report.testResults || []) { + const idx = files.findIndex((f) => String(file.name || "").includes(f)); + if (idx === -1) continue; + for (const a of file.assertionResults || []) { + if (a.title === "double check - emulator is running" && typeof a.duration === "number") { + out[idx] = String(Math.round(a.duration)); + break; + } + } + } + process.stdout.write(out.join("\t")); + ' "$json" 2>/dev/null || true)" + [ -n "$health_line" ] || health_line="$(printf 'na\tna\tna')" + IFS=$'\t' read -r health_ms health_auth health_db <<< "$health_line" + saw_grpc=0 if grep -q "RESOURCE_EXHAUSTED: Received message larger than max" "$log"; then grpc_err=$((grpc_err + 1)) @@ -238,7 +284,8 @@ jobs: if [ "$rc" -eq 0 ]; then pass=$((pass + 1)) - echo "run $i: PASS" + outcome=pass + echo "run $i: PASS (firestore health check ${health_ms}ms)" elif grep -qE "FAIL.*test/firestore\.test\.tsx.*double check - emulator is running" "$log"; then # `test/{auth,firestore,database}.test.tsx` each open with an emulator # health check. If FIRESTORE's fails, its emulator did not come up and no @@ -253,6 +300,7 @@ jobs: # This must precede the hang check either way: a health check fails BY # timing out, so it would otherwise read as the #776 120s hang. infra=$((infra + 1)) + outcome=infra echo "run $i: INFRA FAILURE (rc=$rc), firestore emulator health check failed, excluded from the rate" tail -20 "$log" elif grep -q "expected 'loading' to deeply equal 'success'" <<< "$fs_fails"; then @@ -263,19 +311,22 @@ jobs: if [ "$saw_grpc" -eq 1 ]; then flake_with_grpc=$((flake_with_grpc + 1)) fi - echo "run $i: FLAKE (rc=$rc)" + outcome=flake + echo "run $i: FLAKE (rc=$rc, firestore health check ${health_ms}ms)" elif grep -qE "Test timed out in [0-9]+ms|Hook timed out in [0-9]+ms" <<< "$fs_fails"; then # #776 also reports a ~120s hang. A hang produces no assertion line, so # without this bucket it would land in `infra` and vanish from the rate. # Scoped like the flake check: a timeout in any other test file is not # the #776 hang and must not be presented as one. hang=$((hang + 1)) - echo "run $i: HANG (rc=$rc)" + outcome=hang + echo "run $i: HANG (rc=$rc, firestore health check ${health_ms}ms)" else # Everything else: emulator start failures, a non-firestore health check, # a failure in another test file. Counted separately because folding them # in previously inflated a local flake-rate estimate by ~50%. infra=$((infra + 1)) + outcome=infra echo "run $i: INFRA FAILURE (rc=$rc), excluded from the rate" # Two ways to land here that must not be silent: vitest rewording the #776 # assertion (which would turn every real flake into an infra failure), and @@ -286,6 +337,14 @@ jobs: fi tail -20 "$log" fi + + # One row per iteration, so the health-check duration can be compared across + # outcomes rather than only totalled. The per-arm counts below stay as they + # were; this is additive. + printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + "$NODE_MAJOR" "$arm" "$i" "$outcome" \ + "$health_ms" "$health_auth" "$health_db" "$saw_grpc" \ + >> probe-iterations.tsv done printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ @@ -360,6 +419,65 @@ jobs: echo "" } >> "$GITHUB_STEP_SUMMARY" + # Firestore's health check is a bare `addDoc` round trip and every `waitFor` in + # test/firestore.test.tsx uses the 1000ms default, so a slow round trip would + # explain the failures. Compares distributions, not flake events: a run yields + # few flakes but records a duration every iteration. It describes; overlapping + # ranges are a real answer, not a failed one. + if [ -s probe-iterations.tsv ]; then + { + echo "### Firestore emulator health check, by outcome" + echo "" + node -e ' + const fs = require("fs"); + const rows = fs.readFileSync("probe-iterations.tsv", "utf8").trim().split("\n").filter(Boolean) + .map((l) => l.split("\t")) + .map(([node, arm, i, outcome, fsMs, authMs, dbMs, grpc]) => ({ arm, outcome, fsMs, authMs, dbMs })); + // Even n takes the mean of the two middle values. Taking the upper made + // the median and max cells print the same number at n = 2, and the flake + // row is where n is smallest. + const median = (a) => { + const m = a.length >> 1; + return a.length % 2 ? a[m] : Math.round((a[m - 1] + a[m]) / 2); + }; + // Each cell carries its own n: the row count includes iterations that + // recorded no duration, so a row of 5 can rest on 2 measurements. + const series = (rs, key) => { + const a = rs.map((r) => Number(r[key])).filter((n) => Number.isFinite(n)).sort((x, y) => x - y); + return a.length ? `${median(a)} / ${a[a.length - 1]} (n=${a.length})` : "-"; + }; + // One table per arm. Pooling them would put baseline and override into one + // distribution while the counts tables above stay per-arm. + const arms = [...new Set(rows.map((r) => r.arm))]; + for (const arm of arms) { + const armRows = rows.filter((r) => r.arm === arm); + if (arms.length > 1) { console.log(`Arm: ${arm}`); console.log(""); } + console.log("Median / max, in ms, with the count of iterations that recorded one."); + console.log(""); + console.log("| Outcome | runs | firestore | auth | database |"); + console.log("| --- | --- | --- | --- | --- |"); + for (const name of ["pass", "flake", "hang", "infra"]) { + const rs = armRows.filter((r) => r.outcome === name); + if (!rs.length) continue; + console.log(`| ${name} | ${rs.length} | ${series(rs, "fsMs")} | ${series(rs, "authMs")} | ${series(rs, "dbMs")} |`); + } + console.log(""); + const missing = armRows.filter((r) => !Number.isFinite(Number(r.fsMs))) + .reduce((m, r) => m.set(r.outcome, (m.get(r.outcome) || 0) + 1), new Map()); + if (missing.size) { + const parts = [...missing].map(([outcome, n]) => `${n} ${outcome}`).join(", "); + console.log(`> No firestore duration recorded for ${parts}, usually because the emulator never came up.`); + console.log(""); + } + } + console.log("> Read the columns against each other. Firestore slow while auth and database"); + console.log("> stay flat points at the Firestore client or its stream; all three rising"); + console.log("> together points at runner-wide contention instead, which is a different bug."); + ' || echo "(could not summarize durations)" + echo "" + } >> "$GITHUB_STEP_SUMMARY" + fi + if [ -s probe-unmatched.txt ]; then { echo "### ⚠️ Unrecognized failures" @@ -392,4 +510,5 @@ jobs: probe-logs/ probe-counts.tsv probe-unmatched.txt + probe-iterations.tsv retention-days: 7 From 9dc9bf8936fba180e8dd8f6965ae54f0eadb7fc0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:17:43 -0700 Subject: [PATCH 07/10] chore(deps-dev): bump @hono/node-server from 1.19.14 to 1.19.17 (#792) Bumps [@hono/node-server](https://github.com/honojs/node-server) from 1.19.14 to 1.19.17. - [Release notes](https://github.com/honojs/node-server/releases) - [Commits](https://github.com/honojs/node-server/compare/v1.19.14...v1.19.17) --- updated-dependencies: - dependency-name: "@hono/node-server" dependency-version: 1.19.17 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 818f2eac..e0f85bdf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1361,9 +1361,9 @@ } }, "node_modules/@hono/node-server": { - "version": "1.19.14", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.14.tgz", - "integrity": "sha512-GwtvgtXxnWsucXvbQXkRgqksiH2Qed37H9xHZocE5sA3N8O8O8/8FA3uclQXxXVzc9XBZuEOMK7+r02FmSpHtw==", + "version": "1.19.17", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.17.tgz", + "integrity": "sha512-dSneS5qhiauZWGDCeK4o695Xd9nUNjviSZCMQrj10eetr8Uln1ucn6bbphOM6UynAMMtNIzZNSpL9vnASJwrPQ==", "dev": true, "license": "MIT", "engines": { From 23da94e53fd3e15978d33ab0bf11745ba2f70b98 Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Mon, 17 Aug 2026 12:34:49 -0700 Subject: [PATCH 08/10] test(firestore): let the offline fallback rescue two cache-miss tests (#791) test/firestore.test.tsx flakes in CI at roughly 18% per run (11 of 60 iterations, 2026-08-11). The Firestore emulator intermittently corrupts a Listen frame, so grpc-js reads four body bytes as a length prefix and reports RESOURCE_EXHAUSTED with an absurd size. The SDK special-cases that code with backoff.resetToMax(), parking the stream on a 60 second maximum backoff. This is an unresolved upstream emulator bug, firebase/firebase-tools#8654. There is no fixed version to pin, so the goal is to survive it rather than prevent it. A reconnect is not what rescues the affected tests. The same failure sends the client to OnlineState.Offline after ONLINE_STATE_TIMEOUT_MS (10s), and an offline client raises the pending snapshot from the local cache, empty cache included. Two tests assert that a document is absent, which is exactly what the empty cache reports, so they reach success about ten seconds after the failure with no server involved. They are the only two in the file whose first snapshot cannot be served from local data. Those two currently abandon the wait after one second, before the fallback can fire, and vitest would kill them at five seconds regardless. Raising both ceilings lets the fallback do its work. waitFor polls every 50ms, so a larger budget costs nothing when the stream is healthy: healthy runs measure 80-95ms (9 samples) and 90-140ms (11 samples) per test with the change, against 82-136ms and 101-120ms without it across 3 each. The bands overlap; the wider upper tail on the larger sample is sampling, not cost. The budget is 120s, far above the ~10s the fallback needs, because a ceiling is not a delay. Each test's own timeout clears the sum of the budgets beneath it, or only the first wait could ever spend one. The third wait in useFirestoreDocOnce keeps the 1000ms default deliberately. It waits on the client's own write, raised from the local cache before the acknowledgement returns; measured at 8ms with the client offline, against a control that fails at 1000ms when no write is issued. Verified: the desync reproduced locally twice during this work, having never been seen off CI before, and was rescued both times. One of the two has a full log, carrying both the RESOURCE_EXHAUSTED line and the maximum backoff line, with the vulnerable test taking 9878ms and passing; the other is a 9872ms sample from a batch that discarded output. CI showed the same rescue on this branch at 9795ms. Note that all three observations are with the fix in place: that the old ceilings would have killed a 9878ms wait follows from testing-library's 1000ms default and vitest's 5000ms, not from an observed failure. A simulated 65 second stall survives the new budget while failing under the default, and the per-test ceilings were confirmed to apply by shortening one until it failed. Both typechecks clean and the firestore suite green against the emulator. Scoped cost: a genuine regression in those two tests now takes up to 120s to surface instead of 1s. Refs #776 --- test/firestore.test.tsx | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/test/firestore.test.tsx b/test/firestore.test.tsx index 17ced14d..0dcf0c23 100644 --- a/test/firestore.test.tsx +++ b/test/firestore.test.tsx @@ -30,6 +30,22 @@ describe('Firestore', () => { ); + // The Firestore emulator intermittently corrupts a Listen frame + // (firebase/firebase-tools#8654, unresolved upstream). The SDK reads it as + // RESOURCE_EXHAUSTED and parks the stream on a 60s maximum backoff, but a + // reconnect is not what rescues these tests: the same failure drives the + // client to OnlineState.Offline after ONLINE_STATE_TIMEOUT_MS (10s), and an + // offline client raises the pending snapshot from the local cache, empty + // cache included. These two tests assert that a document is absent, which is + // what the empty cache reports, so they go green at ~10s with no server + // involved. They are the only two whose first snapshot cannot be served from + // local data. The budget is a ceiling, not a delay, since `waitFor` polls. + // Remove when #8654 is fixed upstream. See #776. + const WAIT_FOR_OFFLINE_FALLBACK = 120_000; + // vitest enforces its own per-test ceiling, so each test below gets more than + // the sum of the budgets under it; otherwise only the first `waitFor` could + // ever spend what it is given. + afterEach(async () => { cleanup(); @@ -108,11 +124,11 @@ describe('Firestore', () => { const { result } = renderHook(() => useFirestoreDocData(ref, { idField: 'id' }), { wrapper: Provider }); - await waitFor(() => expect(result.current.status).toEqual('success')); + await waitFor(() => expect(result.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); expect(result.current.status).toEqual('success'); expect(result.current.data).toBeUndefined(); - }); + }, 150_000); it('goes back into a loading state if you swap the query', async () => { const mockData = { a: 'hello' }; @@ -177,17 +193,20 @@ describe('Firestore', () => { const { result: subscribeResult } = renderHook(() => useFirestoreDoc(ref), { wrapper: Provider }); const { result: onceResult } = renderHook(() => useFirestoreDocOnce(ref), { wrapper: Provider }); - await waitFor(() => expect(subscribeResult.current.status).toEqual('success')); - await waitFor(() => expect(onceResult.current.status).toEqual('success')); + await waitFor(() => expect(subscribeResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); + await waitFor(() => expect(onceResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); expect(onceResult.current.data.exists()).toEqual(false); await act(() => setDoc(ref, { a: 'test' })); + // No budget: this waits on the client's own write, which is raised from + // the local cache before the acknowledgement returns (measured at 8ms + // with the client offline). await waitFor(() => expect(subscribeResult.current.data.exists()).toEqual(true)); expect(onceResult.current.data.exists()).toEqual(false); - }); + }, 270_000); }); describe('useFirestoreDocDataOnce', () => { From b2201e1e6851d5d0f73469c78f11222e3e98e1ae Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Wed, 19 Aug 2026 09:46:22 -0700 Subject: [PATCH 09/10] chore(ci): remove the firestore flake probe workflow (#795) The probe was added in #780 as a temporary measurement tool for #776, which is now closed. It has no callers: nothing outside the file referenced it, and it was workflow_dispatch only, so removing it changes no scheduled or PR-triggered behaviour. #783's removal conditions were written when the @grpc/grpc-js override was still the candidate fix. That override was measured and did not work (4/60 for the 1.14.4 override against a 5/60 baseline on 1.9.16, Fisher p = 1.0), so it never landed. The fix was #791's timeout ceilings instead, and the probe verified it: 0/60 flakes against an 11/60 baseline, with the desync rate statistically unchanged. Closes #783 --- .github/workflows/flake-probe.yaml | 514 ----------------------------- 1 file changed, 514 deletions(-) delete mode 100644 .github/workflows/flake-probe.yaml diff --git a/.github/workflows/flake-probe.yaml b/.github/workflows/flake-probe.yaml deleted file mode 100644 index 7ada538a..00000000 --- a/.github/workflows/flake-probe.yaml +++ /dev/null @@ -1,514 +0,0 @@ -# Measures the `test/firestore.test.tsx` flake rate (#776) in CI rather than locally. -# -# Every measurement of this flake so far has been on a laptop, where the failure looks -# like a plain `waitFor` timeout. In CI it comes with a gRPC framing desync -# (`RESOURCE_EXHAUSTED: Received message larger than max`), which may mean the two are -# not the same bug. This runs both `@grpc/grpc-js` arms on both Node versions under CI -# conditions so the comparison is made where the failure actually happens. -# -# Both arms run inside a single job, sequentially on the same runner. They are -# deliberately NOT a matrix dimension: the whole premise is that the machine matters, -# so splitting the arms across two runners would reintroduce the variable being tested. -# -# WORKLOAD. The first run of this probe (2026-08-07) came back 120/120 clean in -# `firestore-only` mode, with zero `RESOURCE_EXHAUSTED`. That configuration runs one -# emulator and one test file, while the failures in #776 come from `npm run test`: five -# emulators and the whole suite. So the isolated suite does not reproduce either failure -# and cannot serve as a control. `full-suite` runs the actual CI workload instead. -# -# WHAT IT MEASURES NOW (2026-08-10). The grpc-js override was tested and does not fix the -# flake, so the open question is timing: whether the iterations that fail are the ones -# where the emulators were slow. Each iteration records its health-check durations. -# -# Manual only. It never runs on a push, a PR or a schedule, so it costs nothing until -# someone asks for it. -name: Firestore flake probe - -on: - workflow_dispatch: - inputs: - workload: - description: "Which workload to run each iteration" - required: false - type: choice - default: "full-suite" - options: - - "full-suite" - - "firestore-only" - iterations: - description: "Test runs per arm (full-suite ~23s each, firestore-only ~8s each)" - required: false - default: "30" - node_versions: - description: "JSON array of Node majors to probe" - required: false - default: '["22", "24"]' - arms: - description: "JSON array of grpc-js arms: baseline, override, or both" - required: false - # Baseline alone by default since 2026-08-10: the override was measured and does - # not fix the flake (#776). Kept as an option rather than deleted. - default: '["baseline"]' - -# Least privilege. This workflow reads the repo and writes nothing back. -permissions: - contents: read - -jobs: - probe: - runs-on: ubuntu-latest - # Sized 2026-08-07 on the `Run tests` step of the real CI job (not the whole job, - # which is ~57s including install and setup). - # - # ⚠️ Do NOT size this on a single figure. Across recent CI runs that step lands - # anywhere from 20 to 31 seconds, and a timeout cares about the slow tail, not the - # median. At 31s the 200 cap wants ~207 minutes, which a flat-23s estimate (~153) - # would have put comfortably inside 180. And if #776's reported ~120s hang ever - # reproduces, those iterations cost far more than any of this. - # - # The default 30 per arm is ~28-38 minutes full-suite and is safe under any reading. - # This ceiling exists for the 200 cap, so it is set past the pessimistic figure - # rather than the average one. The `always()` summary step means a run that does hit - # the wall still reports the arms that finished. - timeout-minutes: 240 - strategy: - matrix: - node: ${{ fromJSON(inputs.node_versions) }} - fail-fast: false - name: Probe Node ${{ matrix.node }} - steps: - - name: Checkout - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - with: - persist-credentials: false - - - name: Setup node - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 - with: - node-version: ${{ matrix.node }} - check-latest: true - cache: 'npm' - - - name: Setup Java - uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0 - with: - distribution: 'temurin' - java-version: '21' - - - name: Firebase emulator cache - uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 - with: - path: ~/.cache/firebase/emulators - key: firebase_emulators - - - name: Install deps - run: npm ci - - # The full suite starts the functions emulator, which will not come up without - # these. `test.yaml` does the same thing before `npm run test`. Skipped in - # firestore-only mode, where no functions emulator is started. - - name: Install deps for functions - if: ${{ inputs.workload == 'full-suite' }} - run: npm install --no-audit --no-fund - working-directory: ./functions - - # Inputs and matrix values are passed through `env` rather than interpolated into - # the script body, so nothing from the dispatch form can be executed as shell. - # - # Counts are appended to `probe-counts.tsv` as each arm finishes, and the summary - # is rendered by a separate `always()` step. If the job hits its timeout partway - # through, whatever was measured before the wall still gets reported. - - name: Run the probe - env: - ITERATIONS: ${{ inputs.iterations }} - ARMS: ${{ inputs.arms }} - WORKLOAD: ${{ inputs.workload }} - NODE_MAJOR: ${{ matrix.node }} - run: | - # ⚠️ errexit is ON here even though nothing below turns it on: GitHub runs an - # undeclared `run:` step as `bash -e {0}`, and `set -uo pipefail` does not - # disable it. Every command that is ALLOWED to fail therefore has to say so. - # Getting this wrong means the step dies on the first failing iteration and - # the probe can only ever report a clean table, which is the one failure mode - # that makes the whole workflow useless. See the loop below. - set -uo pipefail - - # Guard against a non-numeric or absurd `iterations` before it reaches the loop. - case "$ITERATIONS" in - ''|*[!0-9]*) echo "iterations must be a positive integer, got '$ITERATIONS'"; exit 1 ;; - esac - if [ "$ITERATIONS" -lt 1 ] || [ "$ITERATIONS" -gt 200 ]; then - echo "iterations must be between 1 and 200, got '$ITERATIONS'" - exit 1 - fi - - # `full-suite` reproduces what `npm run test` does in CI: every emulator in - # firebase.json, every test file. `firestore-only` is the narrower original, - # kept because it isolates the Firestore client and is ~3x faster per run. - case "$WORKLOAD" in - full-suite) - EMULATOR_ARGS="" - VITEST_ARGS="" - ;; - firestore-only) - EMULATOR_ARGS="--only firestore" - VITEST_ARGS="firestore" - ;; - *) - echo "workload must be full-suite or firestore-only, got '$WORKLOAD'" - exit 1 - ;; - esac - echo "Workload: $WORKLOAD" - - # Validate the arm list rather than trusting the dispatch form, and normalize - # it to a space-separated list. Order is forced baseline-then-override because - # applying the override mutates node_modules for everything after it. - ARM_LIST="$(node -e ' - const arms = JSON.parse(process.env.ARMS); - if (!Array.isArray(arms) || arms.length === 0) throw new Error("arms must be a non-empty JSON array"); - const allowed = ["baseline", "override"]; - for (const a of arms) if (!allowed.includes(a)) throw new Error("unknown arm: " + a); - process.stdout.write(allowed.filter((a) => arms.includes(a)).join(" ")); - ')" || exit 1 - - mkdir -p probe-logs - : > probe-counts.tsv - : > probe-unmatched.txt - : > probe-iterations.tsv - - for arm in $ARM_LIST; do - echo "::group::Arm: $arm" - - # `npm pkg set` mangles keys containing a slash, so edit package.json directly. - # `npm install` (not `npm ci`) is required here because applying an override - # necessarily changes the lockfile. - if [ "$arm" = "override" ]; then - node -e ' - const fs = require("fs"); - const pkg = JSON.parse(fs.readFileSync("package.json", "utf8")); - pkg.overrides = { ...pkg.overrides, "@grpc/grpc-js": "^1.14.0" }; - fs.writeFileSync("package.json", JSON.stringify(pkg, null, 2) + "\n"); - ' - npm install --no-audit --no-fund - fi - - resolved="$(node -p "require('@grpc/grpc-js/package.json').version")" - echo "Arm $arm resolved @grpc/grpc-js: $resolved" - - pass=0 - flake=0 - infra=0 - hang=0 - grpc_err=0 - flake_with_grpc=0 - - for i in $(seq 1 "$ITERATIONS"); do - log="probe-logs/$arm-run-$i.log" - json="probe-logs/$arm-run-$i.json" - - # A fresh emulator start per iteration, matching how `npm test` runs in CI. - # Reusing one emulator across iterations would measure a different thing. - # Unquoted on purpose: both are either empty or a fixed literal set above, - # never user input. - # - # ⚠️ `set +e` is LOAD-BEARING, do not remove it. This command failing is the - # entire point of the probe, but the step runs under `bash -e`, so without - # this the first flake kills the step before `rc` is even read: no - # classification, no tally for the arm, and an empty or half-written - # probe-counts.tsv. It was removed once on the reasoning that the script - # never sets `-e` itself, which is true and irrelevant. - # ⚠️ The json reporter is what makes the timing usable; do not read durations - # out of the human log instead. The default reporter prints a per-test line - # only above its 300ms slow threshold, pass or fail, so the log drops auth - # entirely and censors the fast end of firestore and database. A comparison - # across outcomes needs every iteration, not the slow ones. - set +e - npx firebase emulators:exec $EMULATOR_ARGS --project=rxfire-525a3 \ - "npx vitest run $VITEST_ARGS --reporter=default --reporter=json --outputFile.json=$json" > "$log" 2>&1 - rc=$? - set -e - - # Health-check duration per emulator, in ms: firestore, auth, database. All - # three so a slow firestore round trip can be told apart from a slow runner. - # - # ⚠️ Scoped by FILE, not by test title. All three files open with a test named - # `double check - emulator is running`, so matching the title alone records - # whichever one vitest emitted first. - # - # `na` when the json is missing or unparseable, and for auth and database in - # firestore-only mode. Never read it as a fast run. - health_line="$(node -e ' - const fs = require("fs"); - const p = process.argv[1]; - const files = ["test/firestore.test.tsx", "test/auth.test.tsx", "test/database.test.tsx"]; - const out = files.map(() => "na"); - if (!fs.existsSync(p)) { process.stdout.write(out.join("\t")); process.exit(0); } - let report; - try { report = JSON.parse(fs.readFileSync(p, "utf8")); } - catch { process.stdout.write(out.join("\t")); process.exit(0); } - for (const file of report.testResults || []) { - const idx = files.findIndex((f) => String(file.name || "").includes(f)); - if (idx === -1) continue; - for (const a of file.assertionResults || []) { - if (a.title === "double check - emulator is running" && typeof a.duration === "number") { - out[idx] = String(Math.round(a.duration)); - break; - } - } - } - process.stdout.write(out.join("\t")); - ' "$json" 2>/dev/null || true)" - [ -n "$health_line" ] || health_line="$(printf 'na\tna\tna')" - IFS=$'\t' read -r health_ms health_auth health_db <<< "$health_line" - - saw_grpc=0 - if grep -q "RESOURCE_EXHAUSTED: Received message larger than max" "$log"; then - grpc_err=$((grpc_err + 1)) - saw_grpc=1 - fi - - # ⚠️ EVERY QUESTION BELOW IS SCOPED TO FIRESTORE'S OWN OUTPUT, and it has to - # be. In firestore-only mode anything in the log was necessarily about #776. - # In full-suite mode that is false: `expected 'loading' to deeply equal - # 'success'` is just what vitest prints when a data hook's status assertion - # fails, and it appears in 6 of the 9 test files, 40 times over. An unscoped - # search counts a slow storage upload or a functions failure as a #776 flake. - # - # In a vitest log the FAIL line names the file and the assertion or timeout - # sits on the NEXT line, verified against #781's real overnight failure, so - # -A1 is the correct window. Captured into a variable rather than piped into - # `grep -q`, because an early-exiting `grep -q` can SIGPIPE its producer and - # `pipefail` would turn that 141 into a silent "no match". - fs_fails="$(grep -A1 -E "FAIL.*test/firestore\.test\.tsx" "$log" || true)" - - if [ "$rc" -eq 0 ]; then - pass=$((pass + 1)) - outcome=pass - echo "run $i: PASS (firestore health check ${health_ms}ms)" - elif grep -qE "FAIL.*test/firestore\.test\.tsx.*double check - emulator is running" "$log"; then - # `test/{auth,firestore,database}.test.tsx` each open with an emulator - # health check. If FIRESTORE's fails, its emulator did not come up and no - # firestore result this iteration means anything, so the run is void. - # - # ⚠️ Scoped to firestore deliberately. An unscoped check let ANY emulator's - # health failure outrank a real firestore flake in the same run, filing it - # as infra and dropping it from the rate. A non-firestore health failure - # now falls through to the final `else`, where it is still excluded but is - # recorded in probe-unmatched.txt instead of being silently miscounted. - # - # This must precede the hang check either way: a health check fails BY - # timing out, so it would otherwise read as the #776 120s hang. - infra=$((infra + 1)) - outcome=infra - echo "run $i: INFRA FAILURE (rc=$rc), firestore emulator health check failed, excluded from the rate" - tail -20 "$log" - elif grep -q "expected 'loading' to deeply equal 'success'" <<< "$fs_fails"; then - # The #776 signature, in firestore's output specifically. - flake=$((flake + 1)) - # Whether the gRPC desync and the #776 assertion co-occur is the whole - # question, so count the overlap rather than two independent totals. - if [ "$saw_grpc" -eq 1 ]; then - flake_with_grpc=$((flake_with_grpc + 1)) - fi - outcome=flake - echo "run $i: FLAKE (rc=$rc, firestore health check ${health_ms}ms)" - elif grep -qE "Test timed out in [0-9]+ms|Hook timed out in [0-9]+ms" <<< "$fs_fails"; then - # #776 also reports a ~120s hang. A hang produces no assertion line, so - # without this bucket it would land in `infra` and vanish from the rate. - # Scoped like the flake check: a timeout in any other test file is not - # the #776 hang and must not be presented as one. - hang=$((hang + 1)) - outcome=hang - echo "run $i: HANG (rc=$rc, firestore health check ${health_ms}ms)" - else - # Everything else: emulator start failures, a non-firestore health check, - # a failure in another test file. Counted separately because folding them - # in previously inflated a local flake-rate estimate by ~50%. - infra=$((infra + 1)) - outcome=infra - echo "run $i: INFRA FAILURE (rc=$rc), excluded from the rate" - # Two ways to land here that must not be silent: vitest rewording the #776 - # assertion (which would turn every real flake into an infra failure), and - # a genuine failure in another test file. Record the first FAIL line from - # anywhere in the log, not just firestore's, so both are visible. - if line="$(grep -m1 -E "FAIL |AssertionError|expected .* to " "$log")"; then - printf '%s run %s: %s\n' "$arm" "$i" "$line" >> probe-unmatched.txt - fi - tail -20 "$log" - fi - - # One row per iteration, so the health-check duration can be compared across - # outcomes rather than only totalled. The per-arm counts below stay as they - # were; this is additive. - printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ - "$NODE_MAJOR" "$arm" "$i" "$outcome" \ - "$health_ms" "$health_auth" "$health_db" "$saw_grpc" \ - >> probe-iterations.tsv - done - - printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ - "$NODE_MAJOR" "$arm" "$resolved" \ - "$pass" "$flake" "$infra" "$hang" "$grpc_err" "$flake_with_grpc" \ - >> probe-counts.tsv - - echo "arm=$arm node=$NODE_MAJOR pass=$pass flake=$flake infra=$infra hang=$hang grpc_err=$grpc_err overlap=$flake_with_grpc" - echo "::endgroup::" - done - - # Rendered separately, and on `always()`, so a job killed by `timeout-minutes` - # still reports every arm that finished before the wall. - - name: Summarize - if: ${{ always() }} - env: - NODE_MAJOR: ${{ matrix.node }} - WORKLOAD: ${{ inputs.workload }} - run: | - set -uo pipefail - - if [ ! -s probe-counts.tsv ]; then - echo "No arm completed; nothing to summarize." >> "$GITHUB_STEP_SUMMARY" - exit 0 - fi - - while IFS=$'\t' read -r node arm resolved pass flake infra hang grpc_err overlap; do - counted=$((pass + flake)) - if [ "$counted" -gt 0 ]; then - # `< /dev/null` so the subshell cannot consume the loop's stdin. - rate="$(node -e "process.stdout.write(((${flake}/${counted})*100).toFixed(1))" < /dev/null)" - else - rate="n/a" - fi - - { - echo "### Node ${node} / ${arm} / ${WORKLOAD} (@grpc/grpc-js ${resolved})" - echo "" - echo "| Outcome | Count |" - echo "| --- | --- |" - echo "| Pass | ${pass} |" - echo "| Flake (#776 signature) | ${flake} |" - echo "| ...of which also showed RESOURCE_EXHAUSTED | ${overlap} |" - echo "| Hang (test timeout, no assertion) | ${hang} |" - echo "| Infra failure (excluded) | ${infra} |" - echo "| Runs showing RESOURCE_EXHAUSTED | ${grpc_err} |" - echo "" - echo "**Flake rate: ${rate}% of ${counted} counted runs.**" - echo "" - if [ "$hang" -gt 0 ] || [ "$infra" -gt 0 ]; then - echo "> ${hang} hang(s) and ${infra} infra failure(s) are excluded from the rate." - echo "" - fi - } >> "$GITHUB_STEP_SUMMARY" - done < probe-counts.tsv - - { - echo "---" - echo "" - if [ "$WORKLOAD" = "firestore-only" ]; then - echo "⚠️ **A clean table here is not a verdict on CI.** This ran one emulator and" - echo "one test file; \`npm run test\` in CI starts five emulators and runs the whole" - echo "suite with parallel workers. The 2026-08-07 run of this mode came back 120/120" - echo "clean with zero \`RESOURCE_EXHAUSTED\`, so this configuration is not known to" - echo "reproduce either the local or the CI failure. Prefer \`full-suite\`." - else - echo "This ran the same workload as CI: every emulator in \`firebase.json\` and the" - echo "whole test suite, one fresh emulator start per iteration. A failure in any" - echo "test file counts, but only the #776 assertion signature counts toward the" - echo "flake rate; anything else is reported separately and listed below." - fi - echo "" - } >> "$GITHUB_STEP_SUMMARY" - - # Firestore's health check is a bare `addDoc` round trip and every `waitFor` in - # test/firestore.test.tsx uses the 1000ms default, so a slow round trip would - # explain the failures. Compares distributions, not flake events: a run yields - # few flakes but records a duration every iteration. It describes; overlapping - # ranges are a real answer, not a failed one. - if [ -s probe-iterations.tsv ]; then - { - echo "### Firestore emulator health check, by outcome" - echo "" - node -e ' - const fs = require("fs"); - const rows = fs.readFileSync("probe-iterations.tsv", "utf8").trim().split("\n").filter(Boolean) - .map((l) => l.split("\t")) - .map(([node, arm, i, outcome, fsMs, authMs, dbMs, grpc]) => ({ arm, outcome, fsMs, authMs, dbMs })); - // Even n takes the mean of the two middle values. Taking the upper made - // the median and max cells print the same number at n = 2, and the flake - // row is where n is smallest. - const median = (a) => { - const m = a.length >> 1; - return a.length % 2 ? a[m] : Math.round((a[m - 1] + a[m]) / 2); - }; - // Each cell carries its own n: the row count includes iterations that - // recorded no duration, so a row of 5 can rest on 2 measurements. - const series = (rs, key) => { - const a = rs.map((r) => Number(r[key])).filter((n) => Number.isFinite(n)).sort((x, y) => x - y); - return a.length ? `${median(a)} / ${a[a.length - 1]} (n=${a.length})` : "-"; - }; - // One table per arm. Pooling them would put baseline and override into one - // distribution while the counts tables above stay per-arm. - const arms = [...new Set(rows.map((r) => r.arm))]; - for (const arm of arms) { - const armRows = rows.filter((r) => r.arm === arm); - if (arms.length > 1) { console.log(`Arm: ${arm}`); console.log(""); } - console.log("Median / max, in ms, with the count of iterations that recorded one."); - console.log(""); - console.log("| Outcome | runs | firestore | auth | database |"); - console.log("| --- | --- | --- | --- | --- |"); - for (const name of ["pass", "flake", "hang", "infra"]) { - const rs = armRows.filter((r) => r.outcome === name); - if (!rs.length) continue; - console.log(`| ${name} | ${rs.length} | ${series(rs, "fsMs")} | ${series(rs, "authMs")} | ${series(rs, "dbMs")} |`); - } - console.log(""); - const missing = armRows.filter((r) => !Number.isFinite(Number(r.fsMs))) - .reduce((m, r) => m.set(r.outcome, (m.get(r.outcome) || 0) + 1), new Map()); - if (missing.size) { - const parts = [...missing].map(([outcome, n]) => `${n} ${outcome}`).join(", "); - console.log(`> No firestore duration recorded for ${parts}, usually because the emulator never came up.`); - console.log(""); - } - } - console.log("> Read the columns against each other. Firestore slow while auth and database"); - console.log("> stay flat points at the Firestore client or its stream; all three rising"); - console.log("> together points at runner-wide contention instead, which is a different bug."); - ' || echo "(could not summarize durations)" - echo "" - } >> "$GITHUB_STEP_SUMMARY" - fi - - if [ -s probe-unmatched.txt ]; then - { - echo "### ⚠️ Unrecognized failures" - echo "" - echo "These runs failed with an assertion the classifier does not know, so they" - echo "were counted as infra. If vitest reworded the #776 message, the flake counts" - echo "above are wrong and the pattern needs updating." - echo "" - echo '```' - cat probe-unmatched.txt - echo '```' - echo "" - } >> "$GITHUB_STEP_SUMMARY" - fi - - # The probe reports; it does not fail. A red job here would mean the probe - # broke, not that the flake reproduced. - total_counted="$(awk -F'\t' '{ s += $4 + $5 } END { print s + 0 }' probe-counts.tsv)" - if [ "$total_counted" -eq 0 ]; then - echo "Every run failed for infrastructure reasons; the probe measured nothing." - exit 1 - fi - - - name: Upload probe logs - if: ${{ always() }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: probe-logs-node${{ matrix.node }} - path: | - probe-logs/ - probe-counts.tsv - probe-unmatched.txt - probe-iterations.tsv - retention-days: 7 From ac3ccf9b1c1e3a45577fa37aaaa2552085d61d9f Mon Sep 17 00:00:00 2001 From: Tyler Reitz Date: Thu, 20 Aug 2026 10:58:38 -0700 Subject: [PATCH 10/10] fix(ssr): add getServerSnapshot to useObservable's useSyncExternalStore (#779) useObservable called useSyncExternalStore with two arguments. React requires a third, getServerSnapshot, whenever the tree is server rendered or hydrated; without it React throws "Missing getServerSnapshot, which is required for server-rendered content" and the surrounding subtree silently falls back to client rendering. The server snapshot deliberately does not return observable.immutableStatus the way getSnapshot does. preloadedObservables is a globalThis cache keyed only by observableId, so on a server it is shared by every concurrent request; seeding the server snapshot from it would let one request render data another request fetched for the same path. Only config is read here, because it arrives from the caller on this render. Today that leak is unreachable because SSR throws first, so fixing the crash without this constraint would trade a crash for a cross-request data disclosure. That guarantee holds on React 18 and up. On 16 and 17 the shim's server path ignores the third argument and returns getSnapshot(), so the cached value still reaches the markup there, unchanged from before. Adds six tests under a "Server rendering" block, covering both renderToString and the streaming renderer, all mutation verified: - dropping the third argument fails all six with React's own error - returning observable.immutableStatus instead (the straightforward implementation) passes four and fails two: the cross-request leak test and the one preferring the caller's initialData over a cached value The server snapshot literal carries no type assertion, so tsc reports TS2741 if a required ObservableStatus field goes missing. Fixes #748. --- src/useObservable.ts | 29 ++++++++++- test/useObservable.test.tsx | 100 +++++++++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/useObservable.ts b/src/useObservable.ts index f66a5522..237f6e35 100644 --- a/src/useObservable.ts +++ b/src/useObservable.ts @@ -104,7 +104,34 @@ export function useObservable(observableId: string, source: Observa return observable.immutableStatus; }, [observable]); - const update = useSyncExternalStore(subscribe, getSnapshot); + // Reads only `config`, never `observable.immutableStatus`: `preloadedObservables` is a + // `globalThis` cache keyed only by `observableId`, so a server shares it across concurrent + // requests, and seeding from it would render one request's data into another's HTML. + // React 18 and up only: below that the shim's server path ignores this function and returns + // `getSnapshot()`, so the cached value still reaches the markup there. + // Held in a ref because React requires a stable value across renders. + const serverSnapshotRef = React.useRef | undefined>(undefined); + const getServerSnapshot = React.useCallback<() => ObservableStatus>(() => { + if (serverSnapshotRef.current === undefined) { + const initialDataValue = config?.initialData ?? config?.startWithValue; + + serverSnapshotRef.current = { + status: hasInitialData ? 'success' : 'loading', + hasEmitted: hasInitialData, + isComplete: false, + data: initialDataValue, + error: undefined, + firstValuePromise: observable.firstEmission + }; + } + + return serverSnapshotRef.current; + // Callers pass a fresh `config` literal each render, so the fields read above are kept + // out of the deps; the ref computes the value once per instance anyway. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [observable, hasInitialData]); + + const update = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); // Return a new object with initialData overlaid rather than mutating the shared // _immutableStatus reference, which is the same object across all components diff --git a/test/useObservable.test.tsx b/test/useObservable.test.tsx index f16d327a..ada53ab5 100644 --- a/test/useObservable.test.tsx +++ b/test/useObservable.test.tsx @@ -1,8 +1,10 @@ import '@testing-library/jest-dom/extend-expect'; import { act, cleanup, render, renderHook, waitFor } from '@testing-library/react'; import * as React from 'react'; +import { Writable } from 'node:stream'; +import { renderToString, renderToPipeableStream } from 'react-dom/server'; import { of, Subject, BehaviorSubject, throwError } from 'rxjs'; -import { useObservable } from '../src/index'; +import { useObservable, ReactFireOptions } from '../src/index'; describe('useObservable', () => { afterEach(cleanup); @@ -329,4 +331,100 @@ describe('useObservable', () => { expect(refreshedComp).toHaveTextContent('James'); }); }); + + describe('Server rendering', () => { + // Renders `status` and `data` so assertions read the snapshot React actually used. + const Probe = ({ observableId, observable$, config }: { observableId: string; observable$: Subject; config?: ReactFireOptions }) => { + const { status, data } = useObservable(observableId, observable$, { suspense: false, ...config }); + // One interpolated child: adjacent JSX text nodes render with `` between them. + return
{`${status}:${String(data)}`}
; + }; + + it('renders on the server instead of throwing', () => { + const observable$: Subject = new Subject(); + + // The #748 regression test: delete the third argument to useSyncExternalStore and + // this fails with "Missing getServerSnapshot". + expect(() => renderToString()).not.toThrow(); + }); + + // The App Router streams rather than calling renderToString, and streaming surfaces + // failures the synchronous renderer does not, so the fix is checked against both. + it('renders on the server under the streaming renderer', async () => { + const observable$: Subject = new Subject(); + let error: unknown; + + const html = await new Promise((resolve, reject) => { + const chunks: string[] = []; + const sink = new Writable({ + write(chunk, _encoding, callback) { + chunks.push(chunk.toString()); + callback(); + } + }); + sink.on('finish', () => resolve(chunks.join(''))); + sink.on('error', reject); + + const stream = renderToPipeableStream(, { + onError(caughtError) { + error = caughtError; + }, + onAllReady() { + stream.pipe(sink); + } + }); + }); + + expect(error).toBeUndefined(); + expect(html).toContain('loading:undefined'); + }); + + it('reports loading on the server when there is no initialData', () => { + const observable$: Subject = new Subject(); + + const html = renderToString(); + + expect(html).toContain('loading:undefined'); + }); + + it('reports initialData on the server when it is provided', () => { + const observable$: Subject = new Subject(); + + const html = renderToString(); + + expect(html).toContain('success:seeded'); + }); + + it('does not leak a cached value from another request into the server snapshot', async () => { + // `preloadedObservables` is on `globalThis`, keyed only by observableId, so concurrent + // server requests share it. The first render below stands in for an earlier request. + const observable$: Subject = new Subject(); + const observableId = 'ssr-no-cross-request-leak'; + + const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false })); + act(() => observable$.next('first-request-secret')); + await waitFor(() => expect(result.current.data).toEqual('first-request-secret')); + + const html = renderToString(); + + expect(html).not.toContain('first-request-secret'); + expect(html).toContain('loading:undefined'); + }); + + it('prefers the callers initialData over a value already in the shared cache', async () => { + // The `initialData` branch is only reachable when the cache already holds a value for + // this id; otherwise `useObservable`'s overlay decides and the snapshot never does. + const observable$: Subject = new Subject(); + const observableId = 'ssr-initial-data-beats-cache'; + + const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false })); + act(() => observable$.next('another-requests-value')); + await waitFor(() => expect(result.current.data).toEqual('another-requests-value')); + + const html = renderToString(); + + expect(html).toContain('success:my-own-data'); + expect(html).not.toContain('another-requests-value'); + }); + }); });