Refactor logger methods and update YRedisClient to use Logger for war…

…nings
hpi-schul-cloud · Dec 10, 2024 · bf396e0 · bf396e0
1 parent f7c01d2
commit bf396e0
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 25 deletions.
diff --git a/src/infra/logger/logger.spec.ts b/src/infra/logger/logger.spec.ts
@@ -55,7 +55,7 @@ describe('Logger', () => {
 	describe('WHEN warn logging', () => {
 		it('should call winstonLogger.warning', () => {
 			const error = new Error('custom error');
-			service.warn(error.message, error.stack);
+			service.warning(error.message, error.stack);
 			expect(winstonLogger.warning).toHaveBeenCalled();
 		});
 	});

diff --git a/src/infra/logger/logger.ts b/src/infra/logger/logger.ts
@@ -20,7 +20,7 @@ export class Logger {
 		this.logger.info(this.createMessage(message, context));
 	}
 
-	public warn(message: unknown, context?: string): void {
+	public warning(message: unknown, context?: string): void {
 		this.logger.warning(this.createMessage(message, context));
 	}
 

diff --git a/src/infra/storage/storage.service.ts b/src/infra/storage/storage.service.ts
@@ -1,6 +1,6 @@
 import { Injectable, OnModuleInit } from '@nestjs/common';
-import { Client } from 'minio';
 import { randomUUID } from 'crypto';
+import { Client } from 'minio';
 import { Stream } from 'stream';
 import * as Y from 'yjs';
 import { Logger } from '../logger/index.js';

diff --git a/src/infra/y-redis/y-redis.client.spec.ts b/src/infra/y-redis/y-redis.client.spec.ts
@@ -2,21 +2,23 @@ import { createMock } from '@golevelup/ts-jest';
 import * as Awareness from 'y-protocols/awareness';
 import * as Y from 'yjs';
 // import { RedisFactory } from '../redis/redis.service.js';
+import { Logger } from '../logger/logger.js';
 import { RedisAdapter } from '../redis/interfaces/index.js';
-import { YRedisClient, handleMessageUpdates } from './y-redis.client.js';
 import * as helper from './helper.js';
 import * as protocol from './protocol.js';
 import { DocumentStorage } from './storage.js';
 import { streamMessagesReplyFactory } from './testing/stream-messages-reply.factory.js';
 import { yRedisMessageFactory } from './testing/y-redis-message.factory.js';
+import { YRedisClient, handleMessageUpdates } from './y-redis.client.js';
 
 describe(YRedisClient.name, () => {
 	const setupApi = () => {
 		const store = createMock<DocumentStorage>();
 		const redis = createMock<RedisAdapter>({
 			redisPrefix: 'prefix',
 		});
-		const api = new YRedisClient(store, redis);
+		const logger = createMock<Logger>();
+		const api = new YRedisClient(store, redis, logger);
 
 		return { store, redis, api };
 	};

diff --git a/src/infra/y-redis/y-redis.client.ts b/src/infra/y-redis/y-redis.client.ts
@@ -2,6 +2,7 @@ import { OnModuleInit } from '@nestjs/common';
 import { array, decoding, promise } from 'lib0';
 import { applyAwarenessUpdate, Awareness } from 'y-protocols/awareness';
 import { applyUpdate, applyUpdateV2, Doc } from 'yjs';
+import { Logger } from '../logger/logger.js';
 import { MetricsService } from '../metrics/metrics.service.js';
 import { RedisAdapter, StreamNameClockPair } from '../redis/interfaces/index.js';
 import { computeRedisRoomStreamName, extractMessagesFromStreamReply } from './helper.js';
@@ -41,7 +42,9 @@ export class YRedisClient implements OnModuleInit {
 	public constructor(
 		private readonly store: DocumentStorage, // TODO: Naming?
 		public readonly redis: RedisAdapter,
+		private readonly logger: Logger,
 	) {
+		this.logger.setContext(YRedisClient.name);
 		this.store = store;
 		this.redisPrefix = redis.redisPrefix;
 		this._destroyed = false;
@@ -135,7 +138,7 @@ export class YRedisClient implements OnModuleInit {
 		};
 
 		if (ydoc.store.pendingStructs !== null) {
-			console.warn(`Document ${room} has pending structs ${JSON.stringify(ydoc.store.pendingStructs)}.`);
+			this.logger.warning(`Document ${room} has pending structs ${JSON.stringify(ydoc.store.pendingStructs)}.`);
 		}
 
 		return response;

diff --git a/src/infra/y-redis/y-redis.module.ts b/src/infra/y-redis/y-redis.module.ts
@@ -1,5 +1,7 @@
 import { DynamicModule, Module } from '@nestjs/common';
-import { RedisAdapter } from 'infra/redis/interfaces/redis-adapter.js';
+import { Logger } from '../logger/logger.js';
+import { LoggerModule } from '../logger/logger.module.js';
+import { RedisAdapter } from '../redis/interfaces/redis-adapter.js';
 import { RedisModule } from '../redis/redis.module.js';
 import { StorageModule } from '../storage/storage.module.js';
 import { StorageService } from '../storage/storage.service.js';
@@ -13,26 +15,31 @@ export class YRedisModule {
 	public static forRoot(): DynamicModule {
 		return {
 			module: YRedisModule,
-			imports: [RedisModule.registerFor(REDIS_FOR_SUBSCRIBER), RedisModule.registerFor(REDIS_FOR_API), StorageModule],
+			imports: [
+				RedisModule.registerFor(REDIS_FOR_SUBSCRIBER),
+				RedisModule.registerFor(REDIS_FOR_API),
+				StorageModule,
+				LoggerModule,
+			],
 			providers: [
 				YRedisService,
 				{
 					provide: YRedisClient,
-					useFactory: (redisAdapter: RedisAdapter, storageService: StorageService): YRedisClient => {
-						const yRedisClient = new YRedisClient(storageService, redisAdapter);
+					useFactory: (redisAdapter: RedisAdapter, storageService: StorageService, logger: Logger): YRedisClient => {
+						const yRedisClient = new YRedisClient(storageService, redisAdapter, logger);
 
 						return yRedisClient;
 					},
-					inject: [REDIS_FOR_API, StorageService],
+					inject: [REDIS_FOR_API, StorageService, Logger],
 				},
 				{
 					provide: API_FOR_SUBSCRIBER,
-					useFactory: (redisAdapter: RedisAdapter, storageService: StorageService): YRedisClient => {
-						const yRedisClient = new YRedisClient(storageService, redisAdapter);
+					useFactory: (redisAdapter: RedisAdapter, storageService: StorageService, logger: Logger): YRedisClient => {
+						const yRedisClient = new YRedisClient(storageService, redisAdapter, logger);
 
 						return yRedisClient;
 					},
-					inject: [REDIS_FOR_SUBSCRIBER, StorageService],
+					inject: [REDIS_FOR_SUBSCRIBER, StorageService, Logger],
 				},
 				{
 					provide: Subscriber,

diff --git a/src/modules/server/api/websocket.gateway.spec.ts b/src/modules/server/api/websocket.gateway.spec.ts
@@ -5,8 +5,8 @@ import { TemplatedApp } from 'uWebSockets.js';
 import { AuthorizationService } from '../../../infra/authorization/authorization.service.js';
 import { Logger } from '../../../infra/logger/logger.js';
 import { MetricsService } from '../../../infra/metrics/metrics.service.js';
-import { YRedisClient } from '../../../infra/y-redis/y-redis.client.js';
 import { Subscriber } from '../../../infra/y-redis/subscriber.service.js';
+import { YRedisClient } from '../../../infra/y-redis/y-redis.client.js';
 import { REDIS_FOR_SUBSCRIBE_OF_DELETION, UWS } from '../server.const.js';
 import { TldrawServerConfig } from '../tldraw-server.config.js';
 import { WebsocketGateway } from './websocket.gateway.js';

diff --git a/src/modules/server/api/websocket.gateway.ts b/src/modules/server/api/websocket.gateway.ts
@@ -13,14 +13,14 @@ import { Logger } from '../../../infra/logger/index.js';
 import { MetricsService } from '../../../infra/metrics/metrics.service.js';
 import { RedisAdapter } from '../../../infra/redis/interfaces/redis-adapter.js';
 import { computeRedisRoomStreamName, isSmallerRedisId } from '../../../infra/y-redis/helper.js';
+import { YRedisDoc } from '../../../infra/y-redis/interfaces/y-redis-doc.js';
 import { Subscriber } from '../../../infra/y-redis/subscriber.service.js';
+import { YRedisUserFactory } from '../../../infra/y-redis/y-redis-user.factory.js';
 import { YRedisUser } from '../../../infra/y-redis/y-redis-user.js';
 import { YRedisClient } from '../../../infra/y-redis/y-redis.client.js';
 import { YRedisService } from '../../../infra/y-redis/y-redis.service.js';
 import { REDIS_FOR_SUBSCRIBE_OF_DELETION, UWS } from '../server.const.js';
 import { TldrawServerConfig } from '../tldraw-server.config.js';
-import { YRedisDoc } from '../../../infra/y-redis/interfaces/y-redis-doc.js';
-import { YRedisUserFactory } from 'infra/y-redis/y-redis-user.factory.js';
 
 interface RequestHeaderInfos {
 	headerWsExtensions: string;
@@ -80,6 +80,8 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
 	private async upgradeCallback(res: HttpResponse, req: HttpRequest, context: us_socket_context_t): Promise<void> {
 		try {
 			let aborted = false;
+			const { headerWsKey, headerWsProtocol, headerWsExtensions } = this.extractHeaderInfos(req);
+
 			res.onAborted(() => {
 				aborted = true;
 			});
@@ -90,14 +92,13 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
 
 			res.cork(() => {
 				const yRedisUser = YRedisUserFactory.build(authPayload);
-				const { headerWsKey, headerWsProtocol, headerWsExtensions } = this.extractHeaderInfos(req);
 				res.upgrade(yRedisUser, headerWsKey, headerWsProtocol, headerWsExtensions, context);
 			});
 		} catch (error) {
 			res.cork(() => {
 				res.writeStatus('500 Internal Server Error').end('Internal Server Error');
 			});
-			console.error(error); // TODO: this.logger.error nur das interface erlaubt es gerade nicht
+			this.logger.warning(error); // TODO: this.logger.error nur das interface erlaubt es gerade nicht
 		}
 	}
 
@@ -156,7 +157,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
 				this.subscriberService.ensureSubId(stream, yRedisDoc.redisLastId);
 			}
 		} catch (error) {
-			console.error(error);
+			this.logger.warning(error);
 			ws.end(WebSocketErrorCodes.InternalError);
 		}
 	}
@@ -204,7 +205,7 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
 				this.yRedisClient.addMessage(user.room, 'index', message);
 			}
 		} catch (error) {
-			console.error(error);
+			this.logger.warning(error);
 			ws.end(WebSocketErrorCodes.InternalError);
 		}
 	}
@@ -223,8 +224,8 @@ export class WebsocketGateway implements OnModuleInit, OnModuleDestroy {
 
 			MetricsService.openConnectionsGauge.dec();
 		} catch (error) {
-			console.error(error);
-			// TODO: und jetzt wie räumen wir auf?
+			this.logger.warning(error);
+			ws.end(WebSocketErrorCodes.InternalError);
 		}
 	}
 

diff --git a/src/modules/worker/worker.service.spec.ts b/src/modules/worker/worker.service.spec.ts
@@ -9,11 +9,11 @@ import {
 	xAutoClaimResponseFactory,
 } from '../../infra/redis/testing/x-auto-claim-response.factory.js';
 import { StorageService } from '../../infra/storage/storage.service.js';
+import { YRedisDoc } from '../../infra/y-redis/interfaces/y-redis-doc.js';
 import { YRedisClient } from '../../infra/y-redis/y-redis.client.js';
 import { WorkerConfig } from './worker.config.js';
 import { REDIS_FOR_WORKER } from './worker.const.js';
 import { WorkerService } from './worker.service.js';
-import { YRedisDoc } from 'infra/y-redis/interfaces/y-redis-doc.js';
 
 const mapStreamMessageReplaysToTask = (streamMessageReplys: StreamMessageReply[]) => {
 	const tasks = streamMessageReplys.map((m) => ({
@@ -31,6 +31,7 @@ const createYRedisDocMock = (): YRedisDoc => {
 		redisLastId: '0',
 		storeReferences: null,
 		docChanged: true,
+		getAwarenessStateSize: () => 1,
 	};
 
 	return yDocMock;

diff --git a/src/modules/worker/worker.service.ts b/src/modules/worker/worker.service.ts
@@ -4,9 +4,9 @@ import { RedisKey } from 'ioredis';
 import { Logger } from '../../infra/logger/index.js';
 import { RedisAdapter, StreamMessageReply, Task, XAutoClaimResponse } from '../../infra/redis/interfaces/index.js';
 import { StorageService } from '../../infra/storage/storage.service.js';
-import { YRedisClient } from '../../infra/y-redis/y-redis.client.js';
 import { decodeRedisRoomStreamName, RoomStreamInfos } from '../../infra/y-redis/helper.js';
 import { YRedisDoc } from '../../infra/y-redis/interfaces/y-redis-doc.js';
+import { YRedisClient } from '../../infra/y-redis/y-redis.client.js';
 import { WorkerConfig } from './worker.config.js';
 import { REDIS_FOR_WORKER } from './worker.const.js';