[Pro] Backport RSC retries and license configuration to 17.0.0 - #4590
Closed
justin808 wants to merge 2 commits into
Closed
[Pro] Backport RSC retries and license configuration to 17.0.0#4590justin808 wants to merge 2 commits into
justin808 wants to merge 2 commits into
Conversation
Alternative implementation to #4562 for the client-side request amplifier reported in [shakacode/react_on_rails_rsc#187](shakacode/react_on_rails_rsc#187). This PR and #4562 solve the same client-side problem and are mutually exclusive. This implementation is intended to replace #4562’s render-driven retry architecture, not build on top of it. relying on React’s retry render to create attempt two: ```text attempt one rejects → delete its Promise → React rerenders → create a second Promise → separate per-key state reconnects both attempts ``` This PR keeps one Promise for the complete logical load: ```text one cached Promise → attempt one → one internal retry when appropriate → resolve or retain the final rejection ``` The correctness distinction is ownership: the retry budget belongs to the logical payload load, not to React renders. With one Promise, React scheduling cannot change the number of requests, and stale attempts, cache eviction, and explicit refetch do not need to synchronize with a separate attempt record. This also changes the behavior of the retry itself. #4562’s retry re-enters the ordinary `getComponent` path, so malformed embedded Flight data may be read a second time without making an HTTP request. This implementation retries with `enforceRefetch: true`, bypassing the failed embedded value and requesting a fresh payload. Controlled browser comparison: - #4562 with malformed embedded payload: zero HTTP requests; the same embedded failure is surfaced. - This implementation: one HTTP fallback; the healthy payload renders successfully. `RSCProvider` now caches one Promise for the complete logical browser load: 1. Perform the initial payload request. 2. Retry once internally when the failure may be transient. 3. Resolve if either attempt succeeds. 4. Otherwise retain the final rejected Promise briefly so React can surface it through the Error Boundary. 5. Allow a later lookup to start a fresh bounded load after the retention window. The automatic retry uses `enforceRefetch: true`, ensuring a failed embedded or prefetched payload is bypassed in favor of a fresh HTTP request. Retry policy: - Retry once: network failures, malformed payloads, HTTP 408/429, and 5xx. - Do not automatically retry: ordinary 4xx responses and `AbortError`. - Explicit refetch replaces a retained failure immediately. - Official server rendering explicitly disables the browser retry, including when server dependencies shim `window`. Final failures remain pinned during the five-second retention window so ordinary LRU pressure cannot erase the request limit. This means a high-cardinality outage may temporarily exceed the normal 50-entry cache limit; those entries are removed and unpinned when retention ends. The retry budget belongs to the logical payload load, not to React renders. React always receives the same cached Promise regardless of how often it rerenders. React scheduling therefore cannot create additional attempts, and explicit refetches or stale operations cannot corrupt the current load’s ownership. This differs from a render-driven retry, where attempt one is deleted and a later React render creates attempt two, requiring separate state to reconnect both Promises. | Scenario | Result | | --- | --- | | Vulnerable RC with persistent malformed payload | 3,424 requests in 6 seconds | | Fixed package with persistent malformed payload | Exactly 2 requests through 12 seconds; final error surfaced | | First payload fails, retry succeeds | Exactly 2 requests; Blue Page rendered | | Malformed embedded payload, healthy endpoint | Exactly 1 HTTP fallback; Blue Page rendered | | SSR with server-side `window` shim | Exactly 1 producer call | - Full React on Rails Pro package suite: **600 tests passed** - TypeScript type-check - ESLint - Prettier - Pro license-header validation - Bundle-size limits - Production-package browser reproduction - Stale retry/refetch and last-good restoration regression coverage Labels: `ready-for-hosted-ci`, `benchmark` — this changes Pro RSC cache and retry behavior under concurrency and cache pressure, so it needs optimized hosted confirmation plus the Pro benchmark route. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Bug Fixes** * Prevented repeated/unbounded browser fetches when server component payloads fail. * Added bounded automatic retry for transient failures, while preserving terminal failures and non-retryable cancellations. * Improved reuse during failure retention windows so repeated lookups don’t constantly restart requests. * Enhanced HTTP error reporting for non-OK responses and improved recovery from malformed or invalid preloaded/deferred payloads. * **New Features** * Introduced a configurable option to enable/disable rejected-payload retries (browser defaults on; server rendering defaults off). <!-- end of auto-generated comment: release notes by coderabbit.ai --> - Release-mode gate: `development` from release tracker #3823; target `main` is beta phase. The standard beta gate is satisfied. - Current head: `5d895d25388a5cf55ee903a65c2138c991142750`. - CI: `pr-ci-readiness` is `READY` with the required gate in use; optimized hosted CI, Pro package/integration jobs, bundle-size checks, CodeQL, and all four benchmark suites completed successfully for this head. Selector skips are explained by `script/ci-changes-detector origin/main`. - Review threads: 8 total, 0 unresolved. The strict merge ledger reports `complete_allowed: true` with `changelog_present` and no `UNKNOWN` fields. - Review coverage: Claude review and CodeRabbit produced current-head artifacts, satisfying the two-system coverage floor. Greptile only produced evidence for stale head `b2eb62ee4967de17ad56f7dfe65eff569b7732e5`; it is degraded for the final head and is not cited as a merge gate. - Independent QA: 137 focused affected tests and 453 broader Pro tests passed, plus type-check, lint, Prettier, diff checks, and all 847 Pro license headers. - Proof replay: the identical permanent-failure/same-key probe failed on merge base `f45df4d6c082caf0e2e064256bbcd63af623e88d` with 3 producer calls instead of 2, then passed on this head with exactly 2 calls, `enforceRefetch: true` on attempt two, and later lookups reusing the original rejected Promise. Confidence note: - Validated: full Pro package suite (602 tests); independent focused/broader QA (590 tests); type-check; repository lint; Prettier; Pro license headers; `git diff --check`; deterministic base-red/head-green replay; hosted CI and benchmarks. - Evidence: current-head GitHub checks and reviews on this PR; strict `script/pr-merge-ledger 4564 --changelog-classification changelog_present --strict`; QA replay at the merge-base and current head above. - UNKNOWN: no additional real-browser wall-clock replay was run during coordinator closeout; the deterministic cache/timer replay directly proves the amplification mechanism and bounded fix, and the PR records prior production-browser verification. - Residual risk: terminal failures may temporarily exceed the soft 50-entry cache cap during the bounded five-second retention window; this is intentional, documented, and covered by cache-pressure tests. - **Non-blocking:** Retain non-retryable terminal failures, including `AbortError`, for the bounded retention window. - **Decision:** Keep the retained rejection so render-driven lookups cannot reopen the request budget; explicit refetch still replaces it immediately. - **Why:** Focused tests and the red/green replay prove the hard two-attempt ceiling while preserving explicit recovery. - **Review later:** Optional production-wrapper SSR integration coverage may be added separately; no current defect was demonstrated. (cherry picked from commit 9d470c4)
## Why React on Rails Pro license validation only consumed `REACT_ON_RAILS_PRO_LICENSE`, even though Rails applications commonly keep secrets in encrypted credentials and the renderer password already supports application configuration. That forced Rails deployments to expose the license through the process environment and left the separate Node renderer's configuration boundary unclear. ## What changed - add `ReactOnRailsPro.configuration.license_token`, with explicit nonblank Rails configuration taking precedence over `REACT_ON_RAILS_PRO_LICENSE` - add the Node renderer's `licenseToken` option with the same precedence and blank-value fallback - validate the configured token during both clustered-master and single-process renderer startup - mask license values in every sanitized Node renderer configuration view - preserve `REACT_ON_RAILS_PRO_LICENSE` as the backward-compatible default and generator path - document Rails credentials, standalone Node secret configuration, process separation, CI behavior, security guidance, troubleshooting, and migration from the removed key file across the canonical and package docs Rails configuration intentionally does not propagate to a separately running Node service. Each process must receive the same token through its own configuration or environment. ## Validation - `pnpm run lint` - `pnpm start format.listDifferent` - `pnpm run type-check` - `pnpm run build` - `(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion)` - `(cd react_on_rails && bundle exec rake rbs:validate)` - `node script/generate-llms-full.mjs --check` - `script/check-docs-sidebar` - `script/check-pro-license-headers` - `git diff --check` - pre-commit hooks, including offline Markdown links - pre-push hooks, including online Markdown links Local test suites were intentionally left to CI under the repository's React on Rails PR policy. An automated local review pass was stopped when it attempted to start those prohibited suites; no local test result is claimed. ## Review churn - Pre-push review gate: manual end-to-end flow and secret-redaction review; automated Codex review stopped for attempting prohibited local tests - Post-push review churn: one planned changelog-only commit completed after obtaining the PR number - Remaining gaps: optimized hosted CI has been requested for the Ruby and Node test suites ## Demo coordination No flagship demo change is needed: the existing environment-variable setup remains supported and generated defaults are unchanged. This adds optional secret-source configuration without changing the recommended generated deployment path. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `config.license_token` support for Rails apps (including credentials/config) with precedence over `REACT_ON_RAILS_PRO_LICENSE`. * Added standalone Node renderer `licenseToken` option with env fallback and masked diagnostics. * **Bug Fixes** * Updated license validation and startup logging to respect precedence/blank values and avoid token disclosure in sanitized output. * Improved license-status caching to be scoped per effective token. * **Documentation** * Refreshed Pro license setup, installation, upgrading, troubleshooting, and CI/license-task guidance, including review-app token exposure security. * **Tests** * Expanded test coverage for token resolution, masking, and startup validation flows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Codex Decision Log - **Non-blocking:** Should this PR target `release/17.0.0` rather than `main`? - **Decision:** Keep the `main` target. - **Why:** The 17.0.0 tracker is in development mode, the RC branch is for stabilizing fixes only, and this is an optional configuration capability with no demonstrated RC regression or release hard-gate need. - **Review later:** Backport only if a maintainer identifies a concrete 17.0.0 stabilizing need. ## Review Coverage Note - Current head SHA: `1325f12e7d921699f09b8c52270b4a964f814273`. - Current-head review systems: `claude-review` and CodeRabbit completed successfully. - Degraded advisory coverage: visible Greptile and GitHub Codex review artifacts predate the final code head, so they are treated as stale advisory history rather than merge evidence. - Local fallback review attempts: `codex review` could not start because the installed CLI is too old for its configured model; the Claude CLI report-only fallback is not authenticated. Independent implementation review and required runtime QA were completed instead. (cherry picked from commit 91aeba1)
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
size-limit report 📦
|
Contributor
Review: [Pro] Backport RSC retries and license configuration to 17.0.0Overview: this backports #4564 (bounded browser retry for rejected RSC payload promises) and #4552 (configurable 🔴 CHANGELOG.md: backport appears to delete already-released entries and the
|
Member
Author
|
Closing in favor of the serialized release-backport flow: one merged source PR per release-targeted backport PR. #4564 is being backported independently from the latest release/17.0.0 tip; #4552 must be evaluated and, if it qualifies for the release train, backported in its own separately reviewed and reversible PR. The branch is intentionally retained for evidence. |
This was referenced Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backports #4564 and #4552 to the 17.0 release line. The companion RSC cache-integrity backport is already merged as #4589.
Validation
git diff --check.