From 6495835c9bd78f3607cb3b3700aa254364d1e473 Mon Sep 17 00:00:00 2001 From: Timo Reymann Date: Tue, 10 Sep 2024 12:46:44 +0200 Subject: [PATCH 1/4] feat: Support entire 2xx HTTP status range for webhook to be considered successful --- webhook.go | 2 +- webhook_test.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index 6cdffee..5825171 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 < 200 || resp.StatusCode >= 300 { 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..3548920 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -82,6 +82,15 @@ func TestWebhook_Send(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "non-OK status code: 404") assert.NotContains(t, err.Error(), "body") + + wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusNoContent, + Body: io.NopCloser(errReader{}), + }, nil + }) + err = wh.Send(context.Background(), "http:/example.org/no-content-url", "") + assert.NoError(t, err) } func TestWebhook_String(t *testing.T) { From 012dcee585b904c5eff835f1bcfdbd94693b6609 Mon Sep 17 00:00:00 2001 From: Timo Reymann Date: Mon, 23 Sep 2024 11:02:32 +0200 Subject: [PATCH 2/4] refactor: use constants for http status check --- webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index 5825171..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 < 200 || resp.StatusCode >= 300 { + 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 { From e22b0653daac6e72b670ebcb9c5ddd167ebab112 Mon Sep 17 00:00:00 2001 From: Timo Reymann Date: Mon, 23 Sep 2024 11:02:41 +0200 Subject: [PATCH 3/4] chore: Group tests --- webhook_test.go | 131 ++++++++++++++++++++++++++++++------------------ 1 file changed, 82 insertions(+), 49 deletions(-) diff --git a/webhook_test.go b/webhook_test.go index 3548920..92aef20 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "net/http" "testing" @@ -32,65 +33,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) - - 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("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 = funcWebhookClient(func(*http.Request) (*http.Response, error) { - return nil, errors.New("request failed") + 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") }) - 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("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(), "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 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") - assert.NotContains(t, err.Error(), "body") - - wh.webhookClient = funcWebhookClient(func(*http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: http.StatusNoContent, - 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/no-content-url", "") - assert.NoError(t, err) + + 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) { From 9e94596ee135e6b9f60c009cf2aaca74fbc8ae35 Mon Sep 17 00:00:00 2001 From: Timo Reymann Date: Thu, 26 Sep 2024 10:27:06 +0200 Subject: [PATCH 4/4] chore: Remove unused variable to make golangcilint happy --- webhook_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/webhook_test.go b/webhook_test.go index 92aef20..53ef9c5 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -19,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 { }