refactor(client): remove debug console statements and add logger #205
Conversation
WalkthroughThis pull request replaces console.log and console.error statements across the client application with a structured logging utility (loglevel). A new logger helper module is introduced, configured to use "warn" level in production and "debug" otherwise. Error handling in nine files is updated to use the logger, and the loglevel dependency is added to package.json. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Minor attention points:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/app/helpers/pinToIPFS.ts (1)
14-24: Implement HTTP error handling or error sanitization to prevent unfiltered API errors from being logged.The logger uses
loglevelwith no built-in error sanitization. While the JWT token won't appear in caught error objects, the code doesn't checkresponse.okbefore callingresponse.json(), so API error responses could potentially be logged without filtering. Implement one of the following:
- Check
response.okand handle HTTP errors separately before parsing the response- Add error sanitization to filter or redact potentially sensitive data before logging
The same pattern appears in
unpinJSONFileand other helpers.
🧹 Nitpick comments (5)
client/app/components/Cards/BallotCards/ScoreBallot.tsx (1)
43-45: Behavior preserved after removing debug log; small readability tweakRemoving the debug console output while keeping the toast + early return is fine. For readability, consider splitting the toast and return onto separate lines:
- if (newSum > 100) { - toast.error("Credit Limit Exceeded !!"); return; - } + if (newSum > 100) { + toast.error("Credit Limit Exceeded !!"); + return; + }client/app/helpers/fetchFileFromIPFS.ts (1)
1-2: Centralized logging added; consider making the failure contract explicitReplacing the console log with
logger.error("fetchFileFromIPFS: failed to fetch file from IPFS", error);aligns with the new logger helper. Right now the function silently returnsundefinedon failure; if callers depend on this, consider making the contract explicit (e.g.,return nullon failure or rethrow) so downstream code can handle the error path more predictably.Also applies to: 12-14
client/app/helpers/ErrorMessage.ts (1)
1-2: Avoid double‑logging and harden against non‑Error inputsUsing
logger.errorhere is fine, but a couple of tweaks would make this helper more robust:
Double logging: Callers like
create/page.tsxalready log the error with contextual prefixes and then callErrorMessage(error), which now logs again here. To keep logs less noisy, consider choosing a single place to log:
- Either keep contextual
logger.errorin callers and makeErrorMessagepure (no logging), or- Let
ErrorMessageown the logging and have callers rely only on its return value + toast.Safe access to
error.message: If a non‑Error object (or something without amessagestring) is ever passed in,error.message.includes(key)can throw. A small guard makes this safer:-export const ErrorMessage = (error: any) => { - logger.error("ErrorMessage helper received error", error); - for (const [key, message] of Object.entries(errorMessages)) { - if (error.message.includes(key)) { - return message; - } - } - return "Something went wrong. Please try again."; -}; +export const ErrorMessage = (error: any) => { + logger.error("ErrorMessage helper received error", error); + + const rawMessage = (error as any)?.message; + if (typeof rawMessage === "string") { + for (const [key, message] of Object.entries(errorMessages)) { + if (rawMessage.includes(key)) { + return message; + } + } + } + + return "Something went wrong. Please try again."; +};Also applies to: 17-24
client/app/components/ChatBot/ChatBot.tsx (1)
7-8: User‑facing error toast is a good replacement for debug console loggingSwitching from a console error to
toast.error("Failed to get response. Please try again.");gives users immediate feedback without cluttering the console. If you later want more diagnostic detail during development, you could also log via the sharedloggerhelper in this catch block in addition to the toast.Also applies to: 42-55
client/app/components/Helper/CrossChain.tsx (1)
7-9: Logging + toast on cross‑chain failure look good; consider awaitingswitchChainUsing
logger.error("CrossChain: failed to add election", error);plus a user‑facingtoast.error(...)in the catch block is a nice upgrade from raw console logging.One behavioral improvement to consider while you’re here:
if (chain?.id !== 43113) switchChain({ chainId: avalancheFuji.id }); await writeContractAsync({ ... });Since
switchChainfromuseSwitchChainis asynchronous in wagmi 2.x, not awaiting it can race the subsequentwriteContractAsynccalls, potentially attempting approvals/transactions before the wallet has actually switched networks. A safer pattern would be:- try { - if (chain?.id !== 43113) switchChain({ chainId: avalancheFuji.id }); + try { + if (chain?.id !== 43113) { + await switchChain({ chainId: avalancheFuji.id }); + } await writeContractAsync({ address: LINK_FUJI, abi: erc20Abi, functionName: "approve", args: [CCIP_FUJI_ADDRESS, BigInt("25000000000000000000")], }); await writeContractAsync({ abi: CCIPSender, address: CCIP_FUJI_ADDRESS, functionName: "addElection", args: [electionAddress], });If you’re unsure, please confirm in the wagmi v2 docs that
switchChainreturns a Promise and is intended to be awaited before subsequent contract calls.Also applies to: 23-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
client/app/components/Cards/BallotCards/ScoreBallot.tsx(1 hunks)client/app/components/ChatBot/ChatBot.tsx(2 hunks)client/app/components/Helper/CrossChain.tsx(2 hunks)client/app/components/Modal/AddCandidate.tsx(2 hunks)client/app/components/Modal/Vote.tsx(2 hunks)client/app/create/page.tsx(2 hunks)client/app/helpers/ErrorMessage.ts(2 hunks)client/app/helpers/fetchFileFromIPFS.ts(2 hunks)client/app/helpers/logger.ts(1 hunks)client/app/helpers/pinToIPFS.ts(3 hunks)client/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/app/components/Modal/AddCandidate.tsx (1)
client/app/helpers/ErrorMessage.ts (1)
ErrorMessage(17-25)
🔇 Additional comments (9)
client/package.json (1)
11-27: loglevel dependency placement looks appropriateAdding
"loglevel": "^1.9.2"underdependenciesmatches its use from client-side code via the new logger helper; no manifest issues from this change alone.If you want to double-check dependency resolution, verify with your package manager (e.g.,
npm ls loglevel) after installing.client/app/create/page.tsx (1)
16-17: CreatePage now uses centralized logger correctlyImporting the shared
loggerhelper and usinglogger.error("CreateElection: failed to create election", error);in the catch block is consistent with the new logging approach and keeps user-facing feedback viatoast.error(ErrorMessage(error)). No issues with this change.Also applies to: 96-99
client/app/helpers/logger.ts (1)
1-9: [rewritten comment]
[classification tag]client/app/components/Modal/Vote.tsx (2)
9-9: LGTM: Logger import added correctly.The import statement is correct and consistent with the pattern used across the PR.
43-43: LGTM: Logger usage in error handling is appropriate.The error logging is descriptive and properly placed. The user-facing error message is preserved via toast, maintaining good UX.
client/app/components/Modal/AddCandidate.tsx (2)
17-17: LGTM: Logger import added correctly.The import statement follows the established pattern from the PR.
55-56: LGTM: Logger usage in error handling is correct.The error logging with descriptive message and preserved user notification maintains good error handling practices.
client/app/helpers/pinToIPFS.ts (2)
1-1: LGTM: Logger import added correctly.The import follows the consistent pattern established across the PR.
27-39: LGTM: Error logging pattern is consistent.The logger usage in
unpinJSONFilefollows the same correct pattern aspinJSONFilewith appropriate error rethrowing.
1 participant
This PR removes ad-hoc console.log, console.warn, and console.error usage from the client code and replaces them with a centralized logging utility.
⸻
Why Use a Logger
Using raw console statements in production code:
• Cannot be controlled per environment
• Leads to inconsistent and noisy logs
• Does not scale well as the codebase grows
A logger provides log levels, centralized control, and cleaner production output while keeping useful debugging information in development.
⸻
What Was Done
• Removed debug-only console statements
• Replaced necessary logs with a logger utility
• Kept user-facing error handling (e.g. toast messages) unchanged
• Scoped all changes strictly to the client codebase
⸻
Fixs:#174
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.