Skip to content

fix: filter Sentry noise, fix bugs, and add error debugging#391

Open
williamluke4 wants to merge 6 commits intomainfrom
williamluke4/sentry-errors
Open

fix: filter Sentry noise, fix bugs, and add error debugging#391
williamluke4 wants to merge 6 commits intomainfrom
williamluke4/sentry-errors

Conversation

@williamluke4
Copy link
Copy Markdown
Member

Summary

  • Filter noisy Sentry errors: Added ignoreErrors on client (DOM manipulation, camera, WalletConnect, router state) and beforeSend on server/edge to drop expected FORBIDDEN TRPCErrors. Wrapped onRequestError to skip permission errors. Reduced tracesSampleRate from 1.0 to 0.2.
  • Fix bugs: Added optional chaining for stats?.transactions?.delta in voucher statistics grid to prevent TypeError. Added isAddress() validation in pool pages to prevent InvalidAddressError on malformed URLs.
  • Add error boundaries: New error.tsx for /vouchers/[address] and /pools/[address] routes with retry and navigation.
  • Improve debugging: File upload now has 30s timeout and includes HTTP status in errors. Webcam component handles camera errors with toast + fallback. Token fetch errors include address and preserve error chain. Writer lock failures add Sentry breadcrumbs. Fixed publicClient vs client bug in token model.

Test plan

  • pnpm check-types passes
  • pnpm test passes (355/355)
  • pnpm lint passes (0 errors)
  • Monitor Sentry after deploy for reduction in filtered error categories

🤖 Generated with Claude Code

williamluke4 and others added 2 commits April 9, 2026 10:32
Filter expected errors from Sentry (FORBIDDEN TRPCErrors, browser extension DOM errors,
camera/WalletConnect noise), fix optional chaining bugs in voucher stats and address
validation in pool pages, add error boundaries for voucher/pool detail pages, improve
file upload error messages with timeouts and status codes, add webcam error handling,
and add Sentry breadcrumbs to writer lock failures.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes React hooks rules violation by calling all hooks unconditionally
before the address validation early return.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sarafu-network Ready Ready Preview, Comment Apr 9, 2026 1:56pm

Request Review

@williamluke4
Copy link
Copy Markdown
Member Author

williamluke4 commented Apr 9, 2026

The CI review bot was hitting the 10-turn limit on larger PRs,
causing the action to fail.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Updated to use actions/checkout@v6, enabled track_progress, and
revised the review prompt with structured focus areas.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
williamluke4 and others added 2 commits April 9, 2026 16:43
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merged isAddress validation guard with new editable banner feature
from main, keeping all hooks before the early return.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 9, 2026

Claude finished @williamluke4's task in 2m 27s —— View job


Code Review: fix: filter Sentry noise, fix bugs, and add error debugging

Tasks:

  • Gather context and understand the request
  • Review Sentry configuration changes
  • Review bug fixes (voucher statistics, pool address validation)
  • Review error boundaries (error.tsx files)
  • Review debugging improvements (file upload, webcam, token model, writer)
  • Compile and post final review

Overall this is a solid, well-scoped PR. The publicClient bug fix alone is worth merging. A few issues worth addressing before shipping:


Critical Bug Fix

src/server/api/models/token.ts — The publicClientclient fix is a real bug: the function accepted a client parameter but completely ignored it, always using the global singleton. This could cause hard-to-diagnose issues in test contexts or when a different client is passed. Good catch.


Issues

1. Error boundaries don't report to Sentry

Both error.tsx files only console.error — errors caught by the boundary are never sent to Sentry in production:

// src/app/(main)/pools/[address]/error.tsx:16-19
useEffect(() => {
  console.error("Pool error:", error); // lost in production
}, [error]);

Add Sentry.captureException(error) here. Without it, the error boundary silently swallows errors that Sentry would otherwise surface. Fix this →

2. error.message exposed to users in production

// src/app/(main)/pools/[address]/error.tsx:33-35
<p className="mb-6 text-muted-foreground">
  {error.message || "An unexpected error occurred..."}
</p>

Next.js sanitizes SSR error messages in production (replaces them with a generic string + digest), but client-side errors that bubble to the boundary will show raw messages — which could include database error details, internal API responses, etc. Consider only showing error.message if it's a known safe user-facing message, or just always use the generic fallback. Fix this →

3. JSON.parse in use-file-upload.tsx can throw unhandled

// src/components/file-uploader/use-file-upload.tsx:39
const response = JSON.parse(data) as { ok: boolean; payload: { s3: string } };

If the server returns HTTP 200 with a non-JSON body (e.g., an HTML error page from a proxy), JSON.parse throws synchronously inside xhr.onload — the Promise rejects with an unhandled parse error that bypasses the toast.error call. Wrap in try/catch:

let response: { ok: boolean; payload: { s3: string } };
try {
  response = JSON.parse(data);
} catch {
  toast.error("File upload failed: unexpected server response");
  reject(new Error("File upload failed: invalid JSON response"));
  return;
}

Fix this →

4. Writer lock breadcrumb missing the error itself

// src/contracts/writer.ts:86-91
Sentry.addBreadcrumb({
  category: "writer",
  message: "Writer lock transaction failed",
  data: { lockId }, // error details missing
  level: "error",
});

lockId alone isn't very useful for diagnosis. Add the error message to data:

data: {
  lockId,
  error: error instanceof Error ? error.message : String(error),
},

Fix this →

5. Client-side FORBIDDEN errors not filtered

sentry.server.config.ts and sentry.edge.config.ts have beforeSend to drop FORBIDDEN TRPCErrors, and instrumentation.ts wraps onRequestError. But instrumentation-client.ts has no equivalent — client-side FORBIDDEN errors (e.g., a logged-out user hitting a protected tRPC query) will still appear in Sentry. Consider adding a beforeSend to the client config as well.


Minor Notes

  • Duplicate error boundaries (pools/error.tsx and vouchers/error.tsx are 95% identical): The current duplication is fine for now, but if a third route needs one, extract a shared RouteErrorBoundary component.

  • Zero address fallback (pool-client-page.tsx:24): Using 0x000...000 as the fallback when !isValid is slightly surprising — a null with conditional hook enabling would be cleaner, though the early return at line 53 makes this safe in practice.

  • "Go Home" button: Links to /pools and /vouchers respectively but is labeled "Go Home" — "Browse Pools" / "Browse Vouchers" would be more accurate.


What's Good

  • The publicClient bug fix is important and correct.
  • Optional chaining on stats?.transactions?.delta is the right fix.
  • Tracing sample rate reduction from 1.0 to 0.2 will meaningfully reduce Sentry volume and cost.
  • The isAddress() guard in pool-client-page.tsx before calling getAddress() is correct — without it, viem throws InvalidAddressError on malformed URLs.
  • Token error now includes the address and preserves the error chain with { cause: error }, making these much easier to diagnose.
  • File upload timeout (30s) + HTTP status in errors are practical improvements.
  • The onRequestError wrapper pattern in instrumentation.ts is clean.

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