-
Notifications
You must be signed in to change notification settings - Fork 7
Added required changes for log messages #18
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.
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
bc496d0
9f8a4e1
69ceacc
a54c2d3
5e89b4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ const sdkVersion = require('../package.json').version; | |
|
||
export async function verifyProof(proof: Proof): Promise<boolean> { | ||
if (!proof.signatures.length) { | ||
throw new SignatureNotFoundError('No signatures') | ||
throw new SignatureNotFoundError('No signatures, proof object is undefined') | ||
} | ||
|
||
try { | ||
|
@@ -59,7 +59,7 @@ export async function verifyProof(proof: Proof): Promise<boolean> { | |
proof.claimData.timestampS | ||
) | ||
} | ||
// then hash the claim info with the encoded ctx to get the identifier | ||
// then hash the claim error with the encoded ctx to get the identifier | ||
const calculatedIdentifier = getIdentifierFromClaimInfo({ | ||
parameters: JSON.parse( | ||
canonicalize(proof.claimData.parameters) as string | ||
|
@@ -70,7 +70,7 @@ export async function verifyProof(proof: Proof): Promise<boolean> { | |
proof.identifier = replaceAll(proof.identifier, '"', '') | ||
// check if the identifier matches the one in the proof | ||
if (calculatedIdentifier !== proof.identifier) { | ||
throw new ProofNotVerifiedError('Identifier Mismatch') | ||
throw new ProofNotVerifiedError(`Identifier Mismatch, ${calculatedIdentifier} is not as ${proof.identifier}`) | ||
} | ||
|
||
const signedClaim: SignedClaim = { | ||
|
@@ -84,20 +84,20 @@ export async function verifyProof(proof: Proof): Promise<boolean> { | |
|
||
assertValidSignedClaim(signedClaim, witnesses) | ||
} catch (e: Error | unknown) { | ||
logger.info(`Error verifying proof: ${e instanceof Error ? e.message : String(e)}`) | ||
logger.error(`Error verifying proof: ${e instanceof Error ? e.message : String(e)}`) | ||
return false | ||
} | ||
|
||
return true | ||
} | ||
|
||
export function transformForOnchain(proof: Proof): { claimInfo: any, signedClaim: any } { | ||
const claimInfoBuilder = new Map([ | ||
export function transformForOnchain(proof: Proof): { claimerror: any, signedClaim: any } { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The parameter name should not be updated here. Please make sure you are not find & replacing all |
||
const claimerrorBuilder = new Map([ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be changes |
||
['context', proof.claimData.context], | ||
['parameters', proof.claimData.parameters], | ||
['provider', proof.claimData.provider], | ||
]); | ||
const claimInfo = Object.fromEntries(claimInfoBuilder); | ||
const claimerror = Object.fromEntries(claimerrorBuilder); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary change. Please revert |
||
const claimBuilder = new Map<string, number | string>([ | ||
['epoch', proof.claimData.epoch], | ||
['identifier', proof.claimData.identifier], | ||
|
@@ -108,7 +108,7 @@ export function transformForOnchain(proof: Proof): { claimInfo: any, signedClaim | |
claim: Object.fromEntries(claimBuilder), | ||
signatures: proof.signatures, | ||
}; | ||
return { claimInfo, signedClaim }; | ||
return { claimerror, signedClaim }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary change. Please revert |
||
} | ||
|
||
export class ReclaimProofRequest { | ||
|
@@ -132,10 +132,10 @@ export class ReclaimProofRequest { | |
this.timeStamp = Date.now().toString(); | ||
this.applicationId = applicationId; | ||
this.sessionId = ""; | ||
if (options?.log) { | ||
loggerModule.setLogLevel('info'); | ||
if (options?.log && options?.logLevel) { | ||
loggerModule.setLogLevel(options?.logLevel!); | ||
} else { | ||
loggerModule.setLogLevel('silent'); | ||
loggerModule.setLogLevel('all') | ||
} | ||
this.options = options; | ||
// Fetch sdk version from package.json | ||
|
@@ -161,8 +161,13 @@ export class ReclaimProofRequest { | |
} | ||
if (options.log) { | ||
validateFunctionParams([ | ||
{ paramName: 'log', input: options.log } | ||
{ paramName: 'log', input: options.log }, | ||
], 'the constructor') | ||
|
||
if(options.logLevel){ | ||
validateFunctionParams([{ paramName: 'logLevel', input: options.logLevel } | ||
], 'the constructor') | ||
} | ||
} | ||
|
||
} | ||
|
@@ -179,7 +184,8 @@ export class ReclaimProofRequest { | |
|
||
return proofRequestInstance | ||
} catch (error) { | ||
logger.info('Failed to initialize ReclaimProofRequest', error as Error); | ||
logger.error('Failed to initialize ReclaimProofRequest', error as Error); | ||
logger.info("Try changing values of applicationId, applicationSecret, ProviderId, Option's Parameters") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This info log should be not be in the catch block. Better to move it at top inside the try block |
||
throw new InitError('Failed to initialize ReclaimProofRequest', error as Error) | ||
} | ||
} | ||
|
@@ -234,7 +240,7 @@ export class ReclaimProofRequest { | |
proofRequestInstance.sdkVersion = sdkVersion; | ||
return proofRequestInstance | ||
} catch (error) { | ||
logger.info('Failed to parse JSON string in fromJsonString:', error); | ||
logger.error('Failed to parse JSON string in fromJsonString:', error); | ||
throw new InvalidParamError('Invalid JSON string provided to fromJsonString'); | ||
} | ||
} | ||
|
@@ -258,7 +264,8 @@ export class ReclaimProofRequest { | |
], 'addContext'); | ||
this.context = { contextAddress: address, contextMessage: message }; | ||
} catch (error) { | ||
logger.info("Error adding context", error) | ||
logger.error("Error adding context", error) | ||
logger.info(`contextId: A unique identifier for the context (hex address) \nContext message: Additional information about the proof request (string)`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This info log should be not be in the catch block. Better to move it at top inside the try block |
||
throw new AddContextError("Error adding context", error as Error) | ||
} | ||
} | ||
|
@@ -285,7 +292,8 @@ export class ReclaimProofRequest { | |
} | ||
this.requestedProof.parameters = { ...requestedProof.parameters, ...params } | ||
} catch (error) { | ||
logger.info('Error Setting Params:', error); | ||
logger.error('Error Setting Params:', error); | ||
logger.info(`Arguments passed in setParams must be object which contains key => value pairs`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This info log should be not be in the catch block. Better to move it at top inside the try block |
||
throw new SetParamsError("Error setting params", error as Error) | ||
} | ||
} | ||
|
@@ -296,7 +304,8 @@ export class ReclaimProofRequest { | |
validateFunctionParams([{ input: this.sessionId, paramName: 'sessionId', isString: true }], 'getAppCallbackUrl'); | ||
return this.appCallbackUrl || `${constants.DEFAULT_RECLAIM_CALLBACK_URL}${this.sessionId}` | ||
} catch (error) { | ||
logger.info("Error getting app callback url", error) | ||
logger.error("Error getting app callback url", error) | ||
logger.info(`Make sure to pass URL and as a string`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This info log should be not be in the catch block. Better to move it at top inside the try block |
||
throw new GetAppCallbackUrlError("Error getting app callback url", error as Error) | ||
} | ||
} | ||
|
@@ -306,7 +315,8 @@ export class ReclaimProofRequest { | |
validateFunctionParams([{ input: this.sessionId, paramName: 'sessionId', isString: true }], 'getStatusUrl'); | ||
return `${constants.DEFAULT_RECLAIM_STATUS_URL}${this.sessionId}` | ||
} catch (error) { | ||
logger.info("Error fetching Status Url", error) | ||
logger.error("Error fetching Status Url", error) | ||
logger.info(`Make sure to pass URL and as a string`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This info log should be not be in the catch block. Better to move it at top inside the try block |
||
throw new GetStatusUrlError("Error fetching status url", error as Error) | ||
} | ||
} | ||
|
@@ -318,7 +328,7 @@ export class ReclaimProofRequest { | |
this.signature = signature; | ||
logger.info(`Signature set successfully for applicationId: ${this.applicationId}`); | ||
} catch (error) { | ||
logger.info("Error setting signature", error) | ||
logger.error("Error setting signature", error) | ||
throw new SetSignatureError("Error setting signature", error as Error) | ||
} | ||
} | ||
|
@@ -337,7 +347,7 @@ export class ReclaimProofRequest { | |
|
||
return await wallet.signMessage(ethers.getBytes(messageHash)); | ||
} catch (err) { | ||
logger.info(`Error generating proof request for applicationId: ${this.applicationId}, providerId: ${this.providerId}, signature: ${this.signature}, timeStamp: ${this.timeStamp}`, err); | ||
logger.error(`Error generating proof request for applicationId: ${this.applicationId}, providerId: ${this.providerId}, signature: ${this.signature}, timeStamp: ${this.timeStamp}`, err); | ||
throw new SignatureGeneratingError(`Error generating signature for applicationSecret: ${applicationSecret}`) | ||
} | ||
} | ||
|
@@ -347,7 +357,7 @@ export class ReclaimProofRequest { | |
this.requestedProof = generateRequestedProof(provider); | ||
return this.requestedProof; | ||
} catch (err: Error | unknown) { | ||
logger.info(err instanceof Error ? err.message : String(err)); | ||
logger.error(err instanceof Error ? err.message : String(err)); | ||
throw new BuildProofRequestError('Something went wrong while generating proof request', err as Error); | ||
} | ||
} | ||
|
@@ -370,7 +380,7 @@ export class ReclaimProofRequest { | |
return [...new Set(availableParamsStore)]; | ||
|
||
} catch (error) { | ||
logger.info("Error fetching available params", error) | ||
logger.error("Error fetching available params", error) | ||
throw new AvailableParamsError("Error fetching available params", error as Error) | ||
} | ||
} | ||
|
@@ -428,16 +438,16 @@ export class ReclaimProofRequest { | |
await updateSession(this.sessionId, SessionStatus.SESSION_STARTED) | ||
return link | ||
} catch (error) { | ||
logger.info('Error creating Request Url:', error) | ||
logger.error('Error creating Request Url:', error) | ||
throw error | ||
} | ||
} | ||
|
||
async startSession({ onSuccess, onError }: StartSessionParams): Promise<void> { | ||
if (!this.sessionId) { | ||
const message = "Session can't be started due to undefined value of sessionId"; | ||
logger.info(message); | ||
throw new SessionNotStartedError(message); | ||
const message = "Session can't be started due to undefined value of statusUrl and sessionId" | ||
logger.warn(message) | ||
throw new SessionNotStartedError(message) | ||
} | ||
|
||
logger.info('Starting session'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,22 @@ | ||
// Define the possible log levels | ||
export type LogLevel = 'info' | 'warn' | 'error' | 'silent'; | ||
export type LogLevel = 'info' | 'warn' | 'error' ; | ||
export type ExtendedLog = LogLevel | 'all' | ||
|
||
// Define a simple logger class | ||
class SimpleLogger { | ||
private level: LogLevel = 'info'; | ||
private level: ExtendedLog = 'info'; | ||
|
||
setLevel(level: LogLevel) { | ||
setLevel(level: LogLevel | 'all') { | ||
this.level = level; | ||
} | ||
|
||
private shouldLog(messageLevel: LogLevel): boolean { | ||
const levels: LogLevel[] = ['error', 'warn', 'info', 'silent']; | ||
const levels: ExtendedLog[] = ['error', 'warn', 'info', 'all']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this log all there types of log 'error', 'warn', 'info' when logLevel is set to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didnt get your question |
||
return levels.indexOf(this.level) >= levels.indexOf(messageLevel); | ||
} | ||
|
||
private log(level: LogLevel, message: string, ...args: any[]) { | ||
if (this.shouldLog(level) && this.level !== 'silent') { | ||
if (this.shouldLog(level)) { | ||
const logFunction = this.getLogFunction(level); | ||
console.log('current level', this.level); | ||
logFunction(`[${level.toUpperCase()}]`, message, ...args); | ||
|
@@ -31,7 +32,11 @@ class SimpleLogger { | |
case 'info': | ||
return console.info; | ||
default: | ||
return () => {}; // No-op for 'silent' | ||
return (message: string, ...optionalParams: any[]) => { | ||
console.info('info',message, ...optionalParams); | ||
console.warn('warn',message, ...optionalParams); | ||
console.error('error',message, ...optionalParams); | ||
}; | ||
} | ||
} | ||
|
||
|
@@ -52,7 +57,7 @@ class SimpleLogger { | |
const logger = new SimpleLogger(); | ||
|
||
// Function to set the log level | ||
export function setLogLevel(level: LogLevel) { | ||
export function setLogLevel(level: LogLevel | 'all') { | ||
logger.setLevel(level); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't change the comments details. This is not related to logging and serves a different purpose