Skip to content

Commit f4e0504

Browse files
loafoeclaude
andcommitted
fix(slack_webhook): address PR review round 2
- Read and include Slack response body in error messages for debugging - Add code-fence aware text splitting to avoid breaking mrkdwn blocks - Add require.NoError for Start() calls in tests - Fix gci import formatting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 2f2ee95 commit f4e0504

4 files changed

Lines changed: 54 additions & 19 deletions

File tree

pkg/channels/slack_webhook/convert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,4 @@ func parseTableRow(line string) []string {
175175
cells = append(cells, strings.TrimSpace(p))
176176
}
177177
return cells
178-
}
178+
}

pkg/channels/slack_webhook/convert_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,9 @@ func TestSplitContentWithTables(t *testing.T) {
127127

128128
func TestRenderTable(t *testing.T) {
129129
tests := []struct {
130-
name string
131-
input string
132-
expectCode bool
130+
name string
131+
input string
132+
expectCode bool
133133
}{
134134
{
135135
name: "narrow table renders as text",

pkg/channels/slack_webhook/slack_webhook.go

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"fmt"
8+
"io"
89
"net/http"
910
"net/url"
1011
"sort"
@@ -144,11 +145,14 @@ func (c *SlackWebhookChannel) Send(ctx context.Context, msg bus.OutboundMessage)
144145
defer resp.Body.Close()
145146

146147
if resp.StatusCode >= 400 {
148+
respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 512))
147149
logger.ErrorCF("slack_webhook", "Slack API error", map[string]any{
148-
"target": targetName,
149-
"status": resp.StatusCode,
150+
"target": targetName,
151+
"status": resp.StatusCode,
152+
"response": string(respBody),
150153
})
151-
return nil, fmt.Errorf("slack_webhook: send failed: %w", channels.ClassifySendError(resp.StatusCode, fmt.Errorf("status %d", resp.StatusCode)))
154+
return nil, fmt.Errorf("slack_webhook: send failed (status %d: %s): %w",
155+
resp.StatusCode, string(respBody), channels.ClassifySendError(resp.StatusCode, nil))
152156
}
153157

154158
logger.DebugCF("slack_webhook", "Message sent successfully", map[string]any{
@@ -226,19 +230,47 @@ func splitText(text string, maxLen int) []string {
226230
}
227231

228232
var chunks []string
233+
inFence := false
234+
229235
for len(runes) > maxLen {
230-
splitAt := maxLen
231-
window := string(runes[:maxLen])
232-
if idx := strings.LastIndex(window, "\n"); idx > 0 {
233-
splitAt = len([]rune(window[:idx])) + 1
234-
} else if idx := strings.LastIndex(window, " "); idx > 0 {
235-
splitAt = len([]rune(window[:idx])) + 1
236-
}
237-
chunks = append(chunks, string(runes[:splitAt]))
236+
splitAt := findSplitPoint(runes, maxLen, inFence)
237+
chunk := string(runes[:splitAt])
238+
chunks = append(chunks, chunk)
239+
inFence = endsInsideFence(chunk, inFence)
238240
runes = runes[splitAt:]
239241
}
240242
if len(runes) > 0 {
241243
chunks = append(chunks, string(runes))
242244
}
243245
return chunks
244246
}
247+
248+
func findSplitPoint(runes []rune, maxLen int, inFence bool) int {
249+
window := string(runes[:maxLen])
250+
251+
if idx := strings.LastIndex(window, "\n"); idx > 0 {
252+
splitAt := len([]rune(window[:idx])) + 1
253+
if !inFence || !endsInsideFence(string(runes[:splitAt]), inFence) {
254+
return splitAt
255+
}
256+
}
257+
258+
if idx := strings.LastIndex(window, " "); idx > 0 {
259+
splitAt := len([]rune(window[:idx])) + 1
260+
if !inFence {
261+
return splitAt
262+
}
263+
}
264+
265+
if inFence {
266+
if idx := strings.LastIndex(window, "```"); idx > 0 {
267+
return len([]rune(window[:idx]))
268+
}
269+
}
270+
271+
return maxLen
272+
}
273+
274+
func endsInsideFence(text string, wasInFence bool) bool {
275+
return wasInFence != (strings.Count(text, "```")%2 == 1)
276+
}

pkg/channels/slack_webhook/slack_webhook_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ func TestSlackWebhookChannel_FallbackToDefault(t *testing.T) {
147147
ch, err := NewSlackWebhookChannel(bc, cfg, mb)
148148
require.NoError(t, err)
149149
ch.client = server.Client()
150-
ch.Start(context.Background())
150+
err = ch.Start(context.Background())
151+
require.NoError(t, err)
151152

152153
// Send to unknown target - should fall back to default
153154
_, err = ch.Send(context.Background(), bus.OutboundMessage{
@@ -188,11 +189,13 @@ func TestSlackWebhookChannel_ErrorClassification(t *testing.T) {
188189
bc := &config.Channel{Enabled: true}
189190
mb := bus.NewMessageBus()
190191

191-
ch, _ := NewSlackWebhookChannel(bc, cfg, mb)
192+
ch, err := NewSlackWebhookChannel(bc, cfg, mb)
193+
require.NoError(t, err)
192194
ch.client = server.Client()
193-
ch.Start(context.Background())
195+
err = ch.Start(context.Background())
196+
require.NoError(t, err)
194197

195-
_, err := ch.Send(context.Background(), bus.OutboundMessage{Content: "Test"})
198+
_, err = ch.Send(context.Background(), bus.OutboundMessage{Content: "Test"})
196199
require.Error(t, err)
197200

198201
if tt.expectTemp {

0 commit comments

Comments
 (0)