Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions .agents/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,19 @@ proper `Error` before sending to Sentry so they get proper stack traces and
grouping. The `isPostgrestError` duck-type check in `src/lib/sentry.ts` also
handles both shapes.

**Null-safety for PostgREST fields:** PostgREST error responses may return
`details: null` and `hint: null` (e.g. PGRST500 server errors). The `in` operator
in `isPostgrestError` returns `true` when the key exists regardless of value type.
Always null-coalesce when reading these fields:

```typescript
// ✅ Correct — handles null from PostgREST 500 responses
const details = isPostgrestError(error) ? (error.details ?? "") : "";

// ❌ Wrong — crashes with TypeError: null.startsWith() / null.includes()
const details = isPostgrestError(error) ? error.details : "";
```

### Always use `captureApiError` for internal API fetch errors

Non-Supabase fetch calls (e.g. `/api/pages/…/versions`) must use `captureApiError`
Expand Down
12 changes: 7 additions & 5 deletions src/lib/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function isE2ETestRequest(event: ErrorEvent): boolean {
*/
function isPostgrestError(
error: unknown,
): error is { message: string; code: string; details: string; hint: string } {
): error is { message: string; code: string; details: string | null; hint: string | null } {
if (error == null || typeof error !== "object") return false;
return (
"message" in error &&
Expand Down Expand Up @@ -317,7 +317,8 @@ const NODE_FETCH_CAUSE_PATTERNS = [
*/
export function isSupabaseAuthLockError(error: Error): boolean {
const msg = error.message;
const details = isPostgrestError(error) ? error.details : "";
// PostgREST 500 responses may have `details: null` — coalesce to ""
const details = isPostgrestError(error) ? (error.details ?? "") : "";

return (
msg.includes("Lock broken by another request") ||
Expand Down Expand Up @@ -429,7 +430,8 @@ export function isSupabaseAuthLockContention(event: ErrorEvent): boolean {
*/
export function isTransientNetworkError(error: Error): boolean {
const msg = error.message;
const details = isPostgrestError(error) ? error.details : "";
// PostgREST 500 responses may have `details: null` — coalesce to ""
const details = isPostgrestError(error) ? (error.details ?? "") : "";

// Browser-style fetch errors.
// The Supabase client may append the hostname in parentheses, e.g.
Expand Down Expand Up @@ -548,8 +550,8 @@ export function captureSupabaseError(

if (isPostgrestError(error)) {
extra.code = error.code;
extra.details = error.details;
extra.hint = error.hint;
extra.details = error.details ?? "";
extra.hint = error.hint ?? "";
}

// Wrap plain objects in a proper Error for Sentry grouping
Expand Down
37 changes: 37 additions & 0 deletions src/lib/sentry.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,21 @@ describe("isTransientNetworkError", () => {
});
expect(isTransientNetworkError(error as unknown as Error)).toBe(true);
});

// --- Null details regression test (MEMO-2E / #944) ---
// PostgREST 500 responses return `details: null`. The `in` operator in
// isPostgrestError passes (key exists), but `null.startsWith()` crashes.

it("does not crash when PostgREST error has details: null (#944 MEMO-2E)", () => {
const error = { message: "server error", code: "XX000", details: null, hint: null };
expect(() => isTransientNetworkError(error as unknown as Error)).not.toThrow();
expect(isTransientNetworkError(error as unknown as Error)).toBe(false);
});

it("still detects transient fetch error when details is null but message matches (#944 MEMO-2E)", () => {
const error = { message: "TypeError: fetch failed", code: "XX000", details: null, hint: null };
expect(isTransientNetworkError(error as unknown as Error)).toBe(true);
});
});

describe("isSchemaNotFoundError", () => {
Expand Down Expand Up @@ -605,6 +620,14 @@ describe("isSupabaseAuthLockError", () => {
});
expect(isSupabaseAuthLockError(error)).toBe(false);
});

// --- Null details regression test (MEMO-2E / #944) ---

it("does not crash when PostgREST error has details: null (#944 MEMO-2E)", () => {
const error = { message: "server error", code: "XX000", details: null, hint: null };
expect(() => isSupabaseAuthLockError(error as unknown as Error)).not.toThrow();
expect(isSupabaseAuthLockError(error as unknown as Error)).toBe(false);
});
});

describe("captureSupabaseError", () => {
Expand Down Expand Up @@ -894,6 +917,20 @@ describe("captureSupabaseError", () => {
expect(opts.level).toBeUndefined();
expect(opts.extra.code).toBe("PGRST500");
});

// --- Null details/hint regression test (MEMO-2E / #944) ---

it("does not crash when PostgREST error has details: null and hint: null (#944 MEMO-2E)", async () => {
const error = { message: "server error", code: "XX000", details: null, hint: null };
expect(() => captureSupabaseError(error as unknown as Error, "page-tree:fetch-pages")).not.toThrow();
await flush();

expect(captureExceptionMock).toHaveBeenCalledOnce();
const [, opts] = captureExceptionMock.mock.calls[0];
expect(opts.extra.details).toBe("");
expect(opts.extra.hint).toBe("");
expect(opts.extra.code).toBe("XX000");
});
});

function makeSentryEvent(
Expand Down
Loading