🧹 [code health] Replace console.error with application logger in github.ts#107
🧹 [code health] Replace console.error with application logger in github.ts#107
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
There was a problem hiding this comment.
Code Review
This pull request replaces console.error with a centralized logger in src/lib/github.ts. A suggestion was made to update the log message for rejected promises to be more accurate, as the current "suppressed" terminology is misleading given that the errors are handled later in the execution flow.
|
|
||
| // Suppress unhandled promise rejections for subsequent pages if we break early or throw | ||
| promises.forEach((p) => p.catch((e) => console.error("Suppressed event fetch error:", e))); | ||
| promises.forEach((p) => p.catch((e) => logger.error("Suppressed event fetch error:", e))); |
There was a problem hiding this comment.
While this change correctly switches to the centralized logger, the log message 'Suppressed event fetch error' can be misleading. When a promise is awaited in the loop below and rejects, this log is printed, but the error isn't truly 'suppressed'—it's handled by the try/catch block and causes the loop to terminate early. This can make debugging confusing. Per repository guidelines, ensure that when handling promise rejections, the error is logged to facilitate debugging.
| promises.forEach((p) => p.catch((e) => logger.error("Suppressed event fetch error:", e))); | |
| promises.forEach((p) => p.catch((e) => logger.error("Event fetch promise rejected:", e))); |
References
- When suppressing unhandled promise rejections, log the error instead of silently ignoring it to facilitate debugging.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/github.ts`:
- Line 2: The import in src/lib/github.ts uses a relative path for logger;
change the import of logger to use the project path alias (e.g., replace the
relative import of logger with an import from "@/logger") so it follows the repo
rule for src imports; update the import statement that references logger to use
the "@/logger" alias and run a quick typecheck to ensure the alias resolves.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96013737-85f7-458d-a450-2aa833a6d11a
📒 Files selected for processing (1)
src/lib/github.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed both review items in : switched import to and changed the rejection log message to (commit ). Added ❤️ reactions and resolved addressed threads. |
Acknowledged. |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Switch logger import to @/ alias and clarify rejected-promise log wording in fetchActivity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Deployment failed with the following error: Learn More: https://vercel.com/hirokis-projects-afd618c7?upgradeToPro=build-rate-limit |
|
Addressed the review feedback and pushed commit
Could you please re-review this PR? |
Acknowledged. |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
🎯 What: The code health issue addressed
Replaced a hardcoded
console.errorwith the centralizedlogger.errorfor suppressed event fetch errors insrc/lib/github.ts.💡 Why: How this improves maintainability
This improves maintainability and log consistency across the application by ensuring all errors pass through the custom logging utility instead of raw console methods, making it easier to route errors to external services in production.
✅ Verification: How you confirmed the change is safe
Ran
npx vitest run src/lib/__tests__/github.test.tswhich successfully output the new logger message, and ran the full test suite (npm run test -- --run) to confirm no regressions were introduced. Also passed ESLint validation.✨ Result: The improvement achieved
A cleaner, more consistent codebase without raw console calls polluting production logs.
PR created automatically by Jules for task 2039028350236114691 started by @is0692vs