Feature: Implement Retry Mechanism for Notifications

[shahnami]

Problem

Currently, when a notification fails to send (e.g., Slack message, Discord message, email, etc.), the system immediately errors out without any retry attempts. This can lead to missed notifications in cases of temporary failures like network issues or rate limiting.

Current Behavior

Looking at the notification services in src/services/notification/, notifications fail immediately on any error without retry attempts. This is less resilient compared to our RPC endpoint management system which has sophisticated retry and rotation mechanisms.

Proposed Solution

Implement a retry mechanism for notifications similar to the RPC endpoint management system, with the following components:

1. Retry Policy Configuration

   • Configurable number of retry attempts
   • Exponential backoff strategy
   • Configurable retry conditions (e.g., network errors, rate limits)

2. Notification Manager

   • Similar to EndpointManager but for notifications
   • Handles retry logic and backoff
   • Manages notification-specific error handling

3. Retry Strategy

   • Define which errors are retryable
   • Handle rate limiting specifically
   • Log retry attempts and failures

Retryable Error Types (Example)

pub enum NotificationError {
    RateLimitError { retry_after: Duration },
    NetworkError,
    TemporaryError,
    PermanentError,
}

fn is_retryable_error(error: &NotificationError) -> bool {
    matches!(error, 
        NotificationError::RateLimitError { .. } |
        NotificationError::NetworkError |
        NotificationError::TemporaryError
    )
}

Integration Points

1. Existing Notification Services

   • Slack (slack.rs)
   • Discord (discord.rs)
   • Email (email.rs)
   • Webhook (webhook.rs)
   • Telegram (telegram.rs)
   • Script (script.rs)

2. Error Handling

   • Update error.rs to include retry-specific error types
   • Add retry-related logging

Acceptance Criteria

• Implement NotificationManager with retry logic
• Add configuration for retry attempts and backoff strategy
• Implement retryable error types and detection
• Add retry support for all notification services
• Add comprehensive logging for retry attempts
• Add metrics for notification success/failure rates
• Add documentation for retry behavior
• Add tests covering retry scenarios

References

• Current RPC retry implementation in endpoint_manager.rs
• Current notification services in src/services/notification/

Additional Considerations

• Consider implementing different retry strategies for different notification types
• Consider adding circuit breaker pattern for failing notification services
• Consider implementing notification queuing for high-load scenarios

[isSerge]

let me do this

[shahnami]

let me do this

Legend 🔥

I was thinking we could have one "retry" mechanism that encapsulates different strategies (so later we can plug & play new strategies).

For now we should have a 2 strategies;

1. Retry by replay (simply send again based on error message)
2. Retry by rotating (retry with a different URL)

This should make it easy for RPC handler to have both strategies with some priority and for notification requests to have just 1 strategy (retry by replay) for example. We can then also include circuit breakers as an optional strategy or error handler.

We have similar logic in the endpoint manager already, but we should look at extracting it into its own module that we can use for both notification requests and rpc requests to avoid redundancy.

This might be a bit of a bigger refactor, but I think the code would be cleaner and more manageable. This is one idea, but I'm open to other suggestions. What do you think?

[isSerge]

I am afraid we might need more than one: Current retry mechanism relies on reqwest-retry and if abstracted it can be re-used for Notifiers based on WebhookNotifier and include both strategies, but EmailNotifier and ScriptNotifier would probably require a separate mechanism, without rotation strategy. So maybe we can have one HTTP-specific abstraction and second generic for any other async operation.

Do we need a separate retry config for each notification type or a common one will do?

[shahnami]

Ah, yep, you're right, so that makes 3 strategies.
As for retry config, let's allow adapting this per notification type (something we can pass to the notifier, or customise similarly to how we implement customize_request), but we will ship it with a default/common configuration.

[shahnami]

@isSerge - given the size of this feature, it would make sense to have a technical design document before we start making major changes. That way we can align on the architecture/restructuring of components beforehand. It may save some time.
Let me know if that's something you'd want to do or if you prefer me to do?

[isSerge]

let me do a simple draft