From a5912bd674bee0552b581715c5606f8c82935a55 Mon Sep 17 00:00:00 2001 From: Timo Reymann Date: Fri, 27 Sep 2024 23:35:09 +0200 Subject: [PATCH] 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 --- webhook.go | 2 +- webhook_test.go | 129 ++++++++++++++++++++++++++++++------------------ 2 files changed, 83 insertions(+), 48 deletions(-) diff --git a/webhook.go b/webhook.go index 6cdffee..1f26027 100644 --- 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 index 4db9ce9..53ef9c5 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "net/http" "testing" @@ -18,13 +19,6 @@ 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 { } @@ -32,56 +26,97 @@ 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) {