refactor: refactor messaging and file upload with improved error handling (#66)

- Refactor message and file sending logic into separate handleMessages and handleFiles methods
- Stream file uploads via io.Copy rather than loading the entire content into memory
- Add centralized http.Client with timeout for all requests
- Enhance error handling throughout by returning more descriptive and wrapped errors
- Improve response validation for file and message uploads, checking HTTP status and parsing error details
- Update tests to cover plain text messages, embed messages, file uploads, color conversion, and combined features
- Add tests using assert.Error/assert.NoError and checking specific error messages
- Simplify and clarify configuration validation logic

Signed-off-by: appleboy <appleboy.tw@gmail.com>
This commit is contained in:
Bo-Yi Wu
2025-07-05 21:02:30 +08:00
committed by GitHub
parent 1bdf20515c
commit f2b9ede051
2 changed files with 225 additions and 146 deletions
+80 -56
View File
@@ -133,28 +133,25 @@ type (
Config Config
Payload Payload
Commit Commit
httpClient *http.Client
}
)
func (c *Config) validate() error {
var missingFields []string
if c.webhookURL != "" {
_, err := url.Parse(c.webhookURL)
if err != nil {
if _, err := url.Parse(c.webhookURL); err != nil {
return fmt.Errorf("invalid webhook url: %w", err)
}
return nil
}
if c.webhookURL == "" {
var missingFields []string
if c.WebhookID == "" {
missingFields = append(missingFields, "WebhookID")
}
if c.WebhookToken == "" {
missingFields = append(missingFields, "WebhookToken")
}
}
if len(missingFields) > 0 {
return fmt.Errorf("missing discord config: %s", strings.Join(missingFields, ", "))
}
@@ -178,17 +175,12 @@ func templateMessage(t string, plugin Plugin) (string, error) {
func fileUploadRequest(ctx context.Context, uri string, params map[string]string, paramName, path string) (*http.Request, error) {
// Clean and check path
path = filepath.Clean(path)
if _, err := os.Stat(path); err != nil {
file, err := os.Open(path)
if err != nil {
return nil, fmt.Errorf("file %s not accessible: %w", path, err)
}
defer file.Close()
// Read file content
content, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to read file %s: %w", path, err)
}
// Create multipart form
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
@@ -197,7 +189,9 @@ func fileUploadRequest(ctx context.Context, uri string, params map[string]string
if err != nil {
return nil, fmt.Errorf("failed to create form file: %w", err)
}
if _, err = part.Write(content); err != nil {
// Stream file content
if _, err = io.Copy(part, file); err != nil {
return nil, fmt.Errorf("failed to write file content: %w", err)
}
@@ -224,65 +218,84 @@ func fileUploadRequest(ctx context.Context, uri string, params map[string]string
// Exec executes the plugin.
func (p *Plugin) Exec(ctx context.Context) error {
// init http client
p.httpClient = &http.Client{
Timeout: 15 * time.Second,
}
if err := p.Config.validate(); err != nil {
return fmt.Errorf("failed to validate config: %w", err)
}
// check if message is empty
messages := []string{}
if err := p.handleMessages(ctx); err != nil {
return err
}
if err := p.handleFiles(ctx); err != nil {
return err
}
return nil
}
// handleMessages sends all configured messages.
func (p *Plugin) handleMessages(ctx context.Context) error {
// 1. Handle empty message (default template)
if len(p.Config.Message) == 0 {
object := p.Template()
p.Payload.Embeds = []EmbedObject{object}
if err := p.SendMessage(ctx); err != nil {
return fmt.Errorf("failed to send default message: %w", err)
}
return nil
}
// 2. Handle custom messages
for _, m := range p.Config.Message {
if m == "" {
continue
}
messages = append(messages, m)
}
if len(messages) == 0 {
object := p.Template()
p.Payload.Embeds = []EmbedObject{object}
err := p.SendMessage(ctx)
if err != nil {
return err
}
}
if len(messages) > 0 {
for _, m := range messages {
txt, err := templateMessage(m, *p)
if err != nil {
return err
return fmt.Errorf("failed to render template: %w", err)
}
if len(p.Config.Color) != 0 {
// With color, messages are grouped as embeds
if p.Config.Color != "" {
object := p.DefaultTemplate(txt)
p.Payload.Embeds = append(p.Payload.Embeds, object)
} else {
// Without color, send as plain text immediately
p.Payload.Content = txt
err = p.SendMessage(ctx)
if err != nil {
return err
if err := p.SendMessage(ctx); err != nil {
return fmt.Errorf("failed to send plain text message: %w", err)
}
// Reset for next message
p.Clear()
}
}
// 3. Send grouped embeds if any
if len(p.Payload.Embeds) > 0 {
err := p.SendMessage(ctx)
if err != nil {
return err
}
if err := p.SendMessage(ctx); err != nil {
return fmt.Errorf("failed to send embed messages: %w", err)
}
}
return nil
}
// handleFiles sends all configured files.
func (p *Plugin) handleFiles(ctx context.Context) error {
for _, f := range p.Config.File {
if f == "" {
continue
}
err := p.SendFile(ctx, f)
if err != nil {
return err
if err := p.SendFile(ctx, f); err != nil {
return fmt.Errorf("failed to send file %s: %w", f, err)
}
}
return nil
}
@@ -311,12 +324,25 @@ func (p *Plugin) SendFile(ctx context.Context, file string) error {
file,
)
if err != nil {
return err
return fmt.Errorf("failed to create file upload request: %w", err)
}
client := &http.Client{}
_, err = client.Do(request)
resp, err := p.httpClient.Do(request)
if err != nil {
return err
return fmt.Errorf("failed to send file: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode >= http.StatusBadRequest {
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("failed to read response body: %w", err)
}
var jsonResponse map[string]interface{}
if err := json.Unmarshal(bodyBytes, &jsonResponse); err != nil {
return fmt.Errorf("failed to send file, status code: %d, body: %s", resp.StatusCode, string(bodyBytes))
}
return fmt.Errorf("failed to send file, status code: %d, error: %s, code: %v", resp.StatusCode, jsonResponse["message"], jsonResponse["code"])
}
return nil
@@ -327,25 +353,23 @@ func (p *Plugin) SendMessage(ctx context.Context) error {
webhookURL := p.Config.GetWebhookURL()
b := new(bytes.Buffer)
if err := json.NewEncoder(b).Encode(p.Payload); err != nil {
return err
return fmt.Errorf("failed to encode payload: %w", err)
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, webhookURL, b)
if err != nil {
return err
return fmt.Errorf("failed to create request: %w", err)
}
req.Header.Set("Content-Type", "application/json; charset=utf-8")
client := &http.Client{
Timeout: 10 * time.Second,
}
resp, err := client.Do(req)
resp, err := p.httpClient.Do(req)
if err != nil {
return err
return fmt.Errorf("failed to send message: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusNoContent {
// 200 and 204 are both valid status codes for webhooks.
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent {
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("failed to read response body: %w", err)
+124 -69
View File
@@ -10,14 +10,48 @@ import (
)
func TestMissingConfig(t *testing.T) {
var plugin Plugin
plugin := Plugin{}
err := plugin.Exec(context.Background())
assert.NotNil(t, err)
assert.Error(t, err)
assert.Contains(t, err.Error(), "missing discord config")
}
func TestTemplate(t *testing.T) {
func TestSendPlainTextMessage(t *testing.T) {
plugin := Plugin{
Config: Config{
WebhookID: os.Getenv("WEBHOOK_ID"),
WebhookToken: os.Getenv("WEBHOOK_TOKEN"),
Message: []string{"Hello, world!", "This is a test."},
},
Payload: Payload{
Username: "test-bot",
},
}
err := plugin.Exec(context.Background())
assert.NoError(t, err)
}
func TestSendEmbedMessage(t *testing.T) {
plugin := Plugin{
Config: Config{
WebhookID: os.Getenv("WEBHOOK_ID"),
WebhookToken: os.Getenv("WEBHOOK_TOKEN"),
Message: []string{"This is an embed message."},
Color: "#48f442",
},
Payload: Payload{
Username: "embed-bot",
},
}
err := plugin.Exec(context.Background())
assert.NoError(t, err)
}
func TestSendDefaultMessage(t *testing.T) {
plugin := Plugin{
Repo: Repo{
Name: "go-hello",
@@ -26,99 +60,120 @@ func TestTemplate(t *testing.T) {
Commit: Commit{
Author: "appleboy",
Branch: "master",
Message: "update by drone discord plugin. \r\n update by drone discord plugin.",
Message: "feat: new feature",
Avatar: "https://avatars0.githubusercontent.com/u/21979?v=3&s=100",
},
Build: Build{
Number: 101,
Status: "success",
Link: "https://github.com/appleboy/go-hello",
Event: "tag",
Event: "push",
},
Source: Source{
Branch: "feature/awesome-feature",
},
Config: Config{
WebhookID: os.Getenv("WEBHOOK_ID"),
WebhookToken: os.Getenv("WEBHOOK_TOKEN"),
Message: []string{"test one message from drone testing", "test two message from drone testing"},
File: []string{"./images/discord-logo.png"},
Drone: true,
},
Payload: Payload{
Username: "drone",
TTS: false,
Wait: false,
Username: "default-bot",
},
}
err := plugin.Exec(context.Background())
assert.Nil(t, err)
plugin.Clear()
plugin.Config.Message = []string{"I am appleboy"}
plugin.Payload.TTS = true
plugin.Payload.Wait = true
err = plugin.Exec(context.Background())
assert.Nil(t, err)
// send success embed message
plugin.Config.Message = []string{}
plugin.Payload.TTS = false
plugin.Payload.Wait = false
plugin.Clear()
err = plugin.Exec(context.Background())
assert.Nil(t, err)
// send success embed message
plugin.Build.Status = "failure"
plugin.Commit.Message = "send failure embed message"
plugin.Clear()
err = plugin.Exec(context.Background())
assert.Nil(t, err)
time.Sleep(1 * time.Second)
// send default embed message
plugin.Build.Status = "test"
plugin.Commit.Message = "send default embed message"
plugin.Clear()
err = plugin.Exec(context.Background())
assert.Nil(t, err)
// change color for embed message
plugin.Config.Color = "#4842f4"
plugin.Commit.Message = "Change embed color to #4842f4"
plugin.Clear()
err = plugin.Exec(context.Background())
assert.Nil(t, err)
assert.NoError(t, err)
}
func TestDefaultTemplate(t *testing.T) {
func TestSendFile(t *testing.T) {
// Create a dummy file for testing
dummyFile, err := os.Create("test_file.txt")
assert.NoError(t, err)
_, err = dummyFile.WriteString("This is a test file.")
assert.NoError(t, err)
dummyFile.Close()
defer os.Remove("test_file.txt")
plugin := Plugin{
Config: Config{
WebhookID: os.Getenv("WEBHOOK_ID"),
WebhookToken: os.Getenv("WEBHOOK_TOKEN"),
Message: []string{"default message 1", "default message 2"},
Color: "#48f442",
File: []string{"test_file.txt"},
},
Payload: Payload{
Username: "drone-ci",
TTS: false,
Wait: false,
Username: "file-bot",
},
}
time.Sleep(1 * time.Second)
plugin.Clear()
err := plugin.Exec(context.Background())
assert.Nil(t, err)
plugin.Config.Color = "#f4be41"
time.Sleep(1 * time.Second)
plugin.Clear()
err = plugin.Exec(context.Background())
assert.Nil(t, err)
assert.NoError(t, err)
}
func TestColorConversion(t *testing.T) {
tests := []struct {
name string
colorHex string
expectedInt int
buildStatus string
expectedFall int
}{
{"valid hex", "#ffaa00", 16755200, "success", 1752220},
{"invalid hex", "not-a-hex", 0, "failure", 16724530},
{"status success", "", 0, "success", 1752220},
{"status failure", "", 0, "failure", 16724530},
{"status killed", "", 0, "killed", 16724530},
{"status default", "", 0, "running", 16767280},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := Plugin{
Config: Config{Color: tt.colorHex},
Build: Build{Status: tt.buildStatus},
}
if tt.colorHex != "" {
assert.Equal(t, tt.expectedInt, p.Color())
} else {
assert.Equal(t, tt.expectedFall, p.Color())
}
})
}
}
func TestExecWithAllFeatures(t *testing.T) {
time.Sleep(1 * time.Second)
// Create a dummy file for testing
dummyFile, err := os.Create("test_all.txt")
assert.NoError(t, err)
_, err = dummyFile.WriteString("This is a test file for a combined test.")
assert.NoError(t, err)
dummyFile.Close()
defer os.Remove("test_all.txt")
plugin := Plugin{
Repo: Repo{
Name: "go-hello",
Namespace: "appleboy",
},
Commit: Commit{
Author: "appleboy",
Message: "Combined test with multiple features",
},
Build: Build{
Status: "success",
Link: "http://example.com",
},
Config: Config{
WebhookID: os.Getenv("WEBHOOK_ID"),
WebhookToken: os.Getenv("WEBHOOK_TOKEN"),
Message: []string{"First line of embed.", "Second line."},
File: []string{"test_all.txt"},
Color: "#32a852",
Drone: true,
},
Payload: Payload{
Username: "super-bot",
},
}
err = plugin.Exec(context.Background())
assert.NoError(t, err)
}