refactor: Centralize logging and Sentry error reporting by askov · Pull Request #871 · solana-foundation/explorer

askov · 2026-03-06T13:40:27Z

Description

Centralized Logger: Replace app/utils/logger (static class, default export) with app/shared/lib/logger (singleton instance, named export)
Sentry consolidation: All direct @sentry/nextjs imports replaced by Logger.panic()/Logger.warn({ sentry: true }) or re-exports from app/shared/lib/sentry/. ESLint no-restricted-imports rule enforces this boundary
Global error boundaries: New app/error.tsx and app/global-error.tsx catch route-level and root-level errors, replacing per-component ErrorBoundary wrappers
ESLint enforcement: no-console: error globally to prevent regression
Sentry SDK upgrade: Pin @sentry/core and @sentry/nextjs to 10.39.0, migrate build config to webpack.treeshake.removeDebugLogging
Test cleanup: Global Logger mock in test-setup.specs.ts replaces ~8 per-file mock blocks

Breaking changes

NEXT_PUBLIC_ENABLE_CATCH_EXCEPTIONS env var removed — Sentry capture is now always-on when DSN is configured
SENTRY_SUPPRESS_GLOBAL_ERROR_HANDLER_FILE_WARNING env var removed
Import paths changed: @utils/logger → @/app/shared/lib/logger, SentryErrorBoundary and withTraceData moved to @/app/shared/lib/sentry

Type of change

Other (please describe): Enable sentry for all pages, logger refactoring

Screenshots

Testing

Related Issues

Closes HOO-246

Checklist

My code follows the project's style guidelines
I have added tests that prove my fix/feature works
All tests pass locally and in CI
I have updated documentation as needed
I have run build:info script to update build information
CI/CD checks pass

vercel · 2026-03-06T13:40:32Z

@askov is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

app/shared/lib/logger.ts

app/entities/idl/model/use-format-anchor-idl.ts

rogaldh · 2026-03-09T20:09:58Z

app/shared/lib/logger.ts

+        isLoggable(LOG_LEVEL.PANIC) && console.error(...formatArgs(message, context));
+    }
+    error(message: string, context?: Record<string, unknown>) {
+        isLoggable(LOG_LEVEL.ERROR) && console.error(...formatArgs(message, context));


Let's preserve the original logic about using the source error and fall back to a generic error, like here: https://github.com/solana-foundation/explorer/blob/master/app/utils/logger.ts#L44-L48

The intent was to save the original error without modifications. Having "Unrecognised error" means that something is wrong and requires attention.

That also means that we have to wrap strings passed to Logger.error with error instances

I'm not sure if I fully got your idea. I updated it to log 'unexpected' error types and to only send error instances to sentry. But converting an unexpected unknown to an error and console.error it with a default message seems unnecessary, doesn't it? You already have a debug log associated with it.

I mean that Logger.error should allow passing an' Error' instance as its first argument. That seems reasonable.

Oh, now I see what you mean.

Could you elaborate on this?

I don’t see any strong reasons for allowing the Error argument to come first.
Well, the only advantage I can see is that it makes catch blocks more ergonomic. 

Logger.error(err) // is shorter than Logger.error(‘[idl:form] failed to load deps’, { error: err })

However, the cons are:

It breaks the uniform signature, making the Error the odd one out

The quality of sentry messages degrades. If someone passes an Error directly and captureMessage is triggered, the sentry issue title becomes err.message, which is often generic (like "Network request failed" or “Bigint conversion failed”) instead of a stable, searchable string like [tx:fetch] failed to load transaction.

Overloads are typically used for functions that can be called in fundamentally different ways for a reason. But why apply this to logs? I mean, it only increases developer friction and leads to unclear logs, such as Logger.error(error, { sentry: true }), where the error is often some third-party issue that the developer may not understand well.

The uniform (message, context) signature is a deliberate trade-off that prioritizes stable, searchable log messages and clean Sentry grouping over convenience at the call site. The "boilerplate" of Logger.error('[module] descriptive message', { error: err }) is actually a feature—it compels callers to describe what failed, rather than simply forwarding an opaque error.

I see your point.

But I think not using Errors and allowing strings or objects to describe errors discourages their use in general, which is not good and prevents building structures that handle various errors separately.

captureMessage is triggered

If we write code and specifically pass e.message into the capture, we did this intentionally; I do not mean we have to pass it as is to make it shorter, but we can definitely do that if needed, for ex.: .catch(Logger.error).

What stops us from wrapping the original error with a fine-grained error?

Logger.error(new NamespacedError('[idl:form] failed to load deps'), { cause: err }) // OR Logger.error(new IdlFormError('Failed to load deps', { cause: err }))

I think the last line is almost identical to Logger.error(‘[idl:form] failed to load deps’, { error: err }), but the variant with an Error instance has more benefits in the long run than the variant with a string.

Speaking of the uniform signature. That is definitely a good thing, but we do not use captureMessage on par with Logger.error; it is inside the implementation, so we can extract the message from the error. Having a proper error-handling structure outweighs a uniform signature. I do not think we are aiming to allow using Logger.error and Sentry.captureMessage interchangeably.

Using overloads was just a proposal to describe the possibility of using Errors and all other types that fall into the unknown category. We can do unions or anything else.

To summarize:

I do not mean we have to have the shortest form to log the error

Error is designed to handle errors, and having fine-grained/namespaced errors is possible with it

Okay, I agree with you that it's flexible and logical. I've updated the code.

However, the concern about this relaxed usage still remains. When you have an 'unknown' error parameter, it always (I'm intentionally exaggerating here, but you get what I mean - this is an open-source project 😄) leads to people simply using Logger.error(err) without actually creating any specialized error instances or causes.

We could write an ESLint rule that would warn from using it that way. LGTM

… behavior

…n API routes

…r function

…or non-Error values

…th additional context

…gger calls

C0mberry · 2026-03-13T11:45:50Z

app/api/metadata/proxy/route.ts

        const isPrivate = await checkURLForPrivateIP(parsedUrl);
        if (isPrivate) {
-            Logger.error(new Error('Private IP detected'), parsedUrl.hostname);
+            Logger.error('[api:metadata-proxy] Private IP detected', { hostname: parsedUrl.hostname });


the original error message will be lost here
resolveError() will convert it to new Error('Unrecognized error')

Yeh, I missed a few of these during the latest refactoring commit (changed the signature to less strict). Fixed all the cases like this

C0mberry · 2026-03-13T11:50:59Z

app/api/verification/coingecko/[coinId]/route.ts

        if (!response.ok) {
            if (response.status === 429) {
-                Sentry.captureMessage('Coingecko API rate limit exceeded', { level: 'warning' });
+                Logger.error('[api:coingecko] Rate limit exceeded', { sentry: true });


we could also lose the error message
maybe Logger.warn would be fit for this place

Replaced with warn (all the 429 in routes)

C0mberry · 2026-03-13T12:00:39Z

app/error.tsx

+
+    return (
+        <div className="container mt-4">
+            <ErrorCard text={error.message} retry={reset} />


raw error message could be displayed to the user

Updated to the generic message

C0mberry · 2026-03-13T12:06:39Z

app/error.tsx

@@ -0,0 +1,18 @@
+'use client';


do we need tests for it or app/global-error.tsx?

Yes, here're some docs

…ects across various modules

rogaldh reviewed Mar 9, 2026

View reviewed changes

app/shared/lib/logger.ts Show resolved Hide resolved

rogaldh reviewed Mar 9, 2026

View reviewed changes

app/entities/idl/model/use-format-anchor-idl.ts Outdated Show resolved Hide resolved

rogaldh reviewed Mar 9, 2026

View reviewed changes

askov and others added 11 commits March 10, 2026 15:19

refactor: Centralize logging and Sentry error reporting

8c8e719

nit: fix codestyle

1ea9ea8

nit: apply codestyle

278584c

nit: apply codestyle

1365d20

docs: Update .env.example and logger documentation to clarify logging…

8fdcc1b

… behavior

feat: extend logger sentry integration

e8e88f9

fix: Update logging level for rate limit exceeded warnings to error i…

4de2ccb

…n API routes

refactor: Enhance error handling in logger by introducing resolveErro…

7816a93

…r function

refactor: Improve error handling in logger by adding debug warnings f…

bc863ac

…or non-Error values

refactor: update logging for idl formatting errors to use warnings wi…

82c89a2

…th additional context

chore: run lint

38ac7be

askov force-pushed the refactor/centralize-logging-and-sentry branch from 3abff86 to 38ac7be Compare March 10, 2026 08:19

askov added 3 commits March 10, 2026 15:22

refactor: integrate Logger for error handling in useCopyToClipboard

f14fdb3

chore: run lint

ad29691

chore: run build:info

303b338

rogaldh mentioned this pull request Mar 12, 2026

chore(deps): bump @sentry/nextjs from 10.17.0 to 10.27.0 #800

Open

refactor: standardize error logging by removing message context in Lo…

f5d2c49

…gger calls

C0mberry reviewed Mar 13, 2026

View reviewed changes

askov added 2 commits March 13, 2026 19:46

fix: update error message display to a generic fallback text

ac5e1f5

refactor: standardize error logging by wrapping messages in Error obj…

5874845

…ects across various modules

Conversation

askov commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking changes

Type of change

Screenshots

Testing

Related Issues

Checklist

Uh oh!

vercel bot commented Mar 6, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

askov Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

askov commented Mar 6, 2026 •

edited

Loading

askov Mar 13, 2026 •

edited

Loading