fix(errors): point error docs links at a page that exists - #3589
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR adds generated error-reference documentation and CI validation. It changes error URLs from individual paths to fragment anchors on the guide page, updates runtime references and tests, and validates registry-to-documentation coverage. ChangesError documentation and URL migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ErrorRegistry
participant Generator
participant ErrorGuide
participant CI
ErrorRegistry->>Generator: provide registered errors
Generator->>ErrorGuide: generate and format errors.md
CI->>Generator: run docs:errors:check
Generator-->>CI: report documentation status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
f606bbe to
8567b15
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
scripts/docs/generate-error-reference.ts (1)
16-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the internal import map for cross-module imports.
Replace the relative
src/errorsimports with#veryfront/errors/*imports.
scripts/docs/generate-error-reference.ts#L16-L18: import the registry, diagnostic policy, and types through#veryfront/errors/*.tests/docs/error-docs-links.test.ts#L3-L7: import the registry and diagnostic policy through#veryfront/errors/*.Based on learnings: “Do not add relative internal imports outside the
cli/directory.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/docs/generate-error-reference.ts` around lines 16 - 18, Replace the relative imports in scripts/docs/generate-error-reference.ts lines 16-18 with the corresponding `#veryfront/errors/`* imports for the registry, diagnostic policy, and ErrorCategory type. Also update tests/docs/error-docs-links.test.ts lines 3-7 to import the registry and diagnostic policy through `#veryfront/errors/`*; both sites require direct changes.Source: Learnings
src/errors/diagnostic-policy.ts (1)
15-24: 🗄️ Data Integrity & Integration | 🔵 TrivialConfirm documentation publication before release.
ERROR_DOCS_BASE_URLnow points to the generated guide. Synchronize and deploy that guide toveryfront-docsbefore this runtime change reaches production. Otherwise, emittedDocs:links remain dead.Add this synchronization to the release or deployment gate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/errors/diagnostic-policy.ts` around lines 15 - 24, Update the release or deployment gate that publishes documentation to synchronize and deploy the generated errors guide to veryfront-docs before the runtime change is released. Ensure the generated guide is available at ERROR_DOCS_BASE_URL so emitted Docs links resolve to its anchors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/docs/generate-error-reference.ts`:
- Around line 160-167: Update formatMarkdown to capture the result of
command.output(), check its success status, and throw when deno fmt exits
unsuccessfully before the caller reports completion. Preserve the existing
formatting command and arguments.
In `@src/errors/catalog/factory.test.ts`:
- Around line 438-443: Update the hydration URL fixture and matching assertion
in the relevant error catalog test to use the single valid anchor from
docs/guides/errors.md, removing the second fragment delimiter while keeping both
values identical.
In `@src/errors/README.md`:
- Line 73: Update the slug-encoding paragraph in the errors README to describe
<slug> as a URL fragment rather than a documentation URL path segment, while
preserving the existing encoding and redaction guarantees.
In `@src/errors/user-friendly/error-catalog.ts`:
- Line 2: Update the cross-module import in error-catalog.ts to use the
configured `#veryfront/`* internal alias for diagnostic-policy.ts instead of a
relative path, while preserving the existing buildErrorDocsUrl import.
---
Nitpick comments:
In `@scripts/docs/generate-error-reference.ts`:
- Around line 16-18: Replace the relative imports in
scripts/docs/generate-error-reference.ts lines 16-18 with the corresponding
`#veryfront/errors/`* imports for the registry, diagnostic policy, and
ErrorCategory type. Also update tests/docs/error-docs-links.test.ts lines 3-7 to
import the registry and diagnostic policy through `#veryfront/errors/`*; both
sites require direct changes.
In `@src/errors/diagnostic-policy.ts`:
- Around line 15-24: Update the release or deployment gate that publishes
documentation to synchronize and deploy the generated errors guide to
veryfront-docs before the runtime change is released. Ensure the generated guide
is available at ERROR_DOCS_BASE_URL so emitted Docs links resolve to its
anchors.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 635534b4-6dc1-49aa-be88-4b4bfc76f5e0
⛔ Files ignored due to path filters (1)
src/server/services/rsc/endpoints/rsc-bundles.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (28)
deno.jsondocs/guides/errors.mddocs/guides/index.mdscripts/docs/generate-error-reference.tssrc/errors/README.mdsrc/errors/catalog/config-errors.test.tssrc/errors/catalog/factory.test.tssrc/errors/diagnostic-policy.tssrc/errors/error-registry.test.tssrc/errors/http-error.test.tssrc/errors/http-error.tssrc/errors/logging.test.tssrc/errors/middleware/cli-error-boundary.test.tssrc/errors/middleware/cli-error-boundary.tssrc/errors/middleware/http-error-boundary.test.tssrc/errors/safe-diagnostics.test.tssrc/errors/types.test.tssrc/errors/user-friendly/error-catalog.test.tssrc/errors/user-friendly/error-catalog.tssrc/server/handlers/request/agent-stream.handler.test.tssrc/server/handlers/request/api/api-handler-wrapper.test.tssrc/server/handlers/request/module/module.handler.test.tssrc/server/handlers/request/ssr/ssr.handler.test.tssrc/server/runtime-handler/timeout-manager.test.tssrc/server/services/rsc/endpoints/endpoint-router.test.tstests/docs/error-docs-links.test.tstests/docs/guide-contracts.test.tstests/e2e/regressions/rsc-proxy-hydration.test.ts
6531afe to
0308a5a
Compare
Every boundary that reports a VeryfrontError prints a documentation URL built from the error's slug: the CLI formatter, the RFC 9457 `type` field, and the log line all call `buildErrorDocsUrl`. That built `https://veryfront.com/docs/errors/<slug>`, and no `/docs/errors/**` section has ever existed - not on the site, not in veryfront-docs. All 107 registered slugs 404, so every error a user has ever seen ends with a dead link. Registry entries do not carry enough for a page each. `ErrorDefinition` (src/errors/types.ts) is slug, category, status, title, suggestion, exitCode - there is no description field. A generated page per slug would be a title line and a one-line suggestion, 107 times over. So the slugs become anchors on one generated reference page instead: docs/guides/errors.md, grouped by category, published at veryfront.com/docs/code/guides/errors via the existing sync-docs path. ERROR_DOCS_BASE_URL becomes that page plus `#`, and every caller follows without further change. The page is generated from the registry by scripts/docs/generate-error-reference.ts, following the generate-api-reference.ts precedent, with a `--check` staleness mode wired into lint:ci, verify, and verify:quick. Generation fails loudly on an error category that has no page section rather than dropping its errors silently. Regression test in tests/docs/error-docs-links.test.ts iterates the registry rather than sampling slugs: every slug's built URL must resolve to the published page's path and to an anchor that exists on it, the `unknown-error` fallback for unusable slugs must resolve too, the page must carry no anchors for slugs the registry no longer emits, and no shipped source may hardcode an error docs link instead of calling buildErrorDocsUrl. Verified by adding an unpublished slug to the registry: the test fails with `probe-dead-link: no "#probe-dead-link" anchor in docs/guides/errors.md`. src/errors/user-friendly/error-catalog.ts had hardcoded one of these URLs rather than building it; it now calls buildErrorDocsUrl, which the new hardcoded-link check keeps that way. The safe-diagnostics and types tests that pinned a hostile slug to a path segment now pin it to the fragment, asserting the same property in its new shape: a hostile slug cannot change the page the link points at, open a query, or start a second fragment.
The published pages render as MDX, and `mintlify broken-links` rejected the
generated page twice:
Unable to parse code/guides/errors.md - 14:2: Unexpected character `!`
Unable to parse code/guides/errors.md - 355:3: Expected a closing tag for
`<number>` before the end of `paragraph`
The provenance HTML comment is not valid MDX, so it becomes a visible sentence
in the intro instead. And two registry suggestions carry CLI placeholders
(`veryfront dev --port <number>`, `veryfront push --branch <name>`) that MDX
reads as unclosed JSX tags, so all registry-derived text is now escaped for the
characters MDX treats as markup.
Escaping is applied generally rather than to those two strings: any future
error message containing a placeholder would otherwise break the docs build.
A new case in tests/docs/error-docs-links.test.ts asserts the page carries no
unescaped MDX markup characters at all, which is the check that would have
caught this without a Mintlify run.
Generation now runs only under `import.meta.main` so the test can import
escapeMdxText without rewriting the committed page.
Verified with `npx mintlify@latest broken-links` on node 22: no broken links.
CI's `ci (lint)` failed on `docs/api-reference is stale: outdated: veryfront/errors.md`. The new import in src/errors/user-friendly/error-catalog.ts shifted two source line pins by one, and those pins are baked into the generated reference. Regenerated with `deno task docs` under the Deno version CI pins (2.7.7, per .github/actions/setup-deno/action.yml) rather than the local default, which reports every page as outdated. The diff is two line pins and nothing else.
Three findings from the automated review, all valid: - The blanket URL rewrite turned a fixture's `.../docs/errors/hydration-mismatch#hydration` into `.../errors#hydration-mismatch#hydration`. A second `#` is not a second anchor -- the browser reads the whole tail as one fragment. The fixture now uses the single anchor that exists on the page. - generate-error-reference.ts ignored a failing `deno fmt`, so a formatter failure would write an unformatted page and, in --check mode, compare against an unformatted expectation -- reporting "current" for a page that fails `deno fmt --check`. It now throws with the formatter's stderr. Verified by stubbing `deno fmt` to exit 1: the generator fails instead of reporting success. - src/errors/README.md still described the slug as a URL path segment. It is a fragment now, and the guarantee it documents is correspondingly stronger: a slug can never change the page the link points at. Not applied: the suggestion to import diagnostic-policy.ts via `#veryfront/errors/diagnostic-policy.ts`. No such subpath exists in the import map (only `#veryfront/errors` -> index.ts), so that specifier would not resolve, and every other file in src/errors/user-friendly/ already imports its siblings relatively (../catalog/types.ts, ../safe-diagnostics.ts, ../types.ts).
0308a5a to
cdb3ceb
Compare
The defect
Every boundary that reports a
VeryfrontErrorprints a documentation URL built from the error's slug — the CLI formatter, the RFC 9457typefield, and the log line all go throughbuildErrorDocsUrl. It builthttps://veryfront.com/docs/errors/<slug>, and no/docs/errors/**section has ever existed, on the site or inveryfront-docs.Reproduced before changing anything:
All 107 registered slugs are affected, so every error a user has ever seen ends with a dead link.
Why anchors on one page, not a page per slug
The brief suggested registry entries might carry
title/description/suggestion/category— enough for a page each. They don't.ErrorDefinition(src/errors/types.ts) is:There is no
descriptionfield. Reading actual entries,titleis a short noun phrase andsuggestionis one imperative line:107 pages of two short lines each is precisely the "wall of near-empty stubs" the brief warned is worse than one good page. So slugs become anchors on one generated reference page:
docs/guides/errors.md, grouped by category, published atveryfront.com/docs/code/guides/errorsthrough the existing sync-docs path.ERROR_DOCS_BASE_URLbecomes that page plus#; every caller follows with no further change.This keeps option (a)'s real benefit — the destination is generated from the registry, so it cannot drift — without the stub problem. If entries ever grow real prose, splitting into pages is a generator change, not a re-plumbing of every boundary.
Changes
scripts/docs/generate-error-reference.tsgenerates the page from the registry, following thegenerate-api-reference.tsprecedent, with a--checkstaleness mode wired intolint:ci,verify, andverify:quick. It fails loudly on an error category with no page section rather than silently dropping its errors.ERROR_DOCS_BASE_URLnow points at the real page.src/errors/user-friendly/error-catalog.tshad hardcoded one of these URLs instead of building it; it now callsbuildErrorDocsUrl.The regression test
tests/docs/error-docs-links.test.tsiterates the registry rather than sampling slugs. It asserts:unknown-errorfallback used for unusable slugs resolves too;buildErrorDocsUrl.It derives the local file from
ERROR_DOCS_BASE_URL, so moving the page without moving the constant fails.Confirmed failing before the fix for the right reason, and confirmed to catch a new dead link by adding an unpublished slug to the registry:
The
--checkgate catches the same mistake asdocs/guides/errors.md is stale.Security-test shape change
safe-diagnostics.test.tsandtypes.test.tspinned a hostile slug to a path segment. The slug is now a fragment, so those tests assert the same property in its new shape: a hostile slug cannot change the page the link points at, open a query, or start a second fragment.encodeURIComponentstill applies, and confinement to a fragment is if anything stronger than confinement to a path segment.Verification
deno task lint,deno task typecheck,deno task docs:errors:check,scripts/lint/check-doc-links.ts(1230 links OK), and the full pre-push gate all pass.One pre-existing, unrelated
docs:validatefailure is present onorigin/mainand is not touched here:docs/guides/deploying.md:63uses an em dash, from #3566.docs:validateis not part of the CI lint gate.Follow-up
The generated page also needs to reach
veryfront-docsfor the links to resolve in production. Thesync-docsworkflow already coversdocs/guides/**, so this should propagate on merge tomain— worth confirming the dispatch lands before announcing the links as live.Summary by CodeRabbit
New Features
Bug Fixes
Documentation