From e281b9e0d977117a2e861c6325d08a2aafc7be82 Mon Sep 17 00:00:00 2001 From: jahabeebs <47253537+jahabeebs@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:27:43 -0500 Subject: [PATCH] fix: pr comments --- apps/agent/src/index.ts | 2 +- .../src/services/discordNotifier.ts | 12 ++++++---- .../src/services/eboProcessor.ts | 18 +++++++------- .../tests/services/discordNotifier.spec.ts | 24 +++++++------------ 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/apps/agent/src/index.ts b/apps/agent/src/index.ts index f1fcea6..a39c857 100644 --- a/apps/agent/src/index.ts +++ b/apps/agent/src/index.ts @@ -41,7 +41,7 @@ const main = async (): Promise => { logger.debug("Initializing notifier..."); - const notifier = new DiscordNotifier(config.DISCORD_WEBHOOK); + const notifier = new DiscordNotifier(config.DISCORD_WEBHOOK, logger); logger.debug("Notifier initialized..."); diff --git a/packages/automated-dispute/src/services/discordNotifier.ts b/packages/automated-dispute/src/services/discordNotifier.ts index b2f257f..9872e34 100644 --- a/packages/automated-dispute/src/services/discordNotifier.ts +++ b/packages/automated-dispute/src/services/discordNotifier.ts @@ -1,6 +1,6 @@ import { isNativeError } from "util/types"; import type { APIEmbed, JSONEncodable } from "discord.js"; -import { Logger, stringify } from "@ebo-agent/shared"; +import { ILogger, stringify } from "@ebo-agent/shared"; import { WebhookClient, WebhookMessageCreateOptions } from "discord.js"; import { NotificationFailureException } from "../exceptions/index.js"; @@ -15,19 +15,21 @@ export type WebhookMessage = WebhookMessageCreateOptions & { */ export class DiscordNotifier implements NotificationService { private client: WebhookClient; - private defaultAvatarUrl?: string; - private logger: Logger; /** * Creates an instance of DiscordNotifier. * * @param {string} url - The Discord webhook URL. + * @param {ILogger} logger - An injected logger instance implementing ILogger. * @param {string} [defaultAvatarUrl] - The default avatar URL to use if none is provided. */ - constructor(url: string, defaultAvatarUrl?: string) { + constructor( + url: string, + private readonly logger: ILogger, + private readonly defaultAvatarUrl?: string, + ) { this.client = new WebhookClient({ url }); this.defaultAvatarUrl = defaultAvatarUrl; - this.logger = Logger.getInstance(); } /** diff --git a/packages/automated-dispute/src/services/eboProcessor.ts b/packages/automated-dispute/src/services/eboProcessor.ts index 506bc68..78c608e 100644 --- a/packages/automated-dispute/src/services/eboProcessor.ts +++ b/packages/automated-dispute/src/services/eboProcessor.ts @@ -259,7 +259,7 @@ export class EboProcessor { lastBlock: Block, ) { const firstEvent = events[0]; - const actor = this.getOrCreateActor(requestId, firstEvent); + const actor = await this.getOrCreateActor(requestId, firstEvent); if (!actor) { this.logger.warn(droppingUnhandledEventsWarning(requestId)); @@ -299,7 +299,7 @@ export class EboProcessor { * @param firstEvent an event to create an actor if it does not exist * @returns the actor handling the specified request */ - private getOrCreateActor(requestId: RequestId, firstEvent?: EboEvent) { + private async getOrCreateActor(requestId: RequestId, firstEvent?: EboEvent) { const actor = this.actorsManager.getActor(requestId); if (actor) return actor; @@ -322,7 +322,7 @@ export class EboProcessor { } else { this.logger.warn(`Chain ${chainId} not supported by the agent. Skipping...`); - this.notifier.sendError( + await this.notifier.sendError( `Chain ${chainId} not supported by the agent. Skipping...`, { chainId, requestId }, new Error("Unsupported chain"), @@ -354,14 +354,14 @@ export class EboProcessor { return actor; } - private onActorError(requestId: RequestId, error: Error) { + private async onActorError(requestId: RequestId, error: Error) { this.logger.error( `Critical error. Actor event handling request ${requestId} ` + `threw a non-recoverable error: ${error.message}\n\n` + `The request ${requestId} will stop being tracked by the system.`, ); - this.notifier.sendError(`Actor error for request ${requestId}`, { requestId }, error); + await this.notifier.sendError(`Actor error for request ${requestId}`, { requestId }, error); this.terminateActor(requestId); } @@ -432,7 +432,7 @@ export class EboProcessor { `Could not create a request for epoch ${epoch} and chain ${chain}.`, ); - this.notifier.sendError( + await this.notifier.sendError( `Could not create a request for epoch ${epoch} and chain ${chain}.`, { epoch, chain }, err, @@ -450,7 +450,7 @@ export class EboProcessor { this.logger.error(`Requests creation failed: ${err}`); } - this.notifier.sendError("Error creating missing requests", { epoch }, err); + await this.notifier.sendError("Error creating missing requests", { epoch }, err); } } @@ -459,7 +459,7 @@ export class EboProcessor { * * @param requestId the ID of the request the actor is handling */ - private terminateActor(requestId: RequestId) { + private async terminateActor(requestId: RequestId) { this.logger.info(`Terminating actor handling request ${requestId}...`); const deletedActor = this.actorsManager.deleteActor(requestId); @@ -469,7 +469,7 @@ export class EboProcessor { } else { this.logger.warn(alreadyDeletedActorWarning(requestId)); - this.notifier.sendError( + await this.notifier.sendError( `Actor handling request ${requestId} was already terminated.`, { requestId }, new Error("Actor already deleted"), diff --git a/packages/automated-dispute/tests/services/discordNotifier.spec.ts b/packages/automated-dispute/tests/services/discordNotifier.spec.ts index d4ffc8e..41ed90c 100644 --- a/packages/automated-dispute/tests/services/discordNotifier.spec.ts +++ b/packages/automated-dispute/tests/services/discordNotifier.spec.ts @@ -1,4 +1,4 @@ -import { stringify } from "@ebo-agent/shared"; +import { ILogger, stringify } from "@ebo-agent/shared"; import { WebhookClient } from "discord.js"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -18,27 +18,19 @@ vi.mock("discord.js", () => { }; }); -vi.mock("../../src/Logger", () => { - const mockLogger = { - error: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - debug: vi.fn(), - }; - return { - Logger: { - getInstance: () => mockLogger, - }, - }; -}); - describe("DiscordNotifier", () => { let notifier: DiscordNotifier; const webhookUrl = "https://discord.com/api/webhooks/TEST/TEST"; beforeEach(() => { vi.clearAllMocks(); - notifier = new DiscordNotifier(webhookUrl); + const mockLogger: ILogger = { + error: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + }; + notifier = new DiscordNotifier(webhookUrl, mockLogger); }); describe("send", () => {