Skip to content

🧹 [コード健全性向上] ログ出力の共通化 #52

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

Closed
is0692vs wants to merge 1 commit into from
Closed

🧹 [コード健全性向上] ログ出力の共通化 #52

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
4 changes: 3 additions & 1 deletion src/app/[username]/error.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import { useEffect } from "react";
import Link from "next/link";

import { logger } from "@/lib/logger";

export default function ErrorPage({
error,
reset,
Expand All @@ -11,7 +13,7 @@ export default function ErrorPage({
reset: () => void;
}) {
useEffect(() => {
console.error("User page error:", error);
logger.error("User page error:", error);
}, [error]);
Comment on lines 15 to 17
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

React 19 Error Boundary automatic console.error logging behavior

💡 Result:

In React 19, errors that are thrown during rendering and caught by an Error Boundary are still automatically logged by React (i.e., React calls console.error by default), but the shape/amount of logging changed:

  • React ≤18 (DEV): a single caught render error often produced multiple console entries (the error could be thrown/logged more than once, plus the “The above error occurred…” message), which looked like duplicate console.error noise. [1]
  • React 19 (DEV): React deduplicates this so you typically see one consolidated console error that includes both the error + component-stack context (“The above error occurred… React will try to recreate…”) in the same log. [1]
  • React 19 render bailout behavior: after the first render error in a subtree, React 19 is more likely to bail out sooner (so later siblings might not render/log), which can change what you see in the console compared to React 18. [2]

Can you turn off React’s automatic console.error for caught errors?

Not via Error Boundary APIs directly—React logs by default. What React 19 adds is root-level hooks so you can implement your own reporting behavior:

  • createRoot(container, { onCaughtError, onUncaughtError, onRecoverableError }) [3]
  • hydrateRoot(domNode, reactNode, { onCaughtError, onUncaughtError, onRecoverableError }) [4]

The createRoot docs explicitly note: “By default, React will log all errors to the console.” and then show using these hooks for custom reporting. [3]

Practical implication: if you also console.error(...) inside componentDidCatch, you’ll get your log plus React’s default log (so it can look “duplicated”), unless you move logging to onCaughtError (or otherwise dedupe/filter on your side). [1][3]

Sources: [1] React 19 “Better error reporting” section (React blog) https://react.dev/blog/2024/12/05/react-19 • [2] Example/observed behavior change after upgrading to React 19 https://andrei-calazans.com/posts/react-19-error-boundary-changed/ • [3] createRoot docs (“By default, React will log all errors…”) https://react.dev/reference/react-dom/client/createRoot • [4] hydrateRoot docs (root error hooks) https://react.dev/reference/react-dom/client/hydrateRoot

React 19 の Error Boundary による自動ログのため、エラーログが二重記録されます。

React 19 では Error Boundary に捕捉されたエラーが自動的に console.error に報告されます。このため、line 16 の logger.error("User page error:", error) を残すと、React による自動ログと合わせて同じエラーが二重に記録されます。将来 logger を Sentry などの外部サービスに切り替えた場合、同じイベントが重複して送信されることになります。

対応としては以下のいずれかを推奨します：

  • useEffect ごと削除する
  • 外部送信用の特別なハンドラーがある場合のみログを出力するよう条件を追加する

useEffect を削除する場合は、logger と useEffect の import も不要になります。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/app/`[username]/error.tsx around lines 15 - 17, Remove the redundant
useEffect that calls logger.error in the error boundary (the block using
useEffect(() => { logger.error("User page error:", error); }, [error])); either
delete that entire useEffect and also remove the now-unused imports of useEffect
and logger from the module, or replace it with a guarded call that only logs
when an explicit external-error-forwarding flag/handler is present (e.g., check
for a configured externalErrorHandler before calling logger.error). Ensure you
update imports accordingly so no unused symbols remain.


const isRateLimit = error.message.includes("rate limit");
Expand Down
4 changes: 3 additions & 1 deletion src/app/api/og/[username]/route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { ImageResponse } from "next/og";
import { NextRequest } from "next/server";

import { logger } from "@/lib/logger";

export const runtime = "edge";

export async function GET(
Expand Down Expand Up @@ -30,7 +32,7 @@
publicRepos = data.public_repos ?? 0;
}
} catch (error) {
console.error(`Failed to fetch GitHub profile for OG image: ${username}`, error);
logger.error(`Failed to fetch GitHub profile for OG image: ${username}`, error);
Comment on lines 34 to +35
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

メッセージ文字列に username を埋め込まないでください。

Line 35 のような動的メッセージは、logger の送信先を外部監視へ切り替えたときにイベント集約を壊し、ユーザー識別子もそのまま残ります。固定メッセージ + 構造化コンテキストにしてください。

🩹 提案差分 
-    logger.error(`Failed to fetch GitHub profile for OG image: ${username}`, error);
+    logger.error("Failed to fetch GitHub profile for OG image", { username }, error);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/app/api/og/`[username]/route.tsx around lines 34 - 35, Replace the
dynamic interpolated log message in logger.error so it uses a fixed string and
moves the username into structured context: update the catch block that calls
logger.error (the call referencing logger.error and the username variable) to
log a constant message like "Failed to fetch GitHub profile for OG image" and
pass the username and the caught error as structured fields/metadata (e.g., an
object with username and error) instead of embedding ${username} in the message.

// fallback to defaults
}

Expand All @@ -56,7 +58,7 @@
}}
>
{avatarUrl && (
<img

Check warning on line 61 in src/app/api/og/[username]/route.tsx

View workflow job for this annotation

GitHub Actions / Lint 

Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element
src={avatarUrl}
alt=""
width={120}
Expand Down
7 changes: 4 additions & 3 deletions src/components/CardGenerator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useState, useRef, useCallback, useEffect } from "react";
import { createPortal } from "react-dom";
import { toPng, toBlob } from "html-to-image";

import { logger } from "@/lib/logger";
import type {
CardBlockId,
CardDisplayOptions,
Expand Down Expand Up @@ -130,7 +131,7 @@ export default function CardGenerator({ summary }: Props) {
});
return dataUrl;
} catch (err) {
console.error("Failed to generate image", err);
logger.error("Failed to generate image", err);
return null;
}
}, []);
Expand Down Expand Up @@ -173,7 +174,7 @@ export default function CardGenerator({ summary }: Props) {
setPreviewUrl(url);
}
} catch (err) {
console.error("Failed to generate image", err);
logger.error("Failed to generate image", err);
if (!isCancelled) {
setPreviewUrl(null);
}
Expand Down Expand Up @@ -229,7 +230,7 @@ export default function CardGenerator({ summary }: Props) {
setCopyStatus("copied");
setTimeout(() => setCopyStatus("idle"), 2000);
} catch (err) {
console.error("Failed to copy", err);
logger.error("Failed to copy", err);
setCopyStatus("error");
}
}, []);
Expand Down
3 changes: 2 additions & 1 deletion src/components/ThemeController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { useEffect } from "react";
import { FastAverageColor } from "fast-average-color";
import { adjustAccentColor } from "@/lib/color";
import { logger } from "@/lib/logger";

type Props = {
avatarUrl?: string;
Expand Down Expand Up @@ -36,7 +37,7 @@ export default function ThemeController({ avatarUrl, topLanguageColor }: Props)
}
})
.catch((e) => {
console.warn("Failed to extract color from avatar, keeping fallback color.", e);
logger.warn("Failed to extract color from avatar, keeping fallback color.", e);
});
}

Expand Down
25 changes: 25 additions & 0 deletions src/lib/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Centralized logger for the application.
* Currently wraps console methods, but can be easily extended to use
* external logging services (e.g., Sentry, Axiom, Datadog) in the future.
*/

type LogFn = (message: string, ...args: unknown[]) => void;

interface Logger {
info: LogFn;
warn: LogFn;
error: LogFn;
}

export const logger: Logger = {
info: (message, ...args) => {
console.info(message, ...args);
},
warn: (message, ...args) => {
console.warn(message, ...args);
},
error: (message, ...args) => {
console.error(message, ...args);
},
Comment on lines +15 to +24
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🌐 Web query:

DevTools console callsite attribution wrapped function vs console.bind preserving caller location

💡 Result:

In Chrome/Edge DevTools, the “callsite” link on a console message is basically the top non-ignored JavaScript stack frame that ultimately invoked the console method. That’s why the two patterns behave differently:

Why a wrapper changes the callsite

If you do:

const orig = console.log.bind(console);
function log(...args) {
  // extra behavior
  orig(...args);
}

DevTools sees the stack as caller → log(…) → orig(…) → (native console), so the first visible frame is your wrapper, and the callsite link points at the wrapper file/line. This is the behavior people run into when “decorating” console methods. [1], [2]

Why console.log.bind(console) (often) preserves the callsite

If you do:

const log = console.log.bind(console);
log("hi");

you’re still calling the native console method (just with this fixed), so there’s no extra userland wrapper frame added by you—DevTools can attribute the callsite to the actual caller.

Also, if your goal is only to prepend fixed arguments (timestamp/prefix/styles) and keep caller attribution, binding/partial application can work because it stays a direct call into the console implementation:

console.log = Function.prototype.bind.call(console.log, console, new Date(), "You logged:");

This pattern is specifically noted as keeping line numbers/callsite “preserved” while adding a prefix. [3]

If you must run extra code and still want the “real” callsite

You generally can’t make an arbitrary JS wrapper “transparent” to DevTools purely with language features; the practical fix is to hide/ignore the wrapper in DevTools so the next frame (the real caller) becomes the attributed callsite:

  • Blackbox / Ignore-list the wrapper script (DevTools Settings → Blackboxing, or “Add script to ignore list”). Then console messages will attribute to the caller instead of the wrapper. [1], [2]
  • For build tooling, you can also use source map ignore lists (the modern ignoreList mechanism) so Chrome ignores specific generated files automatically. [3]

Rule of thumb

  • Want same callsite: store a bound console method (or bind-with-prefix).
  • Want extra behavior: wrap, but then ignore-list/blackbox that wrapper file if you want DevTools to point to the original caller.

Sources: [1] [2] [3]

console を関数で包むと DevTools 上の出力元が追いづらくなります。

この形だと DevTools が src/lib/logger.ts をコールサイトとして指定しやすく、実際の呼び出し元を辿りにくくなります。共通化だけが目的なら、bind 済みメソッドをそのまま公開したほうがデバッグ性を損なわせません。

♻️ 提案差分 
 export const logger: Logger = {
-  info: (message, ...args) => {
-    console.info(message, ...args);
-  },
-  warn: (message, ...args) => {
-    console.warn(message, ...args);
-  },
-  error: (message, ...args) => {
-    console.error(message, ...args);
-  },
+  info: console.info.bind(console) as LogFn,
+  warn: console.warn.bind(console) as LogFn,
+  error: console.error.bind(console) as LogFn,
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const logger: Logger = {
info: (message, ...args) => {
console.info(message, ...args);
},
warn: (message, ...args) => {
console.warn(message, ...args);
},
error: (message, ...args) => {
console.error(message, ...args);
},
export const logger: Logger = {
info: console.info.bind(console) as LogFn,
warn: console.warn.bind(console) as LogFn,
error: console.error.bind(console) as LogFn,
};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib/logger.ts` around lines 15 - 24, 現在の logger 定義 (export const logger:
Logger with info/warn/error arrow functions) wraps console calls and loses
original callsite info in DevTools; replace these wrappers by exporting the
bound console methods directly (e.g. set logger.info =
console.info.bind(console), same for warn and error) so the Debugger sees
correct callsites while keeping the Logger shape; update the logger constant
(info, warn, error) to use bound methods rather than arrow wrappers.

};
Comment on lines +7 to +25
Copy link

@gemini-code-assist gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

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

medium

現在の LogFn 型は、最初の引数を文字列に限定しています。これは、ラップしている console メソッドよりも制限が厳しいです。console メソッドは、最初の引数として任意の型を受け入れることができます（例: console.error(errorObject)）。
ロガーをより柔軟にし、console メソッドの完全な代替として機能させるために、シグネチャを可変長引数を受け入れるように変更することをお勧めします。これにより、将来的にSentryなどのエラー追跡ツールと統合する際にも、エラーオブジェクトを直接渡しやすくなります。

Suggested change
type LogFn = (message: string, ...args: unknown[]) => void;
interface Logger {
info: LogFn;
warn: LogFn;
error: LogFn;
}
export const logger: Logger = {
info: (message, ...args) => {
console.info(message, ...args);
},
warn: (message, ...args) => {
console.warn(message, ...args);
},
error: (message, ...args) => {
console.error(message, ...args);
},
};
type LogFn = (...args: unknown[]) => void;
interface Logger {
info: LogFn;
warn: LogFn;
error: LogFn;
}
export const logger: Logger = {
info: (...args) => {
console.info(...args);
},
warn: (...args) => {
console.warn(...args);
},
error: (...args) => {
console.error(...args);
},
};

Loading