feature: enhance notification system with advanced configuration options and retry logic - #19
Conversation
…ons and retry logic
WCY-dt
added
documentation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances PongHub's notification system by introducing advanced configuration options and retry logic. The changes significantly expand notification capabilities across all supported channels (Discord, Slack, Telegram, WeChat, Email, and Custom Webhooks) while adding robust error handling and retry mechanisms.
- Restructured configuration types into separate files for better maintainability
- Added comprehensive retry logic with exponential backoff for all notification channels
- Enhanced payload formats with support for rich embeds, mentions, custom templates, and multiple authentication methods
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/types/structures/configure/service.go | Extracted Service and Endpoint type definitions from main configure file |
| internal/types/structures/configure/notification.go | Comprehensive notification configuration types with advanced options for all channels |
| internal/types/structures/configure/default.go | Simple DefaultConfig type definition |
| internal/types/structures/configure/configure.go | Cleaned up main configuration file by removing moved types |
| internal/notifier/channels/wechat.go | Enhanced WeChat notifier with message types, mentions, and retry logic |
| internal/notifier/channels/webhook.go | Major webhook enhancement with templates, authentication, and format support |
| internal/notifier/channels/utils.go | Robust HTTP utility functions with retry logic and error handling |
| internal/notifier/channels/telegram.go | Enhanced Telegram support with multiple parse modes and formatting |
| internal/notifier/channels/slack.go | Added Block Kit support, mentions, and advanced Slack features |
| internal/notifier/channels/email.go | Improved email handling with proper headers and error management |
| internal/notifier/channels/discord.go | Enhanced Discord integration with embeds, mentions, and retry logic |
| README_CN.md | Updated Chinese documentation with new notification features |
| README.md | Updated English documentation with comprehensive notification examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Characters that need to be escaped in MarkdownV2: '_', '*', '[', ']', '(', ')', '~', '`', '>', '#', '+', '-', '=', '|', '{', '}', '.', '!' | ||
| specialChars := []string{"_", "*", "[", "]", "(", ")", "~", "`", ">", "#", "+", "-", "=", "|", "{", "}", ".", "!"} |
There was a problem hiding this comment.
The special characters slice is recreated on every call to escapeMarkdownV2. Consider making this a package-level constant to avoid repeated slice allocation.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func isValidPhoneNumber(phone string) bool { | ||
| // Remove common separators and spaces for validation | ||
| cleanPhone := regexp.MustCompile(`[\s\-().]+`).ReplaceAllString(phone, "") | ||
|
|
||
| // Check various international phone number patterns | ||
| patterns := []*regexp.Regexp{ | ||
| // International format with + prefix (E.164 format) | ||
| regexp.MustCompile(`^\+[1-9]\d{1,14}$`), | ||
| // US/Canada format (10-11 digits, can start with 1) | ||
| regexp.MustCompile(`^1?[2-9]\d{2}[2-9]\d{6}$`), | ||
| // Chinese mobile numbers (11 digits starting with 1) | ||
| regexp.MustCompile(`^1[3-9]\d{9}$`), | ||
| // UK mobile numbers (11 digits starting with 07) | ||
| regexp.MustCompile(`^07\d{9}$`), | ||
| // General international mobile (7-15 digits, not starting with 0) | ||
| regexp.MustCompile(`^[1-9]\d{6,14}$`), | ||
| // European format (8-15 digits) | ||
| regexp.MustCompile(`^[1-9]\d{7,14}$`), | ||
| } |
There was a problem hiding this comment.
The phone number validation patterns have overlapping and potentially contradictory rules. Pattern 5 (^[1-9]\d{6,14}$) and Pattern 6 (^[1-9]\d{7,14}$) are redundant since Pattern 5 already covers the range of Pattern 6. Additionally, the US/Canada pattern is too restrictive and may not match all valid North American numbers. Consider consolidating these patterns and using a well-tested phone validation library instead of maintaining custom regex patterns.
| // WebhookError represents a webhook-specific error | ||
| type WebhookError struct { | ||
| StatusCode int | ||
| Body string | ||
| Retryable bool | ||
| Message string | ||
| } |
There was a problem hiding this comment.
The WebhookError type is defined but never used in the code. Either implement error handling using this type or remove the unused definition to avoid confusion.
| // WebhookError represents a webhook-specific error | |
| type WebhookError struct { | |
| StatusCode int | |
| Body string | |
| Retryable bool | |
| Message string | |
| } |
| } | ||
|
|
||
| // SendHTTPRequest sends an HTTP request with retry logic | ||
| func sendHTTPRequest(url string, method string, payload interface{}, headers map[string]string, maxRetries, timeout int, skipTLSVerify bool) error { |
There was a problem hiding this comment.
The functions sendHTTPRequest and sendHTTPRequestWithCustomBody have significant code duplication in their retry logic and error handling. Consider refactoring to extract the common retry logic into a shared helper function to reduce duplication and improve maintainability.
| } | ||
|
|
||
| // SendHTTPRequestWithCustomBody sends an HTTP request with custom body content | ||
| func sendHTTPRequestWithCustomBody(url string, method string, body io.Reader, contentType string, headers map[string]string, maxRetries, timeout int, skipTLSVerify bool) error { |
There was a problem hiding this comment.
The functions sendHTTPRequest and sendHTTPRequestWithCustomBody have significant code duplication in their retry logic and error handling. Consider refactoring to extract the common retry logic into a shared helper function to reduce duplication and improve maintainability.
| waitTime := time.Duration(attempt) * time.Second | ||
| if waitTime > 10*time.Second { | ||
| waitTime = 10 * time.Second | ||
| } |
There was a problem hiding this comment.
The retry backoff logic uses a linear delay that caps at 10 seconds, which is not optimal for avoiding thundering herd problems. Consider implementing exponential backoff with jitter for better distributed system behavior.
Description
Fixes #18
Type of change
Please delete options that are not relevant.
Checklist