feat: Support entire 2xx HTTP status range for webhooks (#15)

* feat: Support entire 2xx HTTP status range for webhook to be considered successful * refactor: use constants for http status check * chore: Group tests * chore: Remove unused variable to make golangcilint happy
go-pkgz · Sep 27, 2024 · a5912bd · a5912bd
1 parent e4af176
commit a5912bd
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 48 deletions.
diff --git a/webhook.go b/webhook.go
@@ -69,7 +69,7 @@ func (wh *Webhook) Send(ctx context.Context, destination, text string) error {
 	}
 	defer resp.Body.Close()
 
-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
 		errMsg := fmt.Sprintf("webhook request failed with non-OK status code: %d", resp.StatusCode)
 		respBody, e := io.ReadAll(resp.Body)
 		if e != nil {

diff --git a/webhook_test.go b/webhook_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"testing"
@@ -18,70 +19,104 @@ func (c funcWebhookClient) Do(r *http.Request) (*http.Response, error) {
 	return c(r)
 }
 
-var okWebhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
-	return &http.Response{
-		StatusCode: http.StatusOK,
-		Body:       io.NopCloser(bytes.NewBufferString("ok")),
-	}, nil
-})
-
 type errReader struct {
 }
 
 func (errReader) Read(_ []byte) (n int, err error) {
 	return 0, errors.New("test error")
 }
 
+func assertNoErrorWithStatus(t *testing.T, wh *Webhook, status int) {
+	t.Run(fmt.Sprintf("HTTP-Status %d", status), func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
+			return &http.Response{
+				StatusCode: status,
+				Body:       io.NopCloser(errReader{}),
+			}, nil
+		})
+		err := wh.Send(context.Background(), "http:/example.org/url", "")
+		assert.NoError(t, err)
+	})
+}
+
+func assertErrorWithStatus(t *testing.T, wh *Webhook, status int) {
+	t.Run(fmt.Sprintf("HTTP-Status %d", status), func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
+			return &http.Response{
+				StatusCode: status,
+				Body:       io.NopCloser(errReader{}),
+			}, nil
+		})
+		err := wh.Send(context.Background(), "http:/example.org/url", "")
+		assert.Error(t, err)
+	})
+}
+
 func TestWebhook_Send(t *testing.T) {
 	// empty header to check wrong header handling case
 	wh := NewWebhook(WebhookParams{Headers: []string{"Content-Type:application/json,text/plain", ""}})
-	wh.webhookClient = funcWebhookClient(func(r *http.Request) (*http.Response, error) {
-		assert.Len(t, r.Header, 1)
-		assert.Equal(t, r.Header.Get("Content-Type"), "application/json,text/plain")
-
-		return &http.Response{
-			StatusCode: http.StatusOK,
-			Body:       io.NopCloser(bytes.NewBufferString("")),
-		}, nil
-	})
 	assert.NotNil(t, wh)
 
-	err := wh.Send(context.Background(), "https://example.org/webhook", "some_text")
-	assert.NoError(t, err)
+	t.Run("OK with JSON response", func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(r *http.Request) (*http.Response, error) {
+			assert.Len(t, r.Header, 1)
+			assert.Equal(t, r.Header.Get("Content-Type"), "application/json,text/plain")
+
+			return &http.Response{
+				StatusCode: http.StatusOK,
+				Body:       io.NopCloser(bytes.NewBufferString("")),
+			}, nil
+		})
+		err := wh.Send(context.Background(), "https://example.org/webhook", "some_text")
+		assert.NoError(t, err)
+	})
 
-	wh.webhookClient = okWebhookClient
-	assert.NoError(t, err)
-	err = wh.Send(nil, "https://example.org/webhook", "some_text") //nolint
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "unable to create webhook request")
+	t.Run("No context", func(t *testing.T) {
+		err := wh.Send(nil, "https://example.org/webhook", "some_text") //nolint
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "unable to create webhook request")
+	})
 
-	wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
-		return nil, errors.New("request failed")
+	t.Run("Failed request", func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
+			return nil, errors.New("request failed")
+		})
+		err := wh.Send(context.Background(), "https://not-existing-url.net", "some_text")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "webhook request failed")
 	})
-	err = wh.Send(context.Background(), "https://not-existing-url.net", "some_text")
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "webhook request failed")
-
-	wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
-		return &http.Response{
-			StatusCode: http.StatusNotFound,
-			Body:       io.NopCloser(bytes.NewBufferString("not found")),
-		}, nil
+
+	t.Run("Not found with json response", func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
+			return &http.Response{
+				StatusCode: http.StatusNotFound,
+				Body:       io.NopCloser(bytes.NewBufferString("not found")),
+			}, nil
+		})
+		err := wh.Send(context.Background(), "http:/example.org/invalid-url", "some_text")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "non-OK status code: 404, body: not found")
 	})
-	err = wh.Send(context.Background(), "http:/example.org/invalid-url", "some_text")
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "non-OK status code: 404, body: not found")
-
-	wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
-		return &http.Response{
-			StatusCode: http.StatusNotFound,
-			Body:       io.NopCloser(errReader{}),
-		}, nil
+
+	t.Run("Not found with no response", func(t *testing.T) {
+		wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) {
+			return &http.Response{
+				StatusCode: http.StatusNotFound,
+				Body:       io.NopCloser(errReader{}),
+			}, nil
+		})
+		err := wh.Send(context.Background(), "http:/example.org/invalid-url", "some_text")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "non-OK status code: 404")
+		assert.NotContains(t, err.Error(), "body")
 	})
-	err = wh.Send(context.Background(), "http:/example.org/invalid-url", "some_text")
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "non-OK status code: 404")
-	assert.NotContains(t, err.Error(), "body")
+
+	assertErrorWithStatus(t, wh, http.StatusOK-1)
+	assertNoErrorWithStatus(t, wh, http.StatusOK)
+	assertNoErrorWithStatus(t, wh, http.StatusNoContent)
+	assertNoErrorWithStatus(t, wh, http.StatusMultipleChoices-1)
+	assertErrorWithStatus(t, wh, http.StatusMultipleChoices)
+	assertErrorWithStatus(t, wh, http.StatusMultipleChoices+1)
 }
 
 func TestWebhook_String(t *testing.T) {