Skip to content

Commit

Permalink
refactor: revamp logger and set logger level from snap build (#342)
Browse files Browse the repository at this point in the history
* chore: update logger to deine from build

* chore: update default log level config

* chore: update logger test

* chore: fix test
  • Loading branch information
stanleyyconsensys authored Sep 3, 2024
1 parent 8f50a33 commit 98c0224
Show file tree
Hide file tree
Showing 20 changed files with 206 additions and 212 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,21 @@ jobs:
echo "VERSION=${BASE}-dev-${HASH}-${DATE}"
echo "TAG=dev"
echo "ENV=dev"
echo "LOG_LEVEL=all"
echo "LOG_LEVEL=6"
} >> "$GITHUB_OUTPUT"
elif [[ $ENV == "staging" ]]; then
{
echo "VERSION=${BASE}-staging"
echo "TAG=staging"
echo "ENV=staging"
echo "LOG_LEVEL=off"
echo "LOG_LEVEL=0"
} >> "$GITHUB_OUTPUT"
elif [[ $ENV == "production" ]]; then
{
echo "VERSION=${BASE}"
echo "TAG=latest"
echo "ENV=prod"
echo "LOG_LEVEL=off"
echo "LOG_LEVEL=0"
} >> "$GITHUB_OUTPUT"
else
echo "Invalid environment"
Expand Down Expand Up @@ -107,7 +107,7 @@ jobs:
echo "Building UI with version $VERSION"
REACT_APP_DEBUG_LEVEL="${LOG_LEVEL}" REACT_APP_SNAP_VERSION="${VERSION}" yarn workspace wallet-ui build
REACT_APP_SNAP_VERSION="${VERSION}" yarn workspace wallet-ui build
echo "Building Get Starknet with GET_STARKNET_PUBLIC_PATH=$GET_STARKNET_PUBLIC_PATH"
Expand Down
7 changes: 6 additions & 1 deletion packages/starknet-snap/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ SNAP_ENV=dev
VOYAGER_API_KEY=
# Description: Environment variables for API key of ALCHEMY
# Required: false
ALCHEMY_API_KEY=
ALCHEMY_API_KEY=
# Description: Environment variables for Log Level, 0 does not log anything, 6 logs everything
# Possible Options: 0-6
# Default: 0
# Required: false
LOG_LEVEL=6
4 changes: 2 additions & 2 deletions packages/starknet-snap/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
"serve": "mm-snap serve",
"start": "mm-snap watch",
"test": "yarn run test:unit && yarn run cover:report && yarn run jest",
"test:unit": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register \"test/**/*.test.ts\"",
"test:unit:one": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register"
"test:unit": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register --require test/global.ts \"test/**/*.test.ts\"",
"test:unit:one": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register --require test/global.ts"
},
"nyc": {
"exclude": [
Expand Down
1 change: 1 addition & 0 deletions packages/starknet-snap/snap.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const config: SnapConfig = {
SNAP_ENV: process.env.SNAP_ENV ?? 'prod',
VOYAGER_API_KEY: process.env.VOYAGER_API_KEY ?? '',
ALCHEMY_API_KEY: process.env.ALCHEMY_API_KEY ?? '',
LOG_LEVEL: process.env.LOG_LEVEL ?? '0',
/* eslint-disable */
},
polyfills: true,
Expand Down
10 changes: 10 additions & 0 deletions packages/starknet-snap/src/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { LogLevel } from './utils';

export type SnapConfig = {
logLevel: string;
};

export const Config: SnapConfig = {
// eslint-disable-next-line no-restricted-globals
logLevel: process.env.LOG_LEVEL ?? LogLevel.OFF.valueOf().toString(),
};
37 changes: 3 additions & 34 deletions packages/starknet-snap/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
STARKNET_SEPOLIA_TESTNET_NETWORK,
} from './utils/constants';
import * as keyPairUtils from './utils/keyPair';
import { LogLevel, logger } from './utils/logger';
import * as starknetUtils from './utils/starknetUtils';

jest.mock('./utils/logger');
Expand All @@ -21,11 +20,9 @@ describe('onRpcRequest', () => {
const createMockSpy = () => {
const createAccountSpy = jest.spyOn(createAccountApi, 'createAccount');
const keyPairSpy = jest.spyOn(keyPairUtils, 'getAddressKeyDeriver');
const getLogLevelSpy = jest.spyOn(logger, 'getLogLevel');
return {
createAccountSpy,
keyPairSpy,
getLogLevelSpy,
};
};

Expand All @@ -42,55 +39,27 @@ describe('onRpcRequest', () => {
};

it('processes request successfully', async () => {
const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy();
const { createAccountSpy, keyPairSpy } = createMockSpy();

createAccountSpy.mockReturnThis();
keyPairSpy.mockReturnThis();
getLogLevelSpy.mockReturnValue(LogLevel.OFF);

await onRpcRequest(createMockRequest());

expect(keyPairSpy).toHaveBeenCalledTimes(1);
expect(createAccountSpy).toHaveBeenCalledTimes(1);
});

it('throws `Unable to execute the rpc request` error if an error has thrown and LogLevel is 0', async () => {
const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy();
it('throws `Unable to execute the rpc request` error if an error has thrown', async () => {
const { createAccountSpy, keyPairSpy } = createMockSpy();

createAccountSpy.mockRejectedValue(new Error('Custom Error'));
keyPairSpy.mockReturnThis();
getLogLevelSpy.mockReturnValue(LogLevel.OFF);

await expect(onRpcRequest(createMockRequest())).rejects.toThrow(
'Unable to execute the rpc request',
);
});

it.each([
LogLevel.DEBUG,
LogLevel.ALL,
LogLevel.ERROR,
LogLevel.INFO,
LogLevel.TRACE,
LogLevel.WARN,
])(
`throws 'Unable to execute the rpc request' error if an error has thrown and LogLevel is %s`,
async function (logLevel) {
const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy();

createAccountSpy.mockRejectedValue(new Error('Custom Error'));
keyPairSpy.mockReturnThis();
getLogLevelSpy.mockReturnValue(logLevel);

await expect(
onRpcRequest(
createMockRequest({
debugLevel: LogLevel[logLevel],
}),
),
).rejects.toThrow('Custom Error');
},
);
});

describe('onHomePage', () => {
Expand Down
9 changes: 4 additions & 5 deletions packages/starknet-snap/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { ethers } from 'ethers';

import { addErc20Token } from './addErc20Token';
import { addNetwork } from './addNetwork';
import { Config } from './config';
import { createAccount } from './createAccount';
import { declareContract } from './declareContract';
import { estimateAccDeployFee } from './estimateAccountDeployFee';
Expand Down Expand Up @@ -90,10 +91,8 @@ declare const snap;

export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
const requestParams = request?.params as unknown as ApiRequestParams;
const debugLevel = requestParams?.debugLevel;
logger.init(debugLevel as unknown as string);
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
console.log(`debugLevel: ${logger.getLogLevel()}`);

logger.logLevel = parseInt(Config.logLevel, 10);

logger.log(`${request.method}:\nrequestParams: ${toJson(requestParams)}`);

Expand Down Expand Up @@ -300,7 +299,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
logger.error(`Error: ${error}`);
// We don't want to expose the error message to the user when it is a production build
if (logger.getLogLevel() === LogLevel.OFF) {
if (logger.logLevel === LogLevel.OFF) {
throw new SnapError(
'Unable to execute the rpc request',
) as unknown as Error;
Expand Down
2 changes: 2 additions & 0 deletions packages/starknet-snap/src/rpcs/displayPrivateKey.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
import { displayPrivateKey } from './displayPrivateKey';
import type { DisplayPrivateKeyParams } from './displayPrivateKey';

jest.mock('../utils/logger');

describe('displayPrivateKey', () => {
const state: SnapState = {
accContracts: [],
Expand Down
2 changes: 2 additions & 0 deletions packages/starknet-snap/src/rpcs/signTransaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
import { signTransaction } from './signTransaction';
import type { SignTransactionParams } from './signTransaction';

jest.mock('../utils/logger');

describe('signTransaction', () => {
const state: SnapState = {
accContracts: [],
Expand Down
1 change: 0 additions & 1 deletion packages/starknet-snap/src/types/snapApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export type ApiRequestParams =

export type BaseRequestParams = {
chainId?: string;
debugLevel?: string;
};

export type TransactionVersionParams = {
Expand Down
146 changes: 146 additions & 0 deletions packages/starknet-snap/src/utils/logger.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { logger, LogLevel } from './logger';

describe('Logger', () => {
afterAll(() => {
logger.logLevel = LogLevel.OFF;
});
const createLogSpy = () => {
return {
log: jest.spyOn(console, 'log').mockReturnThis(),
error: jest.spyOn(console, 'error').mockReturnThis(),
warn: jest.spyOn(console, 'warn').mockReturnThis(),
info: jest.spyOn(console, 'info').mockReturnThis(),
trace: jest.spyOn(console, 'trace').mockReturnThis(),
debug: jest.spyOn(console, 'debug').mockReturnThis(),
};
};

const testLog = (message: string) => {
logger.log(message);
logger.error(message);
logger.warn(message);
logger.info(message);
logger.trace(message);
logger.debug(message);
};

it.each([
{
logLevel: LogLevel.ALL,
expected: {
info: true,
warn: true,
error: true,
debug: true,
log: true,
trace: true,
},
},
{
logLevel: LogLevel.OFF,
expected: {
info: false,
warn: false,
error: false,
debug: false,
log: false,
trace: false,
},
},
{
logLevel: LogLevel.INFO,
expected: {
info: true,
warn: true,
error: true,
debug: false,
log: false,
trace: false,
},
},
{
logLevel: LogLevel.ERROR,
expected: {
info: false,
warn: false,
error: true,
debug: false,
log: false,
trace: false,
},
},
{
logLevel: LogLevel.WARN,
expected: {
info: false,
warn: true,
error: true,
debug: false,
log: false,
trace: false,
},
},
{
logLevel: LogLevel.DEBUG,
expected: {
info: true,
warn: true,
error: true,
debug: true,
log: false,
trace: false,
},
},
{
logLevel: LogLevel.TRACE,
expected: {
info: true,
warn: true,
error: true,
debug: true,
log: false,
trace: true,
},
},
])(
'logs correctly when `logLevel` is `$logLevel`',
({
logLevel,
expected,
}: {
logLevel: LogLevel;
expected: {
info: boolean;
warn: boolean;
error: boolean;
debug: boolean;
log: boolean;
trace: boolean;
};
}) => {
const spys = createLogSpy();
logger.logLevel = logLevel;
const logMsg = 'log';

testLog(logMsg);

Object.entries(expected)
.filter(([_, value]) => !value)
.forEach(([key]) => {
expect(spys[key]).not.toHaveBeenCalled();
});

Object.entries(expected)
.filter(([_, value]) => value)
.forEach(([key]) => {
expect(spys[key]).toHaveBeenCalledWith(logMsg);
});
},
);

it('return correct `LogLevel`', () => {
logger.logLevel = LogLevel.INFO;

expect(logger.logLevel).toStrictEqual(LogLevel.INFO);
});
});
Loading

0 comments on commit 98c0224

Please sign in to comment.