Adding Custom Logger, Adding more tests for cov + minor fixes

[Faouzijedidi1]

User description

• Added Custom Logger
• Added Tests to increase cov to over %60
• Fixed Minor bugs

---

Type

enhancement, tests

---

Description

• Implemented CustomLogger extending NestJS Logger with file logging capabilities and integrated it across various modules.
• Added comprehensive tests for services including CoingeckoProxyService, MarketdataService, TradeService, and StrategyService.
• Made WebSocket port and server port configurable via environment variables.
• Added Jest configuration file and updated Jest scripts in package.json.
• Added winston package for logging.

---

Changes walkthrough

	Relevant files
Enhancement	14 files app.controller.ts Add New Get Route Handler in App Controller                            server/src/app.controller.ts Added a new @Get() route handler getHello to return the result of appService.getHello(). +4/-0      app.module.ts Integrate Custom Logger into AppModule                                      server/src/app.module.ts Imported LoggerModule and CustomLogger to the AppModule. Added LoggerModule to the imports array. Added CustomLogger to the providers array. +4/-1      subscriptionKey.ts Refactor Subscription Key Helper Functions                              server/src/common/helpers/subscriptionKey.ts Refactored decodeCompositeKey function to use a switch statement. Added array splitting for tickers type in decodeCompositeKey. +13/-11  main.ts Use Custom Logger and Make Port Configurable in Main          server/src/main.ts Integrated CustomLogger for logging incoming requests. Made the server port configurable via environment variable. +7/-3      health.service.ts Use CustomLogger in Health Service                                              server/src/modules/health/health.service.ts Replaced Logger with CustomLogger for logging. +2/-1      logger.module.ts Implement Logger Module                                                                    server/src/modules/logger/logger.module.ts Created a new LoggerModule with CustomLogger as a provider. +9/-0      logger.service.ts Implement Custom Logger Service                                                    server/src/modules/logger/logger.service.ts Implemented CustomLogger extending NestJS Logger with file logging capabilities. +69/-0    marketdata.gateway.ts Configure WebSocket Port and Use CustomLogger in MarketData Gateway server/src/modules/marketdata/marketdata.gateway.ts Made WebSocket port configurable via environment variable. Replaced Logger with CustomLogger. +4/-2      marketdata.service.ts Use CustomLogger in Marketdata Service                                      server/src/modules/marketdata/marketdata.service.ts Replaced Logger with CustomLogger for logging. +2/-1      strategy.module.ts Import LoggerModule into StrategyModule                                    server/src/modules/strategy/strategy.module.ts Imported LoggerModule into StrategyModule. +2/-1      strategy.service.ts Use CustomLogger and Add Logging for Stopping Strategy in StrategyService server/src/modules/strategy/strategy.service.ts Replaced Logger with CustomLogger for logging. Added logging for strategy stopping. +19/-8    trade.controller.ts Use CustomLogger in Trade Controller                                          server/src/modules/trade/trade.controller.ts Replaced Logger with CustomLogger for logging. +2/-1      trade.module.ts Export TradeRepository from TradeModule                                    server/src/modules/trade/trade.module.ts Exported TradeRepository from TradeModule. +1/-1      trade.service.ts Use CustomLogger in Trade Service                                                server/src/modules/trade/trade.service.ts Replaced Logger with CustomLogger for logging. +3/-2
Tests	8 files subscriptionKey.spec.ts Add Tests for Subscription Key Helpers                                      server/src/common/helpers/subscriptionKey.spec.ts Added tests for createCompositeKey and decodeCompositeKey functions. +77/-0    coingecko.service.spec.ts Add Tests for CoingeckoProxyService                                            server/src/modules/coingecko/coingecko.service.spec.ts Added tests for CoingeckoProxyService methods with mock implementations. +100/-3  health.service.spec.ts Add Health Service Tests                                                                  server/src/modules/health/health.service.spec.ts Added tests for HealthService including ping and getExchangeHealth methods. +27/-14  marketdata.service.spec.ts Add Tests for MarketdataService                                                    server/src/modules/marketdata/marketdata.service.spec.ts Added tests for MarketdataService methods with mock implementations. +91/-7    performance.controller.spec.ts Mock PerformanceService in PerformanceController Tests      server/src/modules/performance/performance.controller.spec.ts Added mock PerformanceService for testing PerformanceController. +12/-0    strategy.controller.spec.ts Mock StrategyService in StrategyController Tests                  server/src/modules/strategy/strategy.controller.spec.ts Added mock StrategyService for testing StrategyController. +13/-0    strategy.service.spec.ts Add Comprehensive Tests for StrategyService                            server/src/modules/strategy/strategy.service.spec.ts Added comprehensive tests for StrategyService including strategy execution and cancellation. +120/-1  trade.service.spec.ts Add Comprehensive Tests for TradeService                                  server/src/modules/trade/trade.service.spec.ts Added comprehensive tests for TradeService including limit and market trade execution. +109/-61
Configuration changes	2 files jest.config.js Add Jest Configuration File                                                            server/jest.config.js Added Jest configuration for specifying module file extensions, test environment, and test regex. +15/-0    jest-e2e.json Configure moduleNameMapper for Jest e2e Tests                        server/test/jest-e2e.json Added moduleNameMapper configuration for Jest e2e tests. +3/-0
Dependencies	1 files package.json Add Winston Package and Update Jest Scripts                            server/package.json Added winston package for logging. Updated Jest test scripts to use the Jest configuration file. +5/-4

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mr-market	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 5, 2024 2:32am
mr-market-back	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 5, 2024 2:32am

[github-actions]

PR Description updated to latest commit (b3f56a8)

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	4, due to the extensive changes across multiple files, including the addition of new modules, services, and tests. The complexity of integrating a custom logger and modifying existing services to use it, alongside the introduction of environment variable configurations for ports, requires careful consideration of the overall architecture and potential side effects.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: In server/src/modules/marketdata/marketdata.gateway.ts, the conversion of webSocketPort from string to number using parseInt without specifying a radix might lead to unexpected results. It's recommended to always specify a radix of 10 for decimal numbers.
	Performance Concern: The use of synchronous file operations in server/src/modules/logger/logger.service.ts could potentially block the event loop, especially under high load. Consider using asynchronous file operations instead.
	Error Handling: Several services and controllers lack comprehensive error handling, which could lead to unhandled exceptions and affect the stability of the application.
🔒 Security concerns	No

Code feedback:

---

relevant file	server/src/modules/marketdata/marketdata.gateway.ts
suggestion	Consider specifying a radix of 10 when using parseInt for converting webSocketPort to a number. This ensures the string is always interpreted in the decimal numeral system, avoiding potential bugs. [important]
relevant line	const webSocketPort = process.env.WS_PORT || "3012";

---

relevant file	server/src/modules/logger/logger.service.ts
suggestion	Replace synchronous file operations with their asynchronous counterparts to prevent blocking the event loop. This change enhances the performance and scalability of the application by ensuring that logging operations do not interfere with the main application flow. [important]
relevant line	fs.appendFile(filePath, message, (err) => {

---

relevant file	server/src/modules/trade/trade.service.ts
suggestion	Implement comprehensive error handling for external API calls and database operations. This could include try-catch blocks around API calls and checks for successful database operations, logging the errors and potentially retrying operations or gracefully degrading functionality. [important]
relevant line	service['exchanges'].get('binance').createOrder = jest.fn().mockResolvedValue(mockOrder);

---

relevant file	server/src/modules/strategy/strategy.service.ts
suggestion	Validate the existence and configuration of exchanges before attempting to use them in strategies. This can prevent InternalServerErrorException by ensuring that only configured exchanges are used, improving the robustness of the application. [medium]
relevant line	const exchangeA: ccxt.Exchange = this.exchanges.get(exchangeAName);

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

Utilizing extra instructions The review tool can be configured with extra instructions, which can be used to guide the model to a feedback tailored to the needs of your project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify the relevant sub-tool, and the relevant aspects of the PR that you want to emphasize. Examples for extra instructions: [pr_reviewer] # /review # extra_instructions=""" In the 'possible issues' section, emphasize the following: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

How to enable\disable automation When you first install PR-Agent app, the default mode for the review tool is: pr_commands = ["/review", ...] meaning the review tool will run automatically on every PR, with the default configuration. Edit this field to enable/disable the tool, or to change the used configurations

Auto-labels The review tool can auto-generate two specific types of labels for a PR: a possible security issue label, that detects possible security issues (enable_review_labels_security flag) a Review effort [1-5]: x label, where x is the estimated effort to review the PR (enable_review_labels_effort flag)

Extra sub-tools The review tool provides a collection of possible feedbacks about a PR. It is recommended to review the possible options, and choose the ones relevant for your use case. Some of the feature that are disabled by default are quite useful, and should be considered for enabling. For example: require_score_review, require_soc2_ticket, and more.

Auto-approve PRs By invoking: /review auto_approve The tool will automatically approve the PR, and add a comment with the approval. To ensure safety, the auto-approval feature is disabled by default. To enable auto-approval, you need to actively set in a pre-defined configuration file the following: [pr_reviewer] enable_auto_approval = true (this specific flag cannot be set with a command line argument, only in the configuration file, committed to the repository) You can also enable auto-approval only if the PR meets certain requirements, such as that the estimated_review_effort is equal or below a certain threshold, by adjusting the flag: [pr_reviewer] maximal_review_effort = 5

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Best practice	Use explicit route paths in HTTP method decorators. Consider using explicit HTTP method decorators like @Get('route') to define the route path, enhancing the readability and maintainability of your code. server/src/app.controller.ts [13-15] -@Get() +@Get('/hello') getHello(){ return this.appService.getHello(); }
	Use join(',') instead of toString() for arrays to ensure consistent comma-separated strings. To avoid potential issues with the toString() method on arrays, consider using join(',') for creating a comma-separated string from an array. This ensures consistent behavior across different environments. server/src/common/helpers/subscriptionKey.spec.ts [31] -const expectedKey = `tickers:binance:${symbols.toString()}`; +const expectedKey = `tickers:binance:${symbols.join(',')}`;
	Use a configuration service for managing application settings. For better scalability and environment management, consider using a configuration service or module to manage application settings like port numbers instead of directly accessing process.env. server/src/main.ts [32-33] -const port = process.env.PORT || 3000; +const port = this.configService.get('PORT') || 3000; await app.listen(port);
	Add type annotations to variables. Consider adding explicit type annotations to cacheManager to improve code readability and maintainability. server/src/modules/marketdata/marketdata.service.spec.ts [37] -let cacheManager; +let cacheManager: Cache;
	Use readonly for constants. Initialize cachingTTL with a readonly modifier to prevent accidental reassignment and clarify its constant nature. server/src/modules/marketdata/marketdata.service.ts [18] -private cachingTTL: 10; // 10s +private readonly cachingTTL: number = 10; // 10s
	Avoid using any type for better type safety. Avoid using any type for service['tradeService'] to ensure type safety. Define an interface or use an existing one if available. server/src/modules/strategy/strategy.service.ts [126] -service['tradeService'] = { executeLimitTrade: executeLimitTradeMock } as any; +service['tradeService'] = { executeLimitTrade: executeLimitTradeMock } as Partial<TradeService>;
	Add type annotations to the mockTradeRepository object. Consider adding explicit type annotations to the mockTradeRepository object to ensure type safety and improve code readability. server/src/modules/trade/trade.service.spec.ts [23-26] -mockTradeRepository = { +mockTradeRepository: TradeRepository = { createTrade: jest.fn(), updateTradeStatus: jest.fn(), };
	Add detailed logging before critical operations. For better error handling and debugging, consider adding more detailed logging, especially before performing critical operations like executing trades. server/src/modules/trade/trade.service.ts [69] -this.logger.log(`Market trade executed`, order.toString()); +this.logger.log(`Attempting to execute market trade with order details: ${JSON.stringify(order)}`); +// Followed by the execution
	Mock external dependencies more comprehensively in tests. To improve the robustness of your tests, consider mocking external dependencies more comprehensively. For example, mock the ccxt library's behavior more thoroughly to simulate different trading scenarios and responses. server/src/modules/trade/trade.service.spec.ts [7-15] jest.mock('ccxt', () => ({ pro: { binance: jest.fn().mockImplementation(() => ({ - createOrder: jest.fn(), - cancelOrder: jest.fn().mockResolvedValue, + createOrder: jest.fn().mockImplementation((symbol, type, side, amount, price) => { + // Simulate different responses based on input + }), + cancelOrder: jest.fn().mockResolvedValue(/* Simulate realistic cancellation response */), })), - bitfinex: jest.fn().mockImplementation(() => ({})), - mexc: jest.fn().mockImplementation(() => ({})), + bitfinex: jest.fn().mockImplementation(() => ({ + // Implement mocking for bitfinex + })), + mexc: jest.fn().mockImplementation(() => ({ + // Implement mocking for mexc + })), }, }));
Maintainability	Organize imports by separating external and internal modules. It's a good practice to organize imports by external and internal modules, separated by a newline, to improve code readability. server/src/app.module.ts [23-25] import { LoggerModule } from './modules/logger/logger.module'; import { CustomLogger } from './modules/logger/logger.service'; + dotenv.config();
Enhancement	Implement log rotation to manage log file sizes. To ensure that log files are not excessively large, consider implementing a log rotation mechanism or using a third-party library that supports log rotation out of the box. server/src/modules/logger/logger.service.ts [31-37] +// Example using a third-party library for log rotation private writeToFile(filePath: string, message: string): void { - fs.appendFile(filePath, message, (err) => { - if (err) { - console.error('Failed to write log:', err); - } - }); + // Implement log rotation mechanism here }
	Simplify function parameters with destructuring. Use destructuring in function parameters to simplify the code and improve readability. server/src/modules/strategy/strategy.service.ts [78] -async startArbitrageStrategyForUser(strategyParamsDto: ArbitrageStrategyDto) { +async startArbitrageStrategyForUser({ userId, clientId, pair, exchangeAName, exchangeBName }: ArbitrageStrategyDto) {
	Use a mapping object for conditional assignments. Replace the if statements with a switch case or a mapping object for strategyType to enhance code readability and maintainability. server/src/modules/strategy/strategy.service.ts [132-137] -if (strategyType === 'Arbitrage') { -if (strategyType === 'pureMarketMaking') { +const strategyKeys = { + Arbitrage: `${userId}-${clientId}-Arbitrage`, + pureMarketMaking: `${userId}-${clientId}-pureMarketMaking`, +}; +strategyKey = strategyKeys[strategyType];
	Verify mock method calls with expected arguments in tests. To improve test reliability, consider verifying that mockTradeRepository.createTrade and mockTradeRepository.updateTradeStatus are called with the expected arguments in your test cases. server/src/modules/trade/trade.service.spec.ts [70-80] -expect(mockTradeRepository.createTrade).toHaveBeenCalledWith({ +expect(mockTradeRepository.createTrade).toHaveBeenCalledWith(expect.objectContaining({ userId: 'user123', clientId: 'client123', symbol: 'BTC/USD', side: 'sell', type: 'limit', amount: 1, price: 50000, status: 'closed', orderId: 'order123', -}); +}));
	Add tests for partial failures or exceptions during trade execution. To ensure that your tests cover all possible scenarios, consider adding tests for partial failures or exceptions during the trade execution process, such as network issues or partial order fulfillment. server/src/modules/trade/trade.service.spec.ts [107-120] -it('should throw InternalServerErrorException on createOrder failure', async () => { - const limitTradeDto = { - userId: 'user123', - clientId: 'client123', - exchange: 'binance', - symbol: 'BTC/USD', - side: 'sell', - amount: 1, - price: 50000, - }; - - ccxt.pro.binance.prototype.createOrder = jest.fn().mockRejectedValue(new Error('API Error')); - - await expect(service.executeLimitTrade(limitTradeDto)).rejects.toThrow(InternalServerErrorException); +// Example of an additional test case +it('should handle partial order fulfillment', async () => { + // Setup partial fulfillment scenario + // Assert the expected behavior });

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.
When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

Enabling\disabling automation When you first install the app, the default mode for the improve tool is: pr_commands = ["/improve --pr_code_suggestions.summarize=true", ...] meaning the improve tool will run automatically on every PR, with summarization enabled. Delete this line to disable the tool from running automatically.

Utilizing extra instructions Extra instructions are very important for the improve tool, since they enable to guide the model to suggestions that are more relevant to the specific needs of the project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify relevant aspects that you want the model to focus on. Examples for extra instructions: [pr_code_suggestions] # /improve # extra_instructions=""" Emphasize the following aspects: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

A note on code suggestions quality While the current AI for code is getting better and better (GPT-4), it's not flawless. Not all the suggestions will be perfect, and a user should not accept all of them automatically. Suggestions are not meant to be simplistic. Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base. Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project, or use the custom suggestions 💎 tool With large PRs, best quality will be obtained by using 'improve --extended' mode.

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the improve usage page for a more comprehensive guide on using this tool.