Skip to content

Commit 5681326

Browse files
feat: add Sentry error capture helper, conventions, and regression test (#80) (#84)
- Add captureSupabaseError() in src/lib/sentry.ts for structured Sentry reporting of Supabase errors (PostgrestError and generic Error) - Add Error Handling section to .agents/conventions.md documenting rules for mutations, catch blocks, and client-side error handling - Add static analysis test scanning for bare catch {} and console.error without Sentry capture, with allowlist for known exceptions - Delete dead sentry.client.config.ts (Turbopack never loads it; instrumentation-client.ts is the correct client init) - Update architecture.md component map Co-authored-by: Ona <no-reply@ona.com>
1 parent 3ada95c commit 5681326

5 files changed

Lines changed: 217 additions & 9 deletions

File tree

.agents/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ src/
220220
│ ├── table.tsx
221221
│ └── tooltip.tsx
222222
├── lib/
223+
│ ├── sentry.ts # captureSupabaseError helper (structured Sentry reporting)
223224
│ ├── utils.ts # cn() utility (clsx + tailwind-merge)
224225
│ ├── types.ts # Database entity types
225226
│ ├── workspace.ts # Workspace utilities: slug generation, validation, limits
@@ -234,7 +235,6 @@ Root config files:
234235
├── instrumentation-client.ts # Sentry client init (replay, route transitions)
235236
├── sentry.server.config.ts # Sentry server SDK config
236237
├── sentry.edge.config.ts # Sentry edge SDK config
237-
├── sentry.client.config.ts # Sentry client SDK config
238238
└── components.json # shadcn/ui config (base-nova style, Tailwind v4)
239239
```
240240

.agents/conventions.md

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,71 @@ The `updateSession` function in `src/lib/supabase/proxy.ts` creates a server cli
117117
that reads cookies from the request and writes refreshed cookies to the response.
118118
It calls `supabase.auth.getUser()` to trigger the refresh.
119119

120+
## Error Handling
121+
122+
All errors must reach Sentry. `console.error` alone is never sufficient — always
123+
pair with a Sentry capture call. Bare `catch {}` is banned except for documented
124+
exceptions (see allowlist below).
125+
126+
### Supabase mutations
127+
128+
Every Supabase mutation must check the error return and call `captureSupabaseError`:
129+
130+
```typescript
131+
import { captureSupabaseError } from "@/lib/sentry";
132+
133+
const { error } = await supabase.from("pages").update({ title }).eq("id", pageId);
134+
if (error) {
135+
captureSupabaseError(error, "pages.update");
136+
// handle the error (toast, return error response, etc.)
137+
}
138+
```
139+
140+
The helper accepts `PostgrestError` (from query results) and generic `Error` (from
141+
catch blocks). It tags the Sentry event with the operation name, error code, and
142+
message so errors are filterable in the Sentry dashboard.
143+
144+
### API route catch blocks
145+
146+
All catch blocks in API routes must call `Sentry.captureException`:
147+
148+
```typescript
149+
import * as Sentry from "@sentry/nextjs";
150+
151+
try {
152+
// ... route logic
153+
} catch (error) {
154+
Sentry.captureException(error);
155+
return NextResponse.json({ error: "Internal server error" }, { status: 500 });
156+
}
157+
```
158+
159+
### Client-side mutations
160+
161+
Client-side mutations must show `toast.error()` on failure in addition to Sentry
162+
capture:
163+
164+
```typescript
165+
import { captureSupabaseError } from "@/lib/sentry";
166+
import { toast } from "sonner";
167+
168+
const { error } = await supabase.from("pages").insert({ ... });
169+
if (error) {
170+
captureSupabaseError(error, "pages.insert");
171+
toast.error("Failed to create page");
172+
}
173+
```
174+
175+
### Bare catch allowlist
176+
177+
These files have intentional bare `catch {}` blocks with documented reasons:
178+
179+
- `src/lib/supabase/server.ts` — cookie `setAll` in Server Components (can't set cookies, safe to ignore)
180+
- `src/components/editor/editor.tsx` — URL validation (`new URL()` throws on invalid input)
181+
- `src/app/api/health/route.ts` — intentionally silent, monitored by Performance Monitor
182+
183+
All other catch blocks must capture the error variable and report to Sentry.
184+
120185
## Environment Variable Guards
121186

122187
Route handlers and server utilities that use Supabase must guard against missing

sentry.client.config.ts

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/lib/sentry.test.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { describe, it, expect } from "vitest";
2+
import { readdirSync, readFileSync, statSync } from "fs";
3+
import { join, relative } from "path";
4+
5+
/**
6+
* Static analysis tests for error handling conventions (Issue #80).
7+
*
8+
* Scans source files for bare `catch {}` blocks and `console.error` calls
9+
* without accompanying Sentry capture. Files that will be fixed in subsequent
10+
* PRs are allowlisted — those PRs must remove their file from the allowlist.
11+
*/
12+
13+
const SRC_DIR = join(__dirname, "..");
14+
15+
/** Files with intentional bare `catch {}` that are permanently allowed. */
16+
const BARE_CATCH_PERMANENT_ALLOWLIST = new Set([
17+
"src/lib/supabase/server.ts", // cookie setAll in Server Components
18+
"src/components/editor/editor.tsx", // URL validation (new URL() throws)
19+
"src/app/api/health/route.ts", // intentionally silent, monitored externally
20+
]);
21+
22+
/**
23+
* Files with bare `catch {}` that will be fixed in subsequent PRs.
24+
* Each entry should reference the issue that will fix it.
25+
*/
26+
const BARE_CATCH_PENDING_ALLOWLIST = new Set([
27+
"src/components/sidebar/page-search.tsx", // #81 or follow-up
28+
"src/app/api/search/route.ts", // #81 or follow-up
29+
]);
30+
31+
const BARE_CATCH_ALLOWLIST = new Set([
32+
...BARE_CATCH_PERMANENT_ALLOWLIST,
33+
...BARE_CATCH_PENDING_ALLOWLIST,
34+
]);
35+
36+
/**
37+
* Files with `console.error` without Sentry capture that will be fixed
38+
* in subsequent PRs.
39+
*/
40+
const CONSOLE_ERROR_PENDING_ALLOWLIST = new Set([
41+
"src/components/editor/image-plugin.tsx", // #81 or follow-up
42+
"src/components/page-menu.tsx", // #81 or follow-up
43+
]);
44+
45+
function collectTsFiles(dir: string): string[] {
46+
const files: string[] = [];
47+
48+
function walk(d: string) {
49+
for (const entry of readdirSync(d)) {
50+
const full = join(d, entry);
51+
const stat = statSync(full);
52+
if (stat.isDirectory()) {
53+
walk(full);
54+
} else if (/\.(ts|tsx)$/.test(entry) && !entry.endsWith(".test.ts") && !entry.endsWith(".test.tsx")) {
55+
files.push(full);
56+
}
57+
}
58+
}
59+
60+
walk(dir);
61+
return files;
62+
}
63+
64+
function toRelative(absPath: string): string {
65+
return relative(join(SRC_DIR, ".."), absPath);
66+
}
67+
68+
describe("error handling conventions", () => {
69+
const files = collectTsFiles(SRC_DIR);
70+
71+
it("no bare catch {} outside the allowlist", () => {
72+
const violations: string[] = [];
73+
74+
for (const file of files) {
75+
const rel = toRelative(file);
76+
if (BARE_CATCH_ALLOWLIST.has(rel)) continue;
77+
78+
const content = readFileSync(file, "utf-8");
79+
// Match `catch {` or `catch {` (bare catch without error variable)
80+
// but not `catch (error)` or `catch (e)`
81+
const bareCatchPattern = /\bcatch\s*\{/g;
82+
let match;
83+
while ((match = bareCatchPattern.exec(content)) !== null) {
84+
const line = content.slice(0, match.index).split("\n").length;
85+
violations.push(`${rel}:${line}`);
86+
}
87+
}
88+
89+
expect(
90+
violations,
91+
`Bare catch {} found outside allowlist. Either capture the error and report to Sentry, or add to the allowlist with a comment explaining why:\n${violations.join("\n")}`,
92+
).toEqual([]);
93+
});
94+
95+
it("no console.error without Sentry capture outside the allowlist", () => {
96+
const violations: string[] = [];
97+
98+
for (const file of files) {
99+
const rel = toRelative(file);
100+
if (CONSOLE_ERROR_PENDING_ALLOWLIST.has(rel)) continue;
101+
102+
const content = readFileSync(file, "utf-8");
103+
const lines = content.split("\n");
104+
105+
for (let i = 0; i < lines.length; i++) {
106+
if (lines[i].includes("console.error")) {
107+
// Check if Sentry capture exists anywhere in the same file
108+
const hasSentryCapture =
109+
content.includes("Sentry.captureException") ||
110+
content.includes("captureSupabaseError");
111+
112+
if (!hasSentryCapture) {
113+
violations.push(`${rel}:${i + 1}`);
114+
}
115+
}
116+
}
117+
}
118+
119+
expect(
120+
violations,
121+
`console.error without Sentry capture found outside allowlist. Add Sentry.captureException or captureSupabaseError:\n${violations.join("\n")}`,
122+
).toEqual([]);
123+
});
124+
});

src/lib/sentry.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as Sentry from "@sentry/nextjs";
2+
import { PostgrestError } from "@supabase/supabase-js";
3+
4+
/**
5+
* Report a Supabase error to Sentry with structured context.
6+
*
7+
* Accepts both PostgrestError (from query/mutation results) and generic Error
8+
* (from catch blocks). The `operation` tag makes it easy to filter in Sentry
9+
* by the database operation that failed.
10+
*/
11+
export function captureSupabaseError(
12+
error: PostgrestError | Error,
13+
operation: string,
14+
): void {
15+
const extra: Record<string, string> = {
16+
operation,
17+
message: error.message,
18+
};
19+
20+
if (error instanceof PostgrestError) {
21+
extra.code = error.code;
22+
extra.details = error.details;
23+
extra.hint = error.hint;
24+
}
25+
26+
Sentry.captureException(error, { extra });
27+
}

0 commit comments

Comments
 (0)