Conversation
Introduced a centralized logger in src/lib/logger.ts and replaced all instances of console.error and console.warn with logger methods to improve maintainability. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、アプリケーションのログ出力メカニズムを標準化し、コードの健全性と保守性を向上させることを目的としています。 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughSummary by CodeRabbitリリースノート
Walkthroughアプリケーション全体で分散していたコンソールログを一元化するため、新しい Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、console の呼び出しを中央集権的な logger ユーティリティに置き換えることで、ログ出力の一元化を目指すものですね。全体的なアプローチは素晴らしく、将来のメンテナンス性向上に大きく貢献すると思います。
新しく追加された src/lib/logger.ts について、1点改善提案があります。現在の実装ではロガー関数の最初の引数が文字列に限定されていますが、これをより柔軟にすることで、console と同じように任意の型のデータをログ出力できるようになり、さらに使いやすさが向上します。詳細はコメントをご確認ください。
| 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); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
現在の LogFn 型は、最初の引数を文字列に限定しています。これは、ラップしている console メソッドよりも制限が厳しいです。console メソッドは、最初の引数として任意の型を受け入れることができます（例: console.error(errorObject)）。
ロガーをより柔軟にし、console メソッドの完全な代替として機能させるために、シグネチャを可変長引数を受け入れるように変更することをお勧めします。これにより、将来的にSentryなどのエラー追跡ツールと統合する際にも、エラーオブジェクトを直接渡しやすくなります。
| 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); | |
| }, | |
| }; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[username]/error.tsx:
- Around line 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.
In `@src/app/api/og/`[username]/route.tsx:
- Around line 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.
In `@src/lib/logger.ts`:
- Around line 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.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed93c3cb-7d51-418e-b7c7-1d9d08bce002
📒 Files selected for processing (5)
src/app/[username]/error.tsxsrc/app/api/og/[username]/route.tsxsrc/components/CardGenerator.tsxsrc/components/ThemeController.tsxsrc/lib/logger.ts
| useEffect(() => { | ||
| console.error("User page error:", error); | ||
| logger.error("User page error:", error); | ||
| }, [error]); |
There was a problem hiding this comment.
🧩 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.errornoise. [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.
| } 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); |
There was a problem hiding this comment.
メッセージ文字列に 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.
| 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); | ||
| }, |
There was a problem hiding this comment.
🧹 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
ignoreListmechanism) 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.
| 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.
|
Consolidated into the |
1 participant
🎯 What:
console.errorとconsole.warnの直接呼び出しを、新設したsrc/lib/logger.tsユーティリティに置き換えました。💡 Why: ログ出力を一元化することで、将来的に外部の監視ツール（Sentry等）への移行が容易になり、コードの保守性が向上します。
✅ Verification: スクリプトによる全ファイルのチェックを行い、
consoleの直接呼び出しがloggerに置き換わっていることを確認しました。✨ Result: コードの健全性が向上し、今後の拡張に対応しやすくなりました。
PR created automatically by Jules for task 13561573928386060371 started by @is0692vs