Skip to content

feat(web): add error and not-found UI; align CONTRIBUTING with develop #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mooncitydev wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(web): add error and not-found UI; align CONTRIBUTING with develop #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Project maintainers are responsible for clarifying and enforcing our standards o
```
3. **Add upstream remote**:
```bash
git remote add upstream https://github.com/ORIGINAL_OWNER/pacto-p2p.git
git remote add upstream https://github.com/PACTO-LAT/pacto-p2p.git
```
4. **Install dependencies**:
```bash
Expand Down Expand Up @@ -93,7 +93,7 @@ Examples:

### Making Changes

1. **Create a new branch** from `main`:
1. **Create a new branch** from `develop` (the default integration branch):
```bash
git checkout -b feature/your-feature-name
```
Comment on lines +96 to 99
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make branch creation explicitly start from develop.

Line 96 says “from develop”, but the command in Lines 97–99 creates a branch from the current HEAD. Add an explicit checkout/sync step for develop to avoid accidental wrong-base branches.

Suggested doc fix 
-1. **Create a new branch** from `develop` (the default integration branch):
+1. **Create a new branch** from `develop` (the default integration branch):
    ```bash
+   git checkout develop
+   git pull upstream develop
    git checkout -b feature/your-feature-name
    ```
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CONTRIBUTING.md` around lines 96 - 99, Update the branch-creation
instructions so the new feature branch is explicitly based on develop: before
the existing git checkout -b feature/your-feature-name step, add an explicit git
checkout develop and a git pull upstream develop (or equivalent sync) to ensure
your local develop is up-to-date; then run git checkout -b
feature/your-feature-name as shown.

Expand All @@ -116,7 +116,7 @@ Examples:
5. **Keep your branch updated**:
```bash
git fetch upstream
git rebase upstream/main
git rebase upstream/develop
```

6. **Push to your fork**:
Expand Down Expand Up @@ -221,7 +221,7 @@ export function EscrowCard(props: any) {
1. **Automated checks**: All CI checks must pass
2. **Code review**: At least one maintainer must approve
3. **Address feedback**: Make requested changes and push updates
4. **Keep PR updated**: Rebase on `main` if conflicts arise
4. **Keep PR updated**: Rebase on `develop` if conflicts arise

### After Approval

Expand Down Expand Up @@ -304,25 +304,7 @@ Fixes #456

## 🧪 Testing

### Running Tests

```bash
# Run all tests
npm test

# Run tests in watch mode
npm test -- --watch

# Run tests for specific package
npm test --workspace=@pacto-p2p/shared
```

### Writing Tests

- Write tests for new features
- Ensure existing tests pass
- Aim for good test coverage
- Test edge cases and error scenarios
Automated test scripts are not wired up in the root `package.json` yet. Before opening a PR, run the quality checks listed in [Making Changes](#making-changes) (`type-check`, `biome:check`, `build`). When you add automated tests in a package, document the exact command in your PR description so reviewers can run them.

## 📖 Documentation

Expand Down
43 changes: 43 additions & 0 deletions apps/web/app/error.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'use client';

import { AlertTriangle } from 'lucide-react';
import Link from 'next/link';
import { useEffect } from 'react';
import { Button } from '@/components/ui/button';

export default function ErrorPage({
error,
reset,
}: {
error: Error & { digest?: string };
reset: () => void;
}) {
useEffect(() => {
console.error(error);
}, [error]);

return (
<div className="flex min-h-screen flex-col items-center justify-center px-4">
<div className="mx-auto max-w-md text-center">
<AlertTriangle
className="mx-auto mb-4 h-12 w-12 text-destructive"
aria-hidden
/>
<h1 className="mb-2 text-xl font-semibold text-foreground">
Something went wrong
</h1>
<p className="mb-6 text-sm text-muted-foreground">
{error.message || 'An unexpected error occurred. Please try again.'}
</p>
Comment on lines +29 to +31
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid rendering raw error.message directly in production UI.

Displaying the raw error text can leak internal implementation details. Prefer a generic user message and keep detailed error info in logs/monitoring.

🔧 Suggested hardening 
+  const userMessage = 'An unexpected error occurred. Please try again.';
+
   return (
@@
         <p className="mb-6 text-sm text-muted-foreground">
-          {error.message || 'An unexpected error occurred. Please try again.'}
+          {userMessage}
         </p>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/error.tsx` around lines 29 - 31, Replace direct rendering of
error.message in the error UI with a generic user-facing string and move the
detailed error text to logs/monitoring: in apps/web/app/error.tsx stop rendering
{error.message || ...} and instead render a fixed friendly message like
"Something went wrong. Please try again."; concurrently call your logging
function (e.g., console.error(error) or a central logger/monitoring function) to
capture error.message and stack for debugging, and if you want to display
details only in development wrap the original message render behind a NODE_ENV
=== 'development' check (use the error variable and the JSX paragraph that
currently renders error.message).

<div className="flex flex-col gap-3 sm:flex-row sm:justify-center">
<Button type="button" onClick={() => reset()}>
Try again
</Button>
<Button variant="outline" asChild>
<Link href="/">Back to home</Link>
</Button>
</div>
</div>
</div>
);
}
25 changes: 25 additions & 0 deletions apps/web/app/not-found.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { FileQuestion } from 'lucide-react';
import Link from 'next/link';
import { Button } from '@/components/ui/button';

export default function NotFoundPage() {
return (
<div className="flex min-h-screen flex-col items-center justify-center px-4">
<div className="mx-auto max-w-md text-center">
<FileQuestion
className="mx-auto mb-4 h-12 w-12 text-muted-foreground"
aria-hidden
/>
<h1 className="mb-2 text-xl font-semibold text-foreground">
Page not found
</h1>
<p className="mb-6 text-sm text-muted-foreground">
The page you are looking for does not exist or may have been moved.
</p>
<Button asChild>
<Link href="/">Back to home</Link>
</Button>
</div>
</div>
);
}