fix: pr comments

defi-wonderland · Nov 6, 2024 · f6ceff2 · f6ceff2
1 parent 27c1f7d
commit f6ceff2
Show file tree

Hide file tree

Showing 11 changed files with 247 additions and 246 deletions.
diff --git a/apps/agent/README.md b/apps/agent/README.md
@@ -37,7 +37,7 @@ cp .env.example .env
 | `BLOCK_NUMBER_RPC_URLS_MAP`     | JSON map of chain IDs to arrays of RPC URLs for Block Number calculations | Yes      |
 | `BLOCK_NUMBER_BLOCKMETA_TOKEN`  | Bearer token for the Blockmeta service (see notes below on how to obtain) | Yes      |
 | `EBO_AGENT_CONFIG_FILE_PATH`    | Path to the agent YAML configuration file                                 | Yes      |
-| `DISCORD_WEBHOOK`               | Your Discord channel webhook for notifications                            | Yes      |
+| `DISCORD_WEBHOOK`               | Your Discord channel webhook for notifications                            | No       |
 
 **Notes:**
 

diff --git a/apps/agent/config.example.yml b/apps/agent/config.example.yml
@@ -31,3 +31,6 @@ processor:
         requestModule: "0x1234567890123456789012345678901234567890" # Address of the Request module
         responseModule: "0x1234567890123456789012345678901234567890" # Address of the Response module
         escalationModule: "0x1234567890123456789012345678901234567890" # Address of the Escalation module
+
+notifier:
+    discordDefaultAvatarUrl: "https://cryptologos.cc/logos/the-graph-grt-logo.png" # Default avatar URL for Discord notifications
diff --git a/packages/automated-dispute/src/config.ts b/packages/automated-dispute/src/config.ts
@@ -1,7 +1,7 @@
 import { z } from "zod";
 
 const ConfigSchema = z.object({
-    DISCORD_WEBHOOK: z.string().min(1),
+    DISCORD_WEBHOOK: z.string().url().optional(),
 });
 
 export const config = ConfigSchema.parse(process.env);
diff --git a/packages/automated-dispute/src/exceptions/errorHandler.ts b/packages/automated-dispute/src/exceptions/errorHandler.ts
@@ -24,13 +24,11 @@ export class ErrorHandler {
             this.logger.error(`Error executing custom action: ${actionError}`);
         } finally {
             if (strategy.shouldNotify) {
-                const errorMessage = this.notificationService.createErrorMessage(
-                    error,
-                    context,
+                await this.notificationService.sendError(
                     "An error occurred in the custom contract",
+                    context,
+                    error,
                 );
-
-                await this.notificationService.send(errorMessage);
             }
 
             if (strategy.shouldReenqueue && context.reenqueueEvent) {

diff --git a/packages/automated-dispute/src/interfaces/notificationService.ts b/packages/automated-dispute/src/interfaces/notificationService.ts
@@ -40,13 +40,36 @@ export interface NotificationService {
      */
     send(message: IMessage): Promise<void>;
 
+    /**
+     * Sends a notification message and throws an exception if sending fails.
+     *
+     * @param {IMessage} message - The message to send.
+     * @returns {Promise<void>} A promise that resolves when the message is sent.
+     * @throws {NotificationFailureException} If sending the message fails.
+     */
+    sendOrThrow(message: IMessage): Promise<void>;
+
     /**
      * Creates an IMessage from an error.
      *
-     * @param err - The error object of type unknown.
-     * @param context - Additional context for the error.
-     * @param defaultMessage - A default message describing the error context.
-     * @returns An IMessage object ready to be sent via the notifier.
+     * @param {string} defaultMessage - A default message describing the error context.
+     * @param {unknown} [context] - Additional context for the error.
+     * @param {unknown} [err] - The error object.
+     * @returns {IMessage} An IMessage object ready to be sent via the notifier.
+     */
+    createErrorMessage(defaultMessage: string, context?: unknown, err?: unknown): IMessage;
+
+    /**
+     * Sends an error notification message.
+     *
+     * @param {string} defaultMessage - A default message describing the error context.
+     * @param {Record<string, unknown>} [context] - Additional context for the error.
+     * @param {unknown} [err] - The error object.
+     * @returns {Promise<void>} A promise that resolves when the message is sent.
      */
-    createErrorMessage(err: unknown, context: unknown, defaultMessage: string): IMessage;
+    sendError(
+        defaultMessage: string,
+        context?: Record<string, unknown>,
+        err?: unknown,
+    ): Promise<void>;
 }
diff --git a/packages/automated-dispute/src/services/discordNotifier.ts b/packages/automated-dispute/src/services/discordNotifier.ts
@@ -1,5 +1,6 @@
 import { isNativeError } from "util/types";
 import type { APIEmbed, JSONEncodable } from "discord.js";
+import { Logger } from "@ebo-agent/shared";
 import { WebhookClient, WebhookMessageCreateOptions } from "discord.js";
 
 import { NotificationFailureException } from "../exceptions/index.js";
@@ -14,18 +15,23 @@ 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 {string} [defaultAvatarUrl] - The default avatar URL to use if none is provided.
      */
-    constructor(url: string) {
+    constructor(url: string, defaultAvatarUrl?: string) {
         this.client = new WebhookClient({ url });
+        this.defaultAvatarUrl = defaultAvatarUrl;
+        this.logger = Logger.getInstance();
     }
 
     /**
-     * Sends a notification message to Discord.
+     * Sends a notification message to Discord. Errors are logged but not thrown.
      *
      * @param {IMessage} message - The message to send.
      * @returns {Promise<void>} A promise that resolves when the message is sent.
@@ -34,7 +40,26 @@ export class DiscordNotifier implements NotificationService {
         const webhookMessage = this.buildWebhookMessage(message);
         try {
             await this.client.send(webhookMessage);
-            console.error(this.client, this.client);
+        } catch (error) {
+            this.logger.error(
+                `Failed to send Discord notification: ${
+                    error instanceof Error ? error.message : String(error)
+                }`,
+            );
+        }
+    }
+
+    /**
+     * Sends a notification message to Discord. Throws an exception if sending fails.
+     *
+     * @param {IMessage} message - The message to send.
+     * @returns {Promise<void>} A promise that resolves when the message is sent.
+     * @throws {NotificationFailureException} If sending the message fails.
+     */
+    async sendOrThrow(message: IMessage): Promise<void> {
+        const webhookMessage = this.buildWebhookMessage(message);
+        try {
+            await this.client.send(webhookMessage);
         } catch (error) {
             throw new NotificationFailureException("An error occurred with the Discord client.");
         }
@@ -50,7 +75,7 @@ export class DiscordNotifier implements NotificationService {
         return {
             username: message.username || "EBO Agent",
             content: message.title,
-            avatarURL: message.avatarUrl || "https://cryptologos.cc/logos/the-graph-grt-logo.png",
+            avatarURL: message.avatarUrl || this.defaultAvatarUrl,
             embeds:
                 message.subtitle || message.description
                     ? [this.buildWebhookMessageEmbed(message)]
@@ -76,42 +101,58 @@ export class DiscordNotifier implements NotificationService {
     /**
      * Creates an IMessage from an error.
      *
-     * @param err - The error object of type unknown.
-     * @param context - Additional context for the error.
-     * @param defaultMessage - A default message describing the error context.
-     * @returns An IMessage object ready to be sent via the notifier.
+     * @param {string} defaultMessage - A default message describing the error context.
+     * @param {unknown} [context={}] - Additional context for the error.
+     * @param {unknown} [err] - The error object.
+     * @returns {IMessage} An IMessage object ready to be sent via the notifier.
      */
     public createErrorMessage(
-        err: unknown,
-        context: Record<string, unknown>,
         defaultMessage: string,
+        context: unknown = {},
+        err?: unknown,
     ): IMessage {
+        const contextObject = typeof context === "object" && context !== null ? context : {};
         if (isNativeError(err)) {
             return {
                 title: `**Error:** ${err.name} - ${err.message}`,
                 description: `**Context:**\n\`\`\`json\n${JSON.stringify(
                     {
-                        message: defaultMessage,
+                        ...contextObject,
                         stack: err.stack,
-                        ...context,
                     },
                     null,
                     2,
                 )}\n\`\`\``,
             };
         } else {
             return {
-                title: `**Error:** Unknown error`,
+                title: `**Error:** ${defaultMessage}`,
                 description: `**Context:**\n\`\`\`json\n${JSON.stringify(
                     {
-                        message: defaultMessage,
-                        error: String(err),
-                        ...context,
+                        ...contextObject,
+                        error: err ? JSON.stringify(err, null, 2) : undefined,
                     },
                     null,
                     2,
                 )}\n\`\`\``,
             };
         }
     }
+
+    /**
+     * Sends an error notification message to Discord.
+     *
+     * @param {string} defaultMessage - A default message describing the error context.
+     * @param {Record<string, unknown>} [context={}] - Additional context for the error.
+     * @param {unknown} [err] - The error object.
+     * @returns {Promise<void>} A promise that resolves when the message is sent.
+     */
+    public async sendError(
+        defaultMessage: string,
+        context: Record<string, unknown> = {},
+        err?: unknown,
+    ): Promise<void> {
+        const errorMessage = this.createErrorMessage(defaultMessage, context, err);
+        await this.send(errorMessage);
+    }
 }
diff --git a/packages/automated-dispute/src/services/eboProcessor.ts b/packages/automated-dispute/src/services/eboProcessor.ts
@@ -62,14 +62,12 @@ export class EboProcessor {
             } catch (err) {
                 this.logger.error(`Unhandled error during the event loop: ${err}`);
 
-                const errorMessage = this.notifier.createErrorMessage(
-                    err,
-                    { message: "Unhandled error during the event loop" },
+                await this.notifier.sendError(
                     "Unhandled error during the event loop",
+                    { message: "Unhandled error during the event loop" },
+                    err,
                 );
 
-                await this.notifier.send(errorMessage);
-
                 clearInterval(this.eventsInterval);
 
                 throw err;
@@ -148,7 +146,7 @@ export class EboProcessor {
                     if (err instanceof Error) {
                         this.onActorError(requestId, err);
                     } else {
-                        this.onActorError(requestId, new Error(String(err)));
+                        this.onActorError(requestId, new Error(stringify(err)));
                     }
                 }
             });
@@ -167,13 +165,11 @@ export class EboProcessor {
                 this.logger.error(`Sync failed: ${err}`);
             }
 
-            const errorMessage = this.notifier.createErrorMessage(
-                err,
-                { message: "Error during synchronization" },
+            await this.notifier.sendError(
                 "Error during synchronization",
+                { message: "Error during synchronization" },
+                err,
             );
-
-            await this.notifier.send(errorMessage);
         }
     }
 
@@ -338,18 +334,12 @@ export class EboProcessor {
             } else {
                 this.logger.warn(`Chain ${chainId} not supported by the agent. Skipping...`);
 
-                const errorMessage = this.notifier.createErrorMessage(
-                    new Error("Unsupported chain"),
-                    {
-                        message: `Chain ${chainId} not supported by the agent. Skipping...`,
-                        chainId,
-                        requestId,
-                    },
+                this.notifier.sendError(
                     `Chain ${chainId} not supported by the agent. Skipping...`,
+                    { chainId, requestId },
+                    new Error("Unsupported chain"),
                 );
 
-                this.notifier.send(errorMessage);
-
                 return null;
             }
         } else {
@@ -383,16 +373,7 @@ export class EboProcessor {
                 `The request ${requestId} will stop being tracked by the system.`,
         );
 
-        const errorMessage = this.notifier.createErrorMessage(
-            error,
-            {
-                message: `Actor error for request ${requestId}`,
-                requestId,
-            },
-            `Actor error for request ${requestId}`,
-        );
-
-        this.notifier.send(errorMessage);
+        this.notifier.sendError(`Actor error for request ${requestId}`, { requestId }, error);
 
         this.terminateActor(requestId);
     }
@@ -459,41 +440,15 @@ export class EboProcessor {
                         }
                     }
 
-                    if (err instanceof Error) {
-                        this.logger.error(
-                            `Could not create a request for epoch ${epoch} and chain ${chain}.`,
-                        );
-
-                        this.logger.error(err);
-
-                        const errorMessage = this.notifier.createErrorMessage(
-                            err,
-                            {
-                                message: `Could not create a request for epoch ${epoch} and chain ${chain}.`,
-                                epoch,
-                                chain,
-                            },
-                            `Could not create a request for epoch ${epoch} and chain ${chain}.`,
-                        );
-
-                        await this.notifier.send(errorMessage);
-                    } else {
-                        this.logger.error(
-                            `Could not create a request for epoch ${epoch} and chain ${chain}: ${err}`,
-                        );
-
-                        const errorMessage = this.notifier.createErrorMessage(
-                            err,
-                            {
-                                message: `Could not create a request for epoch ${epoch} and chain ${chain}.`,
-                                epoch,
-                                chain,
-                            },
-                            `Could not create a request for epoch ${epoch} and chain ${chain}.`,
-                        );
-
-                        await this.notifier.send(errorMessage);
-                    }
+                    this.logger.error(
+                        `Could not create a request for epoch ${epoch} and chain ${chain}.`,
+                    );
+
+                    this.notifier.sendError(
+                        `Could not create a request for epoch ${epoch} and chain ${chain}.`,
+                        { epoch, chain },
+                        err,
+                    );
                 }
             });
 
@@ -507,13 +462,7 @@ export class EboProcessor {
                 this.logger.error(`Requests creation failed: ${err}`);
             }
 
-            const errorMessage = this.notifier.createErrorMessage(
-                err,
-                { epoch },
-                "Error creating missing requests",
-            );
-
-            await this.notifier.send(errorMessage);
+            this.notifier.sendError("Error creating missing requests", { epoch }, err);
         }
     }
 
@@ -532,16 +481,11 @@ export class EboProcessor {
         } else {
             this.logger.warn(alreadyDeletedActorWarning(requestId));
 
-            const errorMessage = this.notifier.createErrorMessage(
-                new Error("Actor already deleted"),
-                {
-                    message: `Actor handling request ${requestId} was already terminated.`,
-                    requestId,
-                },
+            this.notifier.sendError(
                 `Actor handling request ${requestId} was already terminated.`,
+                { requestId },
+                new Error("Actor already deleted"),
             );
-
-            this.notifier.send(errorMessage);
         }
     }
 }