fix(ssr): recover custom error pages after transient failures

[kojiwakayama]

Description

Keep custom error pages available after temporary filesystem or dependency failures.

Previously, a failed discovery, source read, or module import could cache an existing error page as absent. Later requests continued to use the generic fallback even after the underlying failure recovered.

This fix caches only confirmed absence. It preserves the existing generic fallback during an outage and retries loading on the next request. It adds no retry loop, dependency, or public API change.

Related issues

Discovered during verification following #4423. Broader renderer integration remains tracked separately in veryfront/veryfront-issue-inbox#1035.

Type of change

• Bug fix (non-breaking)
• Test update

Verification

• Ten colocated recovery cases fail against the original implementation and pass with the fix. Coverage includes cold and warm caches, both resolution paths, and missing dependencies.
• deno task test:file src/server/handlers/request/ssr: 163 steps pass.
• deno task test:file src/platform/compat/fs.test.ts: 58 steps pass.
• Focused Node recovery and prepared-fallback tests: 12 tests pass.
• Focused Bun recovery and prepared-fallback tests: 2 files pass.
• Local HTTP smoke checks: 7 pass. Chromium hydration and interactive state updates pass.
• Formatting, lint, source/test typechecks, test-layout, and diff checks pass.
• codex review --uncommitted and codex review --base origin/main: no actionable findings for head 8af1c6389b43cc1d790316d834115bc5be68d58a.

Local verification does not replace staging rollout verification. This PR does not change resource sizing or deploy to production.

Checklist

• Documentation impact checked: no public API or configuration changes.
• Added tests that reproduce the failure and verify recovery.

Summary by CodeRabbit

• Bug Fixes
  • Error-page fallback recovery now retries after transient discovery, file-reading, module-loading, or dependency failures instead of caching a missing page.
  • Genuine missing error pages continue to use the generic fallback appropriately.
  • Improved handling of missing-file errors during error-page resolution.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[github-actions]

📦 Client bundle boundary

Entrypoint	Modules	Source size	Server leaks
src/index.client.ts	289	2303 KiB	✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: b6d712a6-144f-4661-98d5-ec84b49addb5

📥 Commits

Reviewing files that changed from the base of the PR and between 6be0e33 and 8af1c63.

📒 Files selected for processing (3)

• src/server/handlers/request/ssr/error-page-fallback-recovery.test.ts
• src/server/handlers/request/ssr/error-page-fallback.test.ts
• src/server/handlers/request/ssr/error-page-fallback.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The fallback now distinguishes confirmed missing pages from transient failures. New tests verify that discovery and loading outages do not prevent a later successful 500 error-page response.

Changes

Error-page fallback recovery

Layer / File(s)	Summary
Preserve retryability for transient failures src/server/handlers/request/ssr/error-page-fallback.ts	tryLoadErrorPage uses isNotFoundError and caches a missing page only after confirming that the page is absent.
Validate recovery across failure stages src/server/handlers/request/ssr/error-page-fallback-recovery.test.ts, src/server/handlers/request/ssr/error-page-fallback.test.ts	Tests cover transient failures during discovery, source reading, module loading, dependency lookup, and filesystem-style missing-file detection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 8af1c

Custom SSR error pages now recover after temporary failures instead of remaining stuck on the generic fallback. The supplied validation reports broad passing coverage, with no actionable merge blocker.

Sequence Diagram(s)

sequenceDiagram
  participant tryErrorPageFallback
  participant tryLoadErrorPage
  participant ErrorPageLoader
  tryErrorPageFallback->>tryLoadErrorPage: discover and load error page
  tryLoadErrorPage->>ErrorPageLoader: resolve or probe page
  ErrorPageLoader-->>tryLoadErrorPage: transient failure
  tryLoadErrorPage-->>tryErrorPageFallback: return null without caching absence
  tryErrorPageFallback->>tryLoadErrorPage: retry after recovery
  tryLoadErrorPage-->>tryErrorPageFallback: return rendered 500 response

Loading

Suggested reviewers: mattboon

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: enabling custom SSR error pages to recover after transient failures.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/custom-500-staging

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kwakayama]

Code Review — Score: 88/100 (Good)

Solid, narrowly-scoped bug fix: it stops caching a "no custom error page" miss when the underlying failure is transient (a stat/read/import hiccup) rather than a confirmed absence (ENOENT/ENOTDIR across every extension). The fix is easy to reason about, adds no new dependency or public API surface, and comes with strong regression coverage.

Strengths

• Correct core fix: confirmedMissing is only left true when every extension probe genuinely resolves "not found" (via the existing isNotFoundError classifier); any other error (permission, transient FS failure, module-load/dependency failure) now correctly skips setCachedMiss, matching the stated bug.
• Warm-cache path is handled too — a cached path that fails to (re)load is deleted via deleteCachedPath and falls through to re-probe, rather than being pinned to a stale miss.
• Good test design: error-page-fallback-recovery.test.ts parametrizes both resolution strategies (resolveFile vs. extension probing) × 5 failure stages × warm/cold cache, and asserts the exact failure-mode behavior (outage → generic fallback allowed; recovery → cached custom page served again). The existing test at error-page-fallback.test.ts:548 was correctly updated to tag its mock rejection with code: "ENOENT", since the new logic now depends on error classification rather than treating every catch as an implicit miss.
• Change is minimal and localized to tryLoadErrorPage; no behavior change to the successful-load or render paths.

Concerns (non-blocking, worth a look)

• In the resolveFile branch, when resolveFile finds a path but loadErrorComponent resolves to a non-function/falsy component (no exception thrown), the code now falls through to return null without calling setCachedMiss — this is a behavior change from before (previously it always cached the miss at the end of the function). It's arguably the safer default post-fix, but it's an untested edge case (a page file exists but has no valid default export) and worth either an explicit test or a comment noting it's intentional, since it means such a page will be re-probed on every error request instead of once.
• One pre-existing sibling test (describe("cache behavior with injected repo")'s neighbor around line ~490, the 403 fallback probe test) still rejects with a plain Error("not found") rather than an ENOENT-tagged one. It doesn't currently assert on caching so it still passes, but for consistency with the rest of the suite it'd be cleaner to tag it too, or add a short comment on why it's exempt.
• The PR description leans heavily on local codex review and local test output as verification; given this is fixing a staging-observed bug, it'd strengthen the record to link the original staging incident/observation directly (the linked issue is for "broader renderer integration," not this specific regression).

Nothing here blocks merge — the fix is correct, well-tested, and appropriately scoped. The two edge-case notes above are suggestions for a fast follow-up or a comment, not requirements.

---

Generated by Claude Code

[gitar-bot]

Note

Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime.
Learn more

Code Review ✅ Approved

Fixes custom error pages being unavailable after transient failures by caching only confirmed absence instead of failed discovery attempts. The fix preserves the generic fallback during outages and retries loading on the next request, with no new retry loop, dependency, or public API change. Comprehensive test coverage includes recovery cases across cold and warm caches, both resolution paths, and missing dependencies. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

---

Important

Your trial ends in 1 day — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}

[kojiwakayama]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 8af1c6389b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kojiwakayama]

Reviewed the non-blocking feedback against the queued head:

• An existing file with an invalid export is intentionally not cached as absent. Only a confirmed discovery miss qualifies for negative caching. The generic fallback remains available, and a later request can try loading again.
• The existing 403 test checks which fallback names are probed, not negative-cache behavior. Its generic rejection does not weaken the missing-file cache assertion, which now uses ENOENT. The suggested fixture cleanup and invalid-export coverage are optional follow-ups, not reasons to change the reviewed head while queued.
• The original synthetic staging observation was a failed dependency fetch while CDN egress was unavailable, followed by successful normal rendering after egress recovered but continued generic output for the custom 500 fallback. The isolated reproduction confirmed the cache defect: a temporary loader failure left a cached absence until that entry was cleared. With this fix, the same reproduction recovers on the next request without clearing the cache. Local HTTP and browser verification pass; post-merge staging verification remains separate.

The PR head remains 8af1c6389b43cc1d790316d834115bc5be68d58a. All review threads were checked with pagination; none are unresolved. No branch or queue changes are needed for these notes.

[kojiwakayama]

Staging verification results

Verified framework 0.1.1258-rc.18812, containing this fix, after the successful staging deployment.

The disposable fixture used the deployed image, read-only application sources, no application credentials, and no mounted service-account token.

• With CDN access unavailable, the failing route returned the generic HTTP 500 fallback.
• After enabling fixture-only CDN access, the next request rendered the custom 500 page in the same pod, with zero restarts and no cache clearing.
• All seven HTTP checks passed: health, SSR, MDX, custom 404, custom 500, RSC, and HEAD semantics.
• Chromium verified server-rendered content, hydration, and an interactive state update, with no browser errors.
• Replacing only the disposable test pod produced a healthy new pod with a fresh cache. All seven HTTP checks passed again.
• The shared renderer, proxy, and operator remained healthy at 2/2 replicas at the observed checkpoints. Staging API and site probes returned HTTP 200.

Cleanup is complete: the temporary Deployment, Service, ConfigMap, NetworkPolicy, pods, and local port-forward are gone. Manifests and results are retained locally for reproduction.

Remaining rollout gate

Two-replica failover was not tested. A fresh quota check still shows 76 GiB of 80 GiB allocated to memory limits, leaving only 4 GiB. Two temporary 4 GiB replicas need 8 GiB, plus whatever rollout headroom the capacity owner requires. Resource sizing is being handled separately; no quotas or existing application workloads were modified for this verification.

The single-pod replacement check is not a substitute for multi-replica availability verification. These results do not approve production rollout, and this verification made no production changes.

[kwakayama]

Capacity mission checkpoint — 2026-09-07

PR head 8af1c6389b43cc1d790316d834115bc5be68d58a is merged as f837e5e039ffa2736af75e40e8806a850d530739 (10:44 UTC). The staging deployment run 34115612773 completed successfully. There is no remaining merge blocker on this PR.

The previously documented two-replica staging failover verification remains open. A fresh read shows staging at 80/80 GiB memory limits, 41.4/42 CPU limits, 33.25/40 GiB memory requests, and 44/55 pods, with the API rolling and still 4/4 available. Two additional 4 GiB verification replicas do not fit the current memory-limit quota.

The active capacity lane in veryfront/veryfront-issue-inbox#1041 now explicitly owns the fixture budget and source-of-truth quota/rollout proposal needed to unblock this verification. Renderer memory/root-cause work proceeds separately in veryfront/veryfront-issue-inbox#1040 and #1035.

No quota, workload, HPA or replica changes were made for this checkpoint. Existing passing single-pod checks are preserved; multi-replica availability and production rollout are not claimed verified.

[kwakayama]

Staging failover unblock prepared

The capacity change is now in veryfront-infrastructure#336, head 4c7a6bc401b8df5b28a777ba1d311c5428211581.

It proposes staging admission limits of 96 GiB memory / 48 CPU cores, preserving memory/CPU requests, pod quota and availability/isolation settings. With the recorded 76 GiB / 39.9 CPU steady allocation, the two 4 GiB / 1-CPU verification replicas plus one API surge require 86 GiB / 42.4 CPU. The fixture's retained resource metrics show 2 GiB / 250m requests per replica.

The recovered 12-worker cluster has sufficient aggregate reservation headroom for this bounded scenario including loss of one large worker, but aggregate arithmetic does not prove node-local placement or peak safety. Run the fixture on separate eligible workers and without unrelated rollout overlap; recheck live quota and usage immediately before execution.

Independent review and normal release/apply gates are pending for the capacity PR. The multi-replica failover check has not yet run; no quota was changed. Track execution and results in veryfront/veryfront-issue-inbox#1041. PR #4436 itself remains merged with its previously recorded staging checks preserved.

[kwakayama]

Capacity dependency reviewed

veryfront-infrastructure#336 is now independently reviewed at head 4c7a6bc401b8df5b28a777ba1d311c5428211581: Codex 98/100, APPROVE, zero actionable findings. Its six applicable CI checks pass; three apply jobs are intentionally skipped on the PR. The review gate reports complete, stable evidence and no unresolved threads.

The source proposal is ready for the normal merge/apply gate. Live staging is still 80 GiB / 42 CPU; the proposed 96 GiB / 48 CPU has not been applied. The remaining two-replica staging failover test is therefore still pending approved activation, a fresh quota/usage check, distinct-worker placement and a release window without unrelated rollouts. Existing single-pod/browser evidence remains valid for its recorded scope.

Execution and final verification are tracked in veryfront/veryfront-issue-inbox#1041. This update does not claim the multi-replica test or production rollout is complete.

[kwakayama]

Staging gate unblocked and verified

Applied the merged veryfront-infrastructure#336 quota to only the staging ResourceQuota, using source from merge 8c2b535821f7d22d0ccffdebc24f8627639f26e6 after server-side dry-run:

• Memory limits: 80 → 96 GiB.
• CPU limits: 42 → 48 cores.
• Request quotas and pod quota remain 40 GiB / 22 cores / 55 pods.

The remaining two-replica pod-failover gate is now verified using the exact deployed staging renderer image (20260907111441-492b6645c20e, containing this PR). Each temporary replica requested 2 GiB / 250m and had 4 GiB / 1 CPU limits, with required distinct-worker placement, read-only fixture sources, no application credentials and no service-account token.

Results:

• Both replicas Ready, zero restarts.
• Health, instance API, SSR, custom 404 and custom 500 passed on both replicas: 10/10 checks before replacement, 10/10 afterward.
• 20/20 Service requests before replacement reached both replicas.
• 150/150 Service requests succeeded while one fixture pod was replaced, including 98 responses from the survivor and 52 from the replacement. The replacement became Ready.
• Chromium verified SSR, hydration and an interactive counter update before and after replacement; corrected fixture checks had no console errors.
• Shared staging renderer/proxy/API remained 2/2, 2/2, 4/4. Production health checks remained 10/10, 2/2, 4/4.
• All temporary fixture resources and the local port-forward were removed; cleanup verified.

Fixture setup corrections are recorded: projected ConfigMap symlinks were replaced with regular files copied by an init container, preserving the renderer's file-safety policy; the init request was raised to the namespace's existing 64 MiB minimum. During replacement, scheduling briefly waited for old-pod reservations/required affinity to clear, then recovered. These were resolved setup/transition events, not an unresolved quota blocker.

The team can continue #4436 staging work. This validates bounded two-replica pod replacement at the sampled request cadence, not arbitrary peak load, a real node outage, production rollout, or activation of the separate memory-recycle PR #4438. Continue avoiding overlapping release waves and retain the existing availability/isolation controls.

Tracking: veryfront/veryfront-issue-inbox#1041. Reproduction evidence is retained locally in local-reports/4436-failover/ with raw operational artifacts kept private.

[kojiwakayama]

Two-replica staging verification completed

The capacity blocker from the earlier verification is cleared. Retested the currently deployed framework 0.1.1258-rc.18812 using two disposable renderer replicas on separate nodes.

Results:

• Both replicas became ready on distinct nodes, with two ready Service endpoints.
• The seven HTTP checks passed on each original replica and each replacement: 28 checks total. These cover health, SSR, MDX, custom 404/500, RSC, and HEAD semantics.
• Chromium hydration and interactive state updates passed on all four replica instances, with no browser errors.
• An independent in-cluster client sent 180 requests through the Kubernetes Service over 120 seconds while the two original pods were gracefully deleted one at a time. Each replacement became ready before the next deletion.
• All 180 requests passed with one attempt per request and no client retries. Response markers confirmed traffic reached both original replicas and both replacements. Observed p95 request duration was 199 ms, with a maximum of 1,254 ms.
• The test ended with two ready replicas on distinct nodes and zero container restarts.
• Shared staging renderer, proxy, and operator remained 2/2 ready at the observed checkpoints. Final API and site health checks returned HTTP 200.

All temporary resources have been removed: Deployment, Service, ConfigMap, NetworkPolicy, probe pod, renderer pods, and local port-forwards. The manifests and probe logs are retained locally for reproduction. No quota or existing application workload changes were made by this test.

This completes the remaining controlled two-replica recovery check for this fix. It is a short verification of graceful pod replacement, not an abrupt node-loss, network-partition, sustained-load, or deferred isolated-executor activation test. Production promotion remains a separate approval and release action; this test made no production changes.

[kojiwakayama]

Strict staging release gate passed

Remote E2E Health run 34141011437 completed successfully for staging only:

• Preflight passed.
• Platform-UI: 43 tests passed, including the previously failing fresh-preview hydration check.
• AI-live: 39 tests passed.
• Both lanes ran with Playwright retries set to zero and reported successful cleanup.
• The final requested-lane rollup passed. This does not establish production health.

No source changes, disabled pinning, increased retries, or weakened test expectations were used to obtain this result. The earlier transitive dependency-snapshot 409 is recorded in issue #1042; its cause remains unconfirmed and must not be described as fixed by this rerun.

Draft promotion PRs are prepared for the exact staging-verified artifacts: Operator #240 and Server #340. Their CI and artifact-availability checks pass. Both remain drafts with auto-merge disabled, pending capacity-owner sign-off, release-owner risk review, and second-person Code Owner approval. No production deployment was triggered.