Skip to content

feat: add 404 page and serve metrics on separate port#35

Open
rsbh wants to merge 9 commits intomainfrom
fix_404_otel_server
Open

feat: add 404 page and serve metrics on separate port#35
rsbh wants to merge 9 commits intomainfrom
fix_404_otel_server

Conversation

@rsbh
Copy link
Copy Markdown
Member

@rsbh rsbh commented Apr 14, 2026

Summary

  • Handle /api/page errors in page context with errorStatus state instead of silently catching
  • Show custom 404 page using Apsara EmptyState component when page not found (SSR + client-side)
  • Move Prometheus metrics from /api/metrics route to separate port (default 9090, configurable via telemetry.port)

Test plan

  • Navigate to non-existent page → 404 page renders centered
  • Valid pages still render correctly
  • SSR returns HTTP 404 status for missing pages
  • Client-side navigation to missing page shows 404
  • Metrics available on port 9090 (curl localhost:9090/metrics)
  • Old /api/metrics route returns 404

🤖 Generated with Claude Code

rsbh and others added 6 commits April 14, 2026 15:31
Handle non-ok responses from /api/page instead of silently catching.
Track errorStatus as HTTP status code (number | null) to distinguish
error states from loading state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire existing NotFound component into DocsPage using errorStatus
from page context.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add getInitialErrorStatus() to compute error state from initialPage
and pathname, so SSR also shows NotFound component on missing pages.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace manual Flex/Headline/Text layout with Apsara EmptyState
component for consistent empty state styling.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PrometheusExporter now starts its own HTTP server on configurable
port (default 9090). Removes /api/metrics route and mock request
handler. Adds telemetry.port to config schema.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Warning

Rate limit exceeded

@rsbh has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 33 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 33 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: acfa3f64-6455-4c08-83a3-fd55e1783b52

📥 Commits

Reviewing files that changed from the base of the PR and between ef09ec4 and 004da0f.

📒 Files selected for processing (5)
  • docs/configuration.mdx
  • packages/chronicle/src/lib/page-context.tsx
  • packages/chronicle/src/pages/DocsPage.tsx
  • packages/chronicle/src/server/telemetry.ts
  • packages/chronicle/src/types/config.ts
📝 Walkthrough

Walkthrough

This PR introduces error status tracking to the page context for displaying 404 and 500 error pages, refactors the NotFound component to use an EmptyState layout, removes the metrics API endpoint, refactors telemetry initialization to use PrometheusExporter with a configurable port, and consolidates request tracking by removing the /api/metrics path exclusion from telemetry monitoring.

Changes

Cohort / File(s) Summary
Error Status & Page Context
packages/chronicle/src/lib/page-context.tsx, packages/chronicle/src/pages/DocsPage.tsx
Added errorStatus field to PageContextValue interface with initialization logic for 404 errors on non-existent pages; modified DocsPage to render NotFound component when errorStatus === 404.
NotFound Page UI
packages/chronicle/src/pages/NotFound.tsx, packages/chronicle/src/pages/NotFound.module.css
Refactored NotFound component to use EmptyState with heading "404" and subheading "Page not found"; added .emptyState CSS class for center alignment.
Telemetry Refactoring
packages/chronicle/src/server/telemetry.ts, packages/chronicle/src/server/plugins/telemetry.ts, packages/chronicle/src/server/api/metrics.ts
Removed /api/metrics endpoint; removed getExporter() function; updated telemetry initialization to use PrometheusExporter with configurable port; removed request-hook condition that skipped /api/metrics path from telemetry tracking.
Configuration
packages/chronicle/src/types/config.ts
Added optional port field (default: 9090) to telemetry configuration schema.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • rohilsurana
  • rohanchkrabrty
🚥 Pre-merge checks | ✅ 2 | ❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main features added: a 404 page and moving metrics to a separate port.
Description check ✅ Passed The description clearly relates to the changeset, covering error handling, 404 page rendering, and metrics port migration with a test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix_404_otel_server

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/chronicle/src/lib/page-context.tsx`:
- Line 57: The conditional using pathname.startsWith('/apis') is too broad and
matches routes like '/apis-foo'; update the check that currently reads something
like if (pathname === '/' || pathname.startsWith('/apis')) to only treat the
top-level apis route by changing it to check for pathname === '/apis' or
pathname.startsWith('/apis/') (i.e., if (pathname === '/' || pathname ===
'/apis' || pathname.startsWith('/apis/')) ), and make the same change for the
other occurrence that uses pathname.startsWith('/apis').

In `@packages/chronicle/src/pages/DocsPage.tsx`:
- Around line 13-14: The current DocsPage returns null for any errorStatus that
isn't 404, causing a blank screen on server errors; update the DocsPage
component's error handling (the block checking errorStatus and page) so that
when errorStatus is set and not 404 it renders a proper error/failure UI instead
of null—e.g., render an Error or ServerError component (or a generic error
message) for non-404 statuses, keeping the existing NotFound render for 404 and
preserving the existing page render when page is present.

In `@packages/chronicle/src/types/config.ts`:
- Line 73: The telemetry.port schema currently uses
z.number().optional().default(9090) and allows invalid TCP port values
(negatives, >65535, non-integers); update the schema for telemetry.port to
enforce an integer in the valid TCP range by adding integer, min and max
constraints (e.g., use
z.number().int().min(1).max(65535).optional().default(9090)) so only valid ports
are accepted while preserving the default.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4be53968-c75d-415d-9f2e-4a2bab8b2088

📥 Commits

Reviewing files that changed from the base of the PR and between 752b303 and ef09ec4.

📒 Files selected for processing (8)
  • packages/chronicle/src/lib/page-context.tsx
  • packages/chronicle/src/pages/DocsPage.tsx
  • packages/chronicle/src/pages/NotFound.module.css
  • packages/chronicle/src/pages/NotFound.tsx
  • packages/chronicle/src/server/api/metrics.ts
  • packages/chronicle/src/server/plugins/telemetry.ts
  • packages/chronicle/src/server/telemetry.ts
  • packages/chronicle/src/types/config.ts
💤 Files with no reviewable changes (2)
  • packages/chronicle/src/server/plugins/telemetry.ts
  • packages/chronicle/src/server/api/metrics.ts

rsbh and others added 3 commits April 14, 2026 16:06
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Default imports fail in ESM — these packages only export named.
Fixes build failure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Tighten /apis route matching to avoid false positives on /apis-foo
- Handle non-404 errors in DocsPage to avoid blank screen
- Validate telemetry.port as int in 1-65535 range

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant