Merge pull request #9 from music-tribe/alex/add-timeout

Add timeout option to Checker to be used by the health check package
music-tribe · Sep 9, 2024 · 7205559 · 7205559
2 parents ad3e83b + 2488535
commit 7205559
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 2 deletions.
diff --git a/checker.go b/checker.go
@@ -1,6 +1,8 @@
 package healthy
 
 import (
+	"time"
+
 	"github.com/hellofresh/health-go/v5"
 )
 
@@ -9,6 +11,8 @@ const defaultName = "healthcheck"
 type Checker struct {
 	Name      string
 	CheckFunc health.CheckFunc
+	// Timeout defaults to 2 seconds to match health package: https://github.com/hellofresh/health-go/blob/v5.1.0/health.go#L114
+	Timeout time.Duration
 }
 
 func NewChecker(name string, checkFunc health.CheckFunc) Checker {
@@ -21,5 +25,12 @@ func NewChecker(name string, checkFunc health.CheckFunc) Checker {
 	return Checker{
 		Name:      name,
 		CheckFunc: checkFunc,
+		Timeout:   2 * time.Second,
 	}
 }
+
+func NewCheckerWithTimeout(name string, checkFunc health.CheckFunc, timeout time.Duration) Checker {
+	c := NewChecker(name, checkFunc)
+	c.Timeout = timeout
+	return c
+}
diff --git a/checker_test.go b/checker_test.go
@@ -4,9 +4,11 @@ import (
 	"context"
 	errs "errors"
 	"testing"
+	"time"
 
 	"github.com/hellofresh/health-go/v5"
 	"github.com/music-tribe/errors"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestNewChecker(t *testing.T) {
@@ -79,3 +81,16 @@ func TestNewChecker(t *testing.T) {
 		})
 	}
 }
+
+func TestNewCheckWithTimeout(t *testing.T) {
+	t.Parallel()
+	t.Run("When NewChecker is called we should default Timeout to 2 seconds", func(t *testing.T) {
+		c := NewChecker("hello", NewMockChecker(nil))
+		assert.Equal(t, 2*time.Second, c.Timeout)
+	})
+
+	t.Run("When NewCheckerWithTimeout is called we should set Timeout to timeout passed in", func(t *testing.T) {
+		c := NewCheckerWithTimeout("hello", NewMockChecker(nil), 5*time.Second)
+		assert.Equal(t, 5*time.Second, c.Timeout)
+	})
+}
diff --git a/handlers/echo.go b/handlers/echo.go
@@ -14,12 +14,14 @@ func Handler(svc healthy.Service) echo.HandlerFunc {
 	i := 0
 	for name, checker := range svc.Checkers() {
 		opts[i] = health.Config{
-			Name:  name,
-			Check: getCheckFunc(checker),
+			Name:    name,
+			Check:   getCheckFunc(checker),
+			Timeout: checker.Timeout,
 		}
 		i++
 	}
 
+	// FIXME(alex) check the error here
 	h, _ := health.New(health.WithComponent(health.Component{
 		Name:    svc.Name(),
 		Version: svc.Version(),

diff --git a/handlers/echo_test.go b/handlers/echo_test.go
@@ -1,14 +1,20 @@
 package handlers
 
 import (
+	"context"
+	"encoding/json"
 	errs "errors"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
+	"github.com/hellofresh/health-go/v5"
 	"github.com/labstack/echo/v4"
 	"github.com/music-tribe/errors"
 	"github.com/music-tribe/healthy"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -29,6 +35,15 @@ func TestHandler(t *testing.T) {
 		err := h(ctx)
 		require.NoError(t, err)
 
+		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("OK"))
+
 		gotStatusCode := rec.Result().StatusCode
 		if err != nil {
 			ce := new(errors.CloudError)
@@ -58,6 +73,15 @@ func TestHandler(t *testing.T) {
 		err = h(ctx)
 		require.NoError(t, err)
 
+		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
+
 		gotStatusCode := rec.Result().StatusCode
 		if err != nil {
 			ce := new(errors.CloudError)
@@ -93,6 +117,13 @@ func TestHandler(t *testing.T) {
 		require.NoError(t, err)
 
 		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
 
 		gotStatusCode := rec.Result().StatusCode
 		if err != nil {
@@ -129,6 +160,13 @@ func TestHandler(t *testing.T) {
 		require.NoError(t, err)
 
 		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
 
 		gotStatusCode := rec.Result().StatusCode
 		if err != nil {
@@ -155,6 +193,93 @@ func TestHandler(t *testing.T) {
 		err := h(ctx)
 		require.NoError(t, err)
 
+		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
+
+		gotStatusCode := rec.Result().StatusCode
+		if err != nil {
+			ce := new(errors.CloudError)
+			if errs.As(err, &ce) {
+				gotStatusCode = ce.StatusCode
+			}
+		}
+
+		if wantStatusCode != gotStatusCode {
+			t.Errorf("wanted status code to be %d but got %d\n", wantStatusCode, gotStatusCode)
+		}
+	})
+
+	t.Run("When we pass a checker with timeout we should return a StatusTimeout after the timeout expires", func(t *testing.T) {
+		wantStatusCode := 503
+
+		req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
+		rec := httptest.NewRecorder()
+		ctx := e.NewContext(req, rec)
+
+		slowChecker := health.CheckFunc(func(ctx context.Context) error {
+			time.Sleep(time.Second * 2)
+			return nil
+		})
+		hSvc, _ := healthy.New("some-service", "1.1.3", healthy.NewCheckerWithTimeout("mockStore", slowChecker, 1*time.Second))
+		h := Handler(hSvc)
+		err := h(ctx)
+		require.NoError(t, err)
+
+		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
+		assert.Equal(t, healthCheck.Failures["mockStore"], "Timeout during health check")
+
+		gotStatusCode := rec.Result().StatusCode
+		if err != nil {
+			ce := new(errors.CloudError)
+			if errs.As(err, &ce) {
+				gotStatusCode = ce.StatusCode
+			}
+		}
+
+		if wantStatusCode != gotStatusCode {
+			t.Errorf("wanted status code to be %d but got %d\n", wantStatusCode, gotStatusCode)
+		}
+	})
+
+	t.Run("When we pass a checker and use default timeout we should return a StatusTimeout after the default timeout of 2 seconds expires", func(t *testing.T) {
+		wantStatusCode := 503
+
+		req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
+		rec := httptest.NewRecorder()
+		ctx := e.NewContext(req, rec)
+
+		slowChecker := health.CheckFunc(func(ctx context.Context) error {
+			time.Sleep(time.Second * 3)
+			return nil
+		})
+		hSvc, _ := healthy.New("some-service", "1.1.3", healthy.NewChecker("mockStore", slowChecker))
+		h := Handler(hSvc)
+		err := h(ctx)
+		require.NoError(t, err)
+
+		defer rec.Result().Body.Close()
+		body, err := io.ReadAll(rec.Result().Body)
+		require.NoError(t, err)
+
+		healthCheck := health.Check{}
+		err = json.Unmarshal(body, &healthCheck)
+		require.NoError(t, err)
+		assert.Equal(t, healthCheck.Status, health.Status("Unavailable"))
+		assert.Equal(t, healthCheck.Failures["mockStore"], "Timeout during health check")
+
 		gotStatusCode := rec.Result().StatusCode
 		if err != nil {
 			ce := new(errors.CloudError)