Improve reverse proxy forbidden page to match connection error UI#6488
Improve reverse proxy forbidden page to match connection error UI#6488chiqors wants to merge 4 commits into
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a dedicated "forbidden" error page variant. A new ChangesForbidden Page Variant
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
proxy/web/src/ErrorPage.tsx (1)
124-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd fallback for missing
requestIdto match ForbiddenPage behavior.The middleware may provide an empty
requestIdwhenCapturedDatais nil. WhenrequestIdis undefined or empty, rendering{requestId}displays nothing, leaving "REQUEST-ID: " with a blank value. ForbiddenPage uses a fallback to "Unavailable" for better UX; the standard error page should do the same for consistency.🔧 Suggested fix
- <span className="text-nb-gray-400">REQUEST-ID:</span> {requestId} + <span className="text-nb-gray-400">REQUEST-ID:</span> {requestId || "Unavailable"}🤖 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 `@proxy/web/src/ErrorPage.tsx` at line 124, The RequestId display in ErrorPage does not have a fallback value when requestId is undefined or empty, resulting in blank text after "REQUEST-ID:". Add a fallback to display "Unavailable" when requestId is empty or undefined, similar to how ForbiddenPage handles missing requestId values. Update the JSX expression for requestId to use the logical OR operator or conditional logic to provide the fallback value for consistency across error pages.
🤖 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.
Outside diff comments:
In `@proxy/web/src/ErrorPage.tsx`:
- Line 124: The RequestId display in ErrorPage does not have a fallback value
when requestId is undefined or empty, resulting in blank text after
"REQUEST-ID:". Add a fallback to display "Unavailable" when requestId is empty
or undefined, similar to how ForbiddenPage handles missing requestId values.
Update the JSX expression for requestId to use the logical OR operator or
conditional logic to provide the fallback value for consistency across error
pages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a2004bc-6d2f-4c4f-8f86-86f51efde455
⛔ Files ignored due to path filters (2)
proxy/web/dist/assets/index.jsis excluded by!**/dist/**proxy/web/dist/assets/style.cssis excluded by!**/dist/**
📒 Files selected for processing (1)
proxy/web/src/ErrorPage.tsx
|
2 participants
Describe your changes
This updates the reverse proxy forbidden page to better match the existing NetBird connection error page visual language.
The forbidden state now uses the same overall structure and styling cues as the connection error page, while keeping messaging specific to private-access / policy-blocked requests.
Main updates:
Screenshots
Before
After
Issue ticket number and link
N/A
Stack
Checklist
Documentation
Select exactly one:
Why: this is a UI-only change for the reverse proxy forbidden page and does not change configuration, API behavior, or deployment flow.
Docs PR URL (required if "docs added" is checked)
N/A
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes