fix: replace direct console.* calls with SdkLogger in verifiable-credentials package#1598
Conversation
Original prompt from Guillecan you determine the issue and propose a PR in case it's feasible to maintain the silent logs flag in every part of our sdks? |
🦋 Changeset detectedLatest commit: 0033495 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| credentialsLogger.warn(`API key invalid: ${validationResult.message}`); | ||
| } |
There was a problem hiding this comment.
Logger used before initialization - this warning will use the uninitialized fallback (raw console.warn) and won't respect consoleLogLevel="silent". Move initCredentialsLogger(apiKey) from line 106 to before line 90 to ensure all logger calls are initialized.
| credentialsLogger.warn(`API key invalid: ${validationResult.message}`); | |
| } | |
| initCredentialsLogger(apiKey); | |
| const usageOrigin = this.getOrigin(); | |
| if (usageOrigin != null) { | |
| const validationResult = validateAPIKey(apiKey, { | |
| usageOrigin: usageOrigin, | |
| }); | |
| if (!validationResult.isValid) { | |
| credentialsLogger.warn(`API key invalid: ${validationResult.message}`); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/crossmintAPI.ts
Line: 96-97
Comment:
Logger used before initialization - this warning will use the uninitialized fallback (raw `console.warn`) and won't respect `consoleLogLevel="silent"`. Move `initCredentialsLogger(apiKey)` from line 106 to before line 90 to ensure all logger calls are initialized.
```suggestion
initCredentialsLogger(apiKey);
const usageOrigin = this.getOrigin();
if (usageOrigin != null) {
const validationResult = validateAPIKey(apiKey, {
usageOrigin: usageOrigin,
});
if (!validationResult.isValid) {
credentialsLogger.warn(`API key invalid: ${validationResult.message}`);
```
How can I resolve this? If you propose a fix, please make it concise.| credentialsLogger.warn( | ||
| `Using custom environment: ${config.environment}, you may want to use 'staging' or 'prod' instead` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Logger used before initialization - this warning will use the uninitialized fallback and won't respect consoleLogLevel="silent". Will be fixed if you move initCredentialsLogger earlier as suggested above.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/crossmintAPI.ts
Line: 101-104
Comment:
Logger used before initialization - this warning will use the uninitialized fallback and won't respect `consoleLogLevel="silent"`. Will be fixed if you move `initCredentialsLogger` earlier as suggested above.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| import { type ChainRPCConfig, VCChain } from "./types/chain"; | ||
|
|
||
| const vcSdkLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - vcSdkLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead, which gets initialized during crossmintAPI.init().
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/configs.ts
Line: 5
Comment:
Uninitialized logger - `vcSdkLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead, which gets initialized during `crossmintAPI.init()`.
How can I resolve this? If you propose a fix, please make it concise.| import type { EncryptedVerifiableCredential } from "../types"; | ||
| import { isVerifiableCredential } from "../types/utils"; | ||
|
|
||
| const litLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - litLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/encryption/lit.ts
Line: 10
Comment:
Uninitialized logger - `litLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.| import { abi_ERC_7572 } from "./ABI/ERC7572"; | ||
| import { getProvider } from "./provider"; | ||
|
|
||
| const nftLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - nftLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/onchainServices/nft.ts
Line: 14
Comment:
Uninitialized logger - `nftLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.| import type { Collection, CredentialsCollection } from "../types/collection"; | ||
| import { isVerifiableCredentialContractMetadata } from "../types/utils"; | ||
|
|
||
| const contractMetadataLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - contractMetadataLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/presentation/contractMetadata.ts
Line: 9
Comment:
Uninitialized logger - `contractMetadataLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.| import { isVcChain, isVcNft } from "../types/utils"; | ||
| import { ContractMetadataService } from "./contractMetadata"; | ||
|
|
||
| const collectionsLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - collectionsLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/presentation/getCollections.ts
Line: 10
Comment:
Uninitialized logger - `collectionsLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.| import { isVcChain, isVerifiableCredentialContractMetadata, parseLocator } from "../types/utils"; | ||
| import { ContractMetadataService } from "./contractMetadata"; | ||
|
|
||
| const nftByLocatorLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - nftByLocatorLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/presentation/nftByLocator.ts
Line: 10
Comment:
Uninitialized logger - `nftByLocatorLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| import { configManager } from "../configs"; | ||
|
|
||
| const ipfsLogger = new SdkLogger(); |
There was a problem hiding this comment.
Uninitialized logger - ipfsLogger is never .init()'d, so all calls will fall back to raw console.* and won't respect consoleLogLevel="silent". Consider importing and using the shared credentialsLogger from src/logger.ts instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/verifiable-credentials/src/verifiableCredentialsSDK/services/ipfs.ts
Line: 5
Comment:
Uninitialized logger - `ipfsLogger` is never `.init()`'d, so all calls will fall back to raw `console.*` and won't respect `consoleLogLevel="silent"`. Consider importing and using the shared `credentialsLogger` from `src/logger.ts` instead.
How can I resolve this? If you propose a fix, please make it concise.
Description
Part of a series of PRs to make
consoleLogLevel="silent"work across all SDK packages. This PR replaces all ~40 directconsole.*calls in@crossmint/client-sdk-verifiable-credentialswithSdkLoggerinstances so they respect theconsoleLogLevelsetting fromCrossmintProvider.Two patterns are used:
src/layer (crossmintAPI, decryption, presentation, services): Uses a sharedcredentialsLoggersingleton from a newsrc/logger.ts, initialized viainitCredentialsLogger(apiKey)duringcrossmintAPI.init().src/verifiableCredentialsSDK/layer (configs, encryption/lit, onchainServices/nft, services/ipfs, presentation/*): Each file creates its own localnew SdkLogger()instance.Important items for review
verifiableCredentialsSDK/: The per-fileSdkLoggerinstances (vcSdkLogger,litLogger,nftLogger,ipfsLogger,nftByLocatorLogger,contractMetadataLogger,collectionsLogger) are never.init()'d. Before init,SdkLoggerfalls back to rawconsole.*— meaning these ~20 calls won't actually be silenced byconsoleLogLevel. Reviewer should decide if these should instead import the sharedcredentialsLoggeror if a separate init path is needed.crossmintAPI.ts:initCredentialsLogger(apiKey)is called at line 106, butcredentialsLogger.warn(...)calls at lines 96 and 101 fire before initialization. These early warnings will use the uninitialized fallback.console.log→.info(): Allconsole.logcalls were mapped to.info()level, which is a minor semantic change.Link to Devin run | Requested by @xmint-guille
Test plan
pnpm lint)consoleLogLevel="silent"suppresses logs from this package after thecredentialsLoggeris initializedPackage updates
@crossmint/client-sdk-verifiable-credentials: patch (changeset added)