Slack Provider: Fixes slack_username and adds slack_icon_emoji #504
Conversation
WalkthroughThe changes update the Slack provider's message sending logic to use direct HTTP POST requests to Slack's webhook endpoint, replacing the previous use of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SlackProvider
participant SlackWebhook
User->>SlackProvider: Send(message, options)
SlackProvider->>SlackWebhook: HTTP POST (JSON: text, username, icon_emoji)
SlackWebhook-->>SlackProvider: HTTP response
SlackProvider-->>User: Success/Error
Estimated code review effort3 (~40 minutes) Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/providers/slack/slack.go (1)
55-92: Consider adding empty URL validation before prefix check.While the webhook URL prefix validation is good, consider also checking for empty URLs first to provide clearer error messages.
// Send via webhook with emoji and username +if pr.SlackWebHookURL == "" { + err := errors.Wrap(fmt.Errorf("slack webhook URL is required"), + fmt.Sprintf("failed to send slack notification for id: %s ", pr.ID)) + SlackErr = multierr.Append(SlackErr, err) + continue +} if !strings.HasPrefix(pr.SlackWebHookURL, "https://hooks.slack.com/services/") { err := errors.Wrap(fmt.Errorf("invalid slack webhook URL"), fmt.Sprintf("failed to send slack notification for id: %s ", pr.ID)) SlackErr = multierr.Append(SlackErr, err) continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)pkg/providers/slack/slack.go(4 hunks)
🔇 Additional comments (2)
pkg/providers/slack/slack.go (1)
32-32: Field addition looks good!The new
SlackIconEmojifield follows the existing naming conventions and structure pattern.README.md (1)
91-91: Documentation updates are clear and consistent!The new
slack_icon_emojiconfiguration option is properly documented with a valid example emoji format.Also applies to: 98-98
| resp, err := http.Post(pr.SlackWebHookURL, "application/json", bytes.NewBuffer(jsonPayload)) | ||
| if err != nil || resp.StatusCode >= 400 { | ||
| if err == nil { | ||
| err = fmt.Errorf("received non-success status: %s", resp.Status) | ||
| } | ||
| err = errors.Wrap(err, fmt.Sprintf("failed to send slack notification for id: %s", pr.ID)) | ||
| SlackErr = multierr.Append(SlackErr, err) | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Close response body and add request timeout.
The HTTP response body must be closed to prevent resource leaks. Also, consider adding a timeout to prevent hanging requests.
+client := &http.Client{
+ Timeout: 10 * time.Second,
+}
+
-resp, err := http.Post(pr.SlackWebHookURL, "application/json", bytes.NewBuffer(jsonPayload))
+resp, err := client.Post(pr.SlackWebHookURL, "application/json", bytes.NewBuffer(jsonPayload))
+if resp != nil {
+ defer resp.Body.Close()
+}
if err != nil || resp.StatusCode >= 400 {
if err == nil {
err = fmt.Errorf("received non-success status: %s", resp.Status)
}
err = errors.Wrap(err, fmt.Sprintf("failed to send slack notification for id: %s", pr.ID))
SlackErr = multierr.Append(SlackErr, err)
continue
}Don't forget to add the time import at the top of the file.
🤖 Prompt for AI Agents
In pkg/providers/slack/slack.go around lines 101 to 110, the HTTP response body
is not closed, causing potential resource leaks, and the HTTP request lacks a
timeout, risking hanging requests. Fix this by deferring resp.Body.Close()
immediately after the POST request succeeds, and replace http.Post with an
http.Client that has a timeout set (e.g., 10 seconds). Also, add the "time"
package import at the top of the file to support the timeout configuration.
1 participant
Hi, this PR fixes and implements the use of the
slack_usernameconfiguration, which was previously non-functional.I'm also adding
slack_icon_emoji, which sets the emoji for the username. When adding this, I noticed that shoutrrr does not support setting icon_emoji in Slack messages.Slack expects the icon_emoji to be included in the JSON payload of the POST request, but shoutrrr only supports basic message text and allows the username through a query parameter — it does not permit modifying the JSON payload structure to include icon_emoji.
To fully support both username and emoji, I replaced the shoutrrr.Send() call with a simple http.Post using a custom JSON payload. This provides complete control and ensures compatibility with Slack’s webhook API.
I hope this is okay. If not, I can leave the
slack_icon_emojiout of this PR and revert to the shoutrrr implementation, but at least it will make theslack_usernamework.Summary by CodeRabbit
New Features
Documentation