Skip to content

feat: slack notifications #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/processing/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,7 @@ COINGECKO_API_TYPE=demo
RETRY_MAX_ATTEMPTS=3
RETRY_BASE_DELAY_MS=3000
RETRY_FACTOR=2
RETRY_MAX_DELAY_MS=300000
RETRY_MAX_DELAY_MS=300000

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now or later we should add these to the README and mark if required or not

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the reminder ser

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTIFIER_PROVIDER=# 'slack' or 'null'
SLACK_WEBHOOK_URL=
2 changes: 2 additions & 0 deletions apps/processing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ Available options:
| `RETRY_BASE_DELAY_MS` | Base delay for retry attempts | 3000 | No | |
| `RETRY_FACTOR` | Delay factor for retry attempts | 2 | No | |
| `RETRY_MAX_DELAY_MS` | Maximum delay for retry attempts | 300000 | No | |
| `NOTIFIER_PROVIDER` | Notifier provider (slack or null) | null | No | |
| `SLACK_WEBHOOK_URL` | Slack webhook URL | N/A | Only if NOTIFIER_PROVIDER is slack | |

## Available Scripts

Expand Down
12 changes: 12 additions & 0 deletions apps/processing/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ const baseSchema = z.object({
RETRY_BASE_DELAY_MS: z.coerce.number().int().min(1).default(3000), // 3 seconds
RETRY_FACTOR: z.coerce.number().min(1).default(2),
RETRY_MAX_DELAY_MS: z.coerce.number().int().min(1).optional(), // 5 minute
NOTIFIER_PROVIDER: z.enum(["slack", "null"]).default("null"),
SLACK_WEBHOOK_URL: z.string().url().optional(),
LOG_LEVEL: z.enum(["error", "warn", "info", "debug"]).default("info"),
});

const dummyPricingSchema = baseSchema.extend({
Expand Down Expand Up @@ -75,6 +78,15 @@ const validationSchema = z
...val,
};
})
.superRefine((data, ctx) => {
if (data.NOTIFIER_PROVIDER === "slack" && !data.SLACK_WEBHOOK_URL) {
ctx.addIssue({
code: "custom",
message: "SLACK_WEBHOOK_URL is required when NOTIFIER_PROVIDER is slack",
path: ["SLACK_WEBHOOK_URL"],
});
}
})
.and(
z
.discriminatedUnion("METADATA_SOURCE", [
Expand Down
2 changes: 2 additions & 0 deletions apps/processing/src/services/processing.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class ProcessingService {
kyselyDatabase,
logger,
retryStrategy,
notifier,
} = sharedDependencies;
const {
eventRegistryRepository,
Expand Down Expand Up @@ -84,6 +85,7 @@ export class ProcessingService {
chain.fetchDelayMs,
retryStrategy,
logger,
notifier,
);
const retroactiveProcessor = new RetroactiveProcessor(
chain.id as ChainId,
Expand Down
34 changes: 33 additions & 1 deletion apps/processing/src/services/sharedDependencies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,16 @@ import {
KyselyStrategyRegistryRepository,
KyselyTransactionManager,
} from "@grants-stack-indexer/repository";
import { ExponentialBackoff, ILogger, Logger, RetryStrategy } from "@grants-stack-indexer/shared";
import {
ExponentialBackoff,
ILogger,
INotifier,
Logger,
NotifierConfig,
NotifierFactory,
NotifierProvider,
RetryStrategy,
} from "@grants-stack-indexer/shared";

import { Environment } from "../config/index.js";

Expand All @@ -36,6 +45,7 @@ export type SharedDependencies = {
kyselyDatabase: ReturnType<typeof createKyselyDatabase>;
retryStrategy: RetryStrategy;
logger: ILogger;
notifier: INotifier;
};

/**
Expand All @@ -48,6 +58,8 @@ export class SharedDependenciesService {
static async initialize(env: Environment): Promise<SharedDependencies> {
const logger = Logger.getInstance();

const notifier = NotifierFactory.create(this.getNotifierOptions(env), logger);

// Initialize repositories
const kyselyDatabase = createKyselyDatabase(
{
Expand Down Expand Up @@ -151,6 +163,26 @@ export class SharedDependenciesService {
kyselyDatabase,
retryStrategy,
logger,
notifier,
};
}

private static getNotifierOptions(env: Environment): NotifierConfig<NotifierProvider> {
if (env.NOTIFIER_PROVIDER === "slack") {
if (!env.SLACK_WEBHOOK_URL) {
throw new Error("SLACK_WEBHOOK_URL is required when NOTIFIER_PROVIDER is 'slack'");
}

return {
notifierProvider: env.NOTIFIER_PROVIDER,
opts: {
webhookUrl: env.SLACK_WEBHOOK_URL,
},
};
}

return {
notifierProvider: "null",
};
}
}
22 changes: 19 additions & 3 deletions packages/data-flow/src/orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
ContractName,
Hex,
ILogger,
INotifier,
isAlloEvent,
isStrategyEvent,
ProcessorEvent,
Expand Down Expand Up @@ -79,8 +80,6 @@ type TokenWithTimestamps = {
* - Retry handling with exponential backoff for transient failures
* - Comprehensive error handling and logging for various failure scenarios
* - Registry tracking of supported/unsupported strategies and events
*
* TODO: Enhance logging and observability
*/
export class Orchestrator {
private readonly eventsQueue: IQueue<ProcessorEvent<ContractName, AnyEvent>>;
Expand Down Expand Up @@ -113,6 +112,7 @@ export class Orchestrator {
private fetchDelayInMs: number = 10000,
private retryStrategy: RetryStrategy,
private logger: ILogger,
private notifier: INotifier,
private environment: "development" | "staging" | "production" = "development",
) {
this.eventsFetcher = new EventsFetcher(this.indexerClient);
Expand Down Expand Up @@ -199,7 +199,6 @@ export class Orchestrator {
rawEvent: event,
});
}
// TODO: notify
if (
error instanceof UnsupportedStrategy ||
error instanceof InvalidEvent ||
Expand All @@ -221,6 +220,11 @@ export class Orchestrator {
className: Orchestrator.name,
chainId: this.chainId,
});
void this.notifier.send(error.message, {
chainId: this.chainId,
event: event!,
stack: error.getFullStack(),
});
} else if (error instanceof Error || isNativeError(error)) {
const shouldIgnoreError = this.shouldIgnoreTimestampsUpdatedError(
error,
Expand All @@ -232,6 +236,11 @@ export class Orchestrator {
className: Orchestrator.name,
chainId: this.chainId,
});
void this.notifier.send(error.message, {
chainId: this.chainId,
event: event!,
stack: error.stack,
});
}
} else {
this.logger.error(
Expand All @@ -241,6 +250,13 @@ export class Orchestrator {
chainId: this.chainId,
},
);
void this.notifier.send(
`Error processing event: ${stringify(event)} ${error}`,
{
chainId: this.chainId,
event: event!,
},
);
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions packages/data-flow/test/unit/orchestrator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
Hex,
ICacheable,
ILogger,
INotifier,
ProcessorEvent,
RateLimitError,
StrategyEvent,
Expand Down Expand Up @@ -74,6 +75,9 @@ describe("Orchestrator", { sequential: true }, () => {
info: vi.fn(),
warn: vi.fn(),
};
const notifier: INotifier = {
send: vi.fn(),
};

beforeEach(() => {
// Setup mock implementations
Expand Down Expand Up @@ -133,6 +137,7 @@ describe("Orchestrator", { sequential: true }, () => {
mockFetchDelay,
new ExponentialBackoff({ baseDelay: 10, maxAttempts: 3, factor: 2 }),
logger,
notifier,
);
});

Expand Down
10 changes: 10 additions & 0 deletions packages/shared/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,13 @@ You can import the package in your TypeScript files as follows:
```typescript
import { ILogger, ProcessorEvent, stringify } from "@grants-stack-indexer/shared";
```

### Configuring the Notifier

#### Slack

Visit [Slack API docs](https://api.slack.com/messaging/webhooks) to create a Slack app and configure a webhook URL. You can customize the bot's name, icons and permissions in the app settings.

#### Null

The null notifier is a no-op notifier that does nothing. It is useful when you want to disable notifications.
1 change: 1 addition & 0 deletions packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"test:cov": "vitest run --config vitest.config.ts --coverage"
},
"dependencies": {
"@slack/webhook": "7.0.4",
"bignumber.js": "9.1.2",
"viem": "2.21.19",
"winston": "3.15.0"
Expand Down
8 changes: 8 additions & 0 deletions packages/shared/src/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,11 @@ export { ExponentialBackoff, RetryHandler } from "./internal.js";
export type { RetryStrategy, RetryStrategyOptions } from "./internal.js";

export type { ICacheable } from "./internal.js";

export type {
INotifier,
NotifierConfig,
NotifierProvider,
SlackNotifierConfig,
} from "./internal.js";
export { SlackNotifier, NullNotifier, NotifierFactory } from "./internal.js";
1 change: 1 addition & 0 deletions packages/shared/src/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export * from "./tokens/tokens.js";
export * from "./exceptions/index.js";
export * from "./retry/index.js";
export * from "./caching/index.js";
export * from "./notifier/index.js";
5 changes: 5 additions & 0 deletions packages/shared/src/notifier/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export * from "./notifier.interface.js";
export * from "./notifierConfig.interface.js";
export * from "./slack.notifier.js";
export * from "./null.notifier.js";
export * from "./notifier.factory.js";
28 changes: 28 additions & 0 deletions packages/shared/src/notifier/notifier.factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { ILogger, NullNotifier, SlackNotifier } from "../internal.js";
import { INotifier, NotifierConfig, NotifierProvider } from "./index.js";

/**
* Factory class to create instances of INotifier based on configuration.
*/
export class NotifierFactory {
/**
* Creates an instance of INotifier based on configuration.
*
* @param {NotifierConfig<NotifierProvider>} options - The configuration for the notifier.
* @returns {INotifier} An instance of INotifier.
*/
static create(options: NotifierConfig<NotifierProvider>, logger: ILogger): INotifier {
let notifier: INotifier;

switch (options.notifierProvider) {
case "slack":
notifier = new SlackNotifier(options.opts, logger);
break;
// Can add more notification services here, like TelegramNotifier, etc.
default:
notifier = new NullNotifier();
}

return notifier;
}
}
23 changes: 23 additions & 0 deletions packages/shared/src/notifier/notifier.interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { AnyIndexerFetchedEvent, ChainId } from "../internal.js";

/**
* Context information for notifications.
* @property event - Optional event that triggered the notification.
* @property chainId - The chain ID where the event occurred.
* @property stack - Optional stack trace for error notifications.
*/
export type NotifierContext = {
event?: AnyIndexerFetchedEvent;
chainId: ChainId;
stack?: string;
};
Comment on lines +9 to +13
Copy link

@coderabbitai coderabbitai bot Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add JSDoc documentation for the type.

As per the coding guidelines, please add JSDoc documentation to describe the purpose and properties of the NotifierContext type.

Apply this diff to add documentation:

+/**
+ * Context information for notifications.
+ * @property event - Optional event that triggered the notification.
+ * @property chainId - The blockchain network ID where the event occurred.
+ * @property stack - Optional stack trace for error notifications.
+ */
 export type NotifierContext = {
     event?: AnyIndexerFetchedEvent;
     chainId: ChainId;
     stack?: string;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export type NotifierContext = {
event?: AnyIndexerFetchedEvent;
chainId: ChainId;
stack?: string;
};
/**
* Context information for notifications.
* @property event - Optional event that triggered the notification.
* @property chainId - The blockchain network ID where the event occurred.
* @property stack - Optional stack trace for error notifications.
*/
export type NotifierContext = {
event?: AnyIndexerFetchedEvent;
chainId: ChainId;
stack?: string;
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@0xnigir1 Thank you for implementing the suggested changes! The JSDoc documentation has been added correctly.


🏁 Script executed:

#!/bin/bash
# Fetch the specific commit changes
gh pr view 66 --json commits --jq '.commits[] | select(.oid == "e839723015f662867708394b3c09e21e385040c1") | .messageHeadline'
git show e839723015f662867708394b3c09e21e385040c1 -- packages/shared/src/notifier/notifier.interface.ts

Length of output: 1499


@0xnigir1 Thank you for implementing the documentation changes! I see you've gone above and beyond by also improving the documentation for the INotifier interface's send method. The changes look great!


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!


export interface INotifier {
/**
* Sends a notification to the notifier provider.
* @param message - The message to notify.
* @param context - The context information for the notification.
* @returns A promise that resolves when the notification is sent.
*/
send(message: string, context: NotifierContext): Promise<void>;
}
24 changes: 24 additions & 0 deletions packages/shared/src/notifier/notifierConfig.interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { SlackOptions } from "./index.js";

export type NotifierProvider = "slack" | "null";

/**
* Configuration type for the Slack notifier.
* @property notifierProvider - Must be set to "slack" to use Slack notifications.
* @property opts - Slack-specific configuration options.
*/
export type SlackNotifierConfig = {
notifierProvider: "slack";
opts: SlackOptions;
};
Comment on lines +10 to +13
Copy link

@coderabbitai coderabbitai bot Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add JSDoc documentation for the type.

As per the coding guidelines, please add JSDoc documentation to describe the purpose and properties of the SlackNotifierConfig type.

Apply this diff to add documentation:

+/**
+ * Configuration type for the Slack notifier.
+ * @property notifierProvider - Must be set to "slack" to use Slack notifications.
+ * @property opts - Slack-specific configuration options.
+ */
 export type SlackNotifierConfig = {
     notifierProvider: "slack";
     opts: SlackOptions;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export type SlackNotifierConfig = {
notifierProvider: "slack";
opts: SlackOptions;
};
/**
* Configuration type for the Slack notifier.
* @property notifierProvider - Must be set to "slack" to use Slack notifications.
* @property opts - Slack-specific configuration options.
*/
export type SlackNotifierConfig = {
notifierProvider: "slack";
opts: SlackOptions;
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@0xnigir1 Perfect! The JSDoc documentation has been added correctly. This will help improve code maintainability and developer experience.


🏁 Script executed:

#!/bin/bash
# Fetch the commit details
gh api /repos/defi-wonderland/grants-stack-indexer-v2/commits/e839723015f662867708394b3c09e21e385040c1 | jq -r '.files[] | select(.filename == "packages/shared/src/notifier/notifierConfig.interface.ts") | .patch'

Length of output: 1009


@0xnigir1 Excellent work! Not only did you add the suggested documentation for SlackNotifierConfig, but you also took the initiative to document the NotifierConfig generic type. The documentation is clear and comprehensive, especially the explanation of the generic type parameter. This will greatly help other developers understand the configuration system. 👏


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!


/**
* Generic configuration type for notifiers.
* @template Source - The type of notifier provider to configure.
* @returns Configuration type specific to the provider: SlackNotifierConfig for "slack", or a null configuration otherwise.
*/
export type NotifierConfig<Source extends NotifierProvider> = Source extends "slack"
? SlackNotifierConfig
: {
notifierProvider: "null";
};
Comment on lines +20 to +24
Copy link

@coderabbitai coderabbitai bot Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add JSDoc documentation for the generic type.

As per the coding guidelines, please add JSDoc documentation to describe the purpose and type parameter of the NotifierConfig type.

Apply this diff to add documentation:

+/**
+ * Generic configuration type for notifiers.
+ * @template Source - The type of notifier provider to configure.
+ * @returns Configuration type specific to the provider: SlackNotifierConfig for "slack", or a null configuration otherwise.
+ */
 export type NotifierConfig<Source extends NotifierProvider> = Source extends "slack"
     ? SlackNotifierConfig
     : {
           notifierProvider: "null";
       };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export type NotifierConfig<Source extends NotifierProvider> = Source extends "slack"
? SlackNotifierConfig
: {
notifierProvider: "null";
};
/**
* Generic configuration type for notifiers.
* @template Source - The type of notifier provider to configure.
* @returns Configuration type specific to the provider: SlackNotifierConfig for "slack", or a null configuration otherwise.
*/
export type NotifierConfig<Source extends NotifierProvider> = Source extends "slack"
? SlackNotifierConfig
: {
notifierProvider: "null";
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@0xnigir1 Thank you for implementing the suggested changes! The JSDoc documentation has been added correctly.


🏁 Script executed:

#!/bin/bash
# Fetch the specific commit changes
gh pr view 66 --json commits | jq -r '.commits[] | select(.oid == "e839723015f662867708394b3c09e21e385040c1") | .messageHeadline'

# Check the content of the file after the commit
git show e839723015f662867708394b3c09e21e385040c1:packages/shared/src/notifier/notifierConfig.interface.ts

Length of output: 1058


@0xnigir1 Thank you for implementing the suggested changes and going the extra mile! Not only did you add the requested JSDoc for NotifierConfig, but you also added helpful documentation for SlackNotifierConfig. This comprehensive approach to documentation is excellent.


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

10 changes: 10 additions & 0 deletions packages/shared/src/notifier/null.notifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { INotifier, NotifierContext } from "../internal.js";

/**
* A null implementation of INotifier that performs no actions but allows the application to have an undefined notifier if desired
*/
export class NullNotifier implements INotifier {
async send(_message: string, _context: NotifierContext): Promise<void> {
// Do nothing
}
}
Loading
Loading