From c211275f362f654f3ebc7127a1f43cc84840b46c Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Fri, 9 Jan 2026 17:34:48 +0530 Subject: [PATCH 1/9] initial changes --- docs/advanced-guide/circuit-breaker/page.md | 19 ++- pkg/gofr/service/circuit_breaker.go | 39 ++++- pkg/gofr/service/circuit_breaker_test.go | 153 ++++++++++++++++++++ 3 files changed, 203 insertions(+), 8 deletions(-) diff --git a/docs/advanced-guide/circuit-breaker/page.md b/docs/advanced-guide/circuit-breaker/page.md index 7811092b3f..fcad241204 100644 --- a/docs/advanced-guide/circuit-breaker/page.md +++ b/docs/advanced-guide/circuit-breaker/page.md @@ -16,7 +16,15 @@ The circuit breaker tracks consecutive failed requests for a downstream service. -- **Interval:** Once the circuit is open, GoFr starts a background goroutine that periodically checks the health of the service by making requests to its aliveness endpoint (by default: /.well-known/alive) at the specified interval. When the service is deemed healthy again, the circuit breaker closes, allowing requests to resume. +- **Interval:** Once the circuit is open, GoFr starts a background goroutine that periodically checks the health of the service by making requests to its aliveness endpoint at the specified interval. When the service is deemed healthy again, the circuit breaker closes, allowing requests to resume. + + + +- **HealthEndpoint:** (Optional) A custom health endpoint to use for circuit breaker recovery checks. By default, GoFr uses `/.well-known/alive`. If the downstream service doesn't expose this endpoint, you can specify a custom endpoint (e.g., `health`, `status`, or any valid endpoint that returns HTTP 200 when the service is healthy). + + + +- **HealthTimeout:** (Optional) The timeout in seconds for health check requests. Defaults to 5 seconds if not specified. @@ -41,8 +49,13 @@ func main() { &service.CircuitBreakerConfig{ // Number of consecutive failed requests after which circuit breaker will be enabled Threshold: 4, - // Time interval at which circuit breaker will hit the aliveness endpoint. + // Time interval at which circuit breaker will hit the health endpoint. Interval: 1 * time.Second, + // Custom health endpoint for circuit breaker recovery (optional) + // Use this when the downstream service doesn't expose /.well-known/alive + HealthEndpoint: "health", + // Timeout for health check requests in seconds (optional, defaults to 5) + HealthTimeout: 10, }, ) @@ -54,6 +67,6 @@ func main() { ``` Circuit breaker state changes to open when number of consecutive failed requests increases the threshold. -When it is in open state, GoFr makes request to the aliveness endpoint (default being - /.well-known/alive) at an equal interval of time provided in config. +When it is in open state, GoFr makes request to the health endpoint (default being - /.well-known/alive, or the custom endpoint if configured) at an equal interval of time provided in config. > ##### Check out the example of an inter-service HTTP communication along with circuit-breaker in GoFr: [Visit GitHub](https://github.com/gofr-dev/gofr/blob/main/examples/using-http-service/main.go) \ No newline at end of file diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index f78d9e0b11..3f323ff508 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -24,6 +24,13 @@ var ( type CircuitBreakerConfig struct { Threshold int // Threshold represents the max no of retry before switching the circuit breaker state. Interval time.Duration // Interval represents the time interval duration between hitting the HealthURL + + // HealthEndpoint is the custom endpoint to use for health checks during circuit recovery. + // If not provided, the circuit breaker will use the default /.well-known/alive endpoint. + HealthEndpoint string + // HealthTimeout is the timeout in seconds for health check requests. + // If not provided, defaults to 5 seconds. + HealthTimeout int } // circuitBreaker represents a circuit breaker implementation. @@ -35,6 +42,11 @@ type circuitBreaker struct { interval time.Duration lastChecked time.Time + // healthEndpoint is the custom endpoint to use for health checks during circuit recovery. + healthEndpoint string + // healthTimeout is the timeout in seconds for health check requests. + healthTimeout int + HTTP } @@ -42,11 +54,18 @@ type circuitBreaker struct { // //nolint:revive // Allow returning unexported types as intended. func NewCircuitBreaker(config CircuitBreakerConfig, h HTTP) *circuitBreaker { + healthTimeout := config.HealthTimeout + if healthTimeout == 0 { + healthTimeout = defaultTimeout + } + cb := &circuitBreaker{ - state: ClosedState, - threshold: config.Threshold, - interval: config.Interval, - HTTP: h, + state: ClosedState, + threshold: config.Threshold, + interval: config.Interval, + healthEndpoint: config.HealthEndpoint, + healthTimeout: healthTimeout, + HTTP: h, } // Perform asynchronous health checks @@ -97,8 +116,18 @@ func (cb *circuitBreaker) isOpen() bool { } // healthCheck performs the health check for the circuit breaker. +// If a custom healthEndpoint is configured, it uses that endpoint. +// Otherwise, it falls back to the default /.well-known/alive endpoint. func (cb *circuitBreaker) healthCheck(ctx context.Context) bool { - resp := cb.HealthCheck(ctx) + var resp *Health + + if cb.healthEndpoint != "" { + // Use the custom health endpoint configured in CircuitBreakerConfig + resp = cb.HTTP.getHealthResponseForEndpoint(ctx, cb.healthEndpoint, cb.healthTimeout) + } else { + // Fall back to the default health check (/.well-known/alive) + resp = cb.HTTP.HealthCheck(ctx) + } return resp.Status == serviceUp } diff --git a/pkg/gofr/service/circuit_breaker_test.go b/pkg/gofr/service/circuit_breaker_test.go index dfb2868531..1eb8b33cb1 100644 --- a/pkg/gofr/service/circuit_breaker_test.go +++ b/pkg/gofr/service/circuit_breaker_test.go @@ -606,3 +606,156 @@ func (*customTransport) RoundTrip(r *http.Request) (*http.Response, error) { return nil, testutil.CustomError{ErrorMessage: "cb error"} } + +// customHealthEndpointTransport simulates a service that doesn't have /.well-known/alive +// but has a custom health endpoint like /health or /breeds. +type customHealthEndpointTransport struct { + healthEndpoint string +} + +func (t *customHealthEndpointTransport) RoundTrip(r *http.Request) (*http.Response, error) { + // Custom health endpoint returns OK + if r.URL.Path == "/"+t.healthEndpoint || r.URL.Path == "/success" { + return &http.Response{ + Body: io.NopCloser(bytes.NewBufferString("OK")), + StatusCode: http.StatusOK, + Request: r, + }, nil + } + + // Default /.well-known/alive returns 404 (simulating service without GoFr default endpoint) + if r.URL.Path == "/.well-known/alive" { + return &http.Response{ + Body: io.NopCloser(bytes.NewBufferString("Not Found")), + StatusCode: http.StatusNotFound, + Request: r, + }, nil + } + + return nil, testutil.CustomError{ErrorMessage: "cb error"} +} + +func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { + server := testServer() + defer server.Close() + + mockMetric := &mockMetrics{} + mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + + // Initialize HTTP service with custom transport that only responds to custom health endpoint + service := httpService{ + Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, + url: server.URL, + Tracer: otel.Tracer("gofr-http-client"), + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + // Circuit breaker configuration with custom health endpoint + cbConfig := CircuitBreakerConfig{ + Threshold: 1, + Interval: 1, + HealthEndpoint: "breeds", // Custom endpoint instead of /.well-known/alive + } + + httpService := cbConfig.AddOption(&service) + + // First request fails - circuit opens + _, err := httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + + // Second request fails - circuit is now open + _, err = httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + assert.Equal(t, ErrCircuitOpen, err) + + // Third request should succeed as circuit recovers using custom health endpoint + resp, err := httpService.Get(t.Context(), "success", nil) + require.NoError(t, err) + assert.NotNil(t, resp) + _ = resp.Body.Close() +} + +func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T) { + server := testServer() + defer server.Close() + + mockMetric := &mockMetrics{} + mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + + // Initialize HTTP service with custom transport that doesn't have /.well-known/alive + service := httpService{ + Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, + url: server.URL, + Tracer: otel.Tracer("gofr-http-client"), + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + // Circuit breaker configuration WITHOUT custom health endpoint + cbConfig := CircuitBreakerConfig{ + Threshold: 1, + Interval: 1, + // HealthEndpoint not set - will use default /.well-known/alive which returns 404 + } + + httpService := cbConfig.AddOption(&service) + + // First request fails - circuit opens + _, err := httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + + // Second request fails - circuit is now open + _, err = httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + assert.Equal(t, ErrCircuitOpen, err) + + // Third request should also fail - circuit cannot recover because /.well-known/alive returns 404 + _, err = httpService.Get(t.Context(), "success", nil) + require.Error(t, err) + assert.Equal(t, ErrCircuitOpen, err) +} + +func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { + server := testServer() + defer server.Close() + + mockMetric := &mockMetrics{} + mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + + service := httpService{ + Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "health"}}, + url: server.URL, + Tracer: otel.Tracer("gofr-http-client"), + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + // Circuit breaker configuration with custom health endpoint and timeout + cbConfig := CircuitBreakerConfig{ + Threshold: 1, + Interval: 1, + HealthEndpoint: "health", + HealthTimeout: 10, // 10 seconds timeout + } + + httpService := cbConfig.AddOption(&service) + + // First request fails - circuit opens + _, err := httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + + // Second request fails - circuit is now open + _, err = httpService.Get(t.Context(), "invalid", nil) + require.Error(t, err) + + // Circuit should recover using custom health endpoint + resp, err := httpService.Get(t.Context(), "success", nil) + require.NoError(t, err) + assert.NotNil(t, resp) + _ = resp.Body.Close() +} + From 1ec039024cbfdf15fcc0c13bbf57f378f19ef71f Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 12:33:48 +0530 Subject: [PATCH 2/9] update implementation --- pkg/gofr/service/circuit_breaker.go | 34 ++++----- pkg/gofr/service/circuit_breaker_test.go | 97 ++++++++++++++++-------- pkg/gofr/service/health_config.go | 23 ++++++ 3 files changed, 103 insertions(+), 51 deletions(-) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index d2f6ceecf0..f57ee543ce 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -24,13 +24,6 @@ var ( type CircuitBreakerConfig struct { Threshold int // Threshold represents the max no of retry before switching the circuit breaker state. Interval time.Duration // Interval represents the time interval duration between hitting the HealthURL - - // HealthEndpoint is the custom endpoint to use for health checks during circuit recovery. - // If not provided, the circuit breaker will use the default /.well-known/alive endpoint. - HealthEndpoint string - // HealthTimeout is the timeout in seconds for health check requests. - // If not provided, defaults to 5 seconds. - HealthTimeout int } // circuitBreaker represents a circuit breaker implementation. @@ -56,18 +49,12 @@ type circuitBreaker struct { // //nolint:revive // Allow returning unexported types as intended. func NewCircuitBreaker(config CircuitBreakerConfig, h HTTP) *circuitBreaker { - healthTimeout := config.HealthTimeout - if healthTimeout == 0 { - healthTimeout = defaultTimeout - } - cb := &circuitBreaker{ - state: ClosedState, - threshold: config.Threshold, - interval: config.Interval, - healthEndpoint: config.HealthEndpoint, - healthTimeout: healthTimeout, - HTTP: h, + state: ClosedState, + threshold: config.Threshold, + interval: config.Interval, + healthTimeout: defaultTimeout, + HTTP: h, } // Perform asynchronous health checks @@ -185,6 +172,17 @@ func (cb *circuitBreaker) resetFailureCount() { cb.failureCount = 0 } +// setHealthConfig updates the circuit breaker's health endpoint and timeout. +// This is called by HealthConfig.AddOption when it wraps a circuit breaker, +// ensuring the circuit breaker uses the same health endpoint for recovery checks. +func (cb *circuitBreaker) setHealthConfig(endpoint string, timeout int) { + cb.mu.Lock() + defer cb.mu.Unlock() + + cb.healthEndpoint = endpoint + cb.healthTimeout = timeout +} + func (cb *CircuitBreakerConfig) AddOption(h HTTP) HTTP { circuitBreaker := NewCircuitBreaker(*cb, h) diff --git a/pkg/gofr/service/circuit_breaker_test.go b/pkg/gofr/service/circuit_breaker_test.go index d4aa4fc307..8be9f90de8 100644 --- a/pkg/gofr/service/circuit_breaker_test.go +++ b/pkg/gofr/service/circuit_breaker_test.go @@ -771,39 +771,52 @@ func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { server := testServer() defer server.Close() - mockMetric := &mockMetrics{} - mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(nil) + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().RecordHistogram(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewCounter(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // Initialize HTTP service with custom transport that only responds to custom health endpoint - service := httpService{ + svc := httpService{ Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, url: server.URL, + name: "test-service", Tracer: otel.Tracer("gofr-http-client"), Logger: logging.NewMockLogger(logging.DEBUG), Metrics: mockMetric, } - // Circuit breaker configuration with custom health endpoint + // Circuit breaker configuration cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - HealthEndpoint: "breeds", // Custom endpoint instead of /.well-known/alive + Threshold: 1, + Interval: 1, + } + + // Health config with custom endpoint - this automatically updates the circuit breaker + healthConfig := HealthConfig{ + HealthEndpoint: "breeds", } - httpService := cbConfig.AddOption(&service) + // Apply circuit breaker first, then health config + httpSvc := cbConfig.AddOption(&svc) + httpSvc = healthConfig.AddOption(httpSvc) // First request fails - circuit opens - _, err := httpService.Get(t.Context(), "invalid", nil) + resp, err := httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) + require.NoError(t, resp.Body.Close()) // Second request fails - circuit is now open - _, err = httpService.Get(t.Context(), "invalid", nil) + resp, err = httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) assert.Equal(t, ErrCircuitOpen, err) + require.NoError(t, resp.Body.Close()) // Third request should succeed as circuit recovers using custom health endpoint - resp, err := httpService.Get(t.Context(), "success", nil) + resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) assert.NotNil(t, resp) _ = resp.Body.Close() @@ -813,12 +826,16 @@ func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T server := testServer() defer server.Close() - mockMetric := &mockMetrics{} - mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(nil) + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().RecordHistogram(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewCounter(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // Initialize HTTP service with custom transport that doesn't have /.well-known/alive - service := httpService{ + svc := httpService{ Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, url: server.URL, Tracer: otel.Tracer("gofr-http-client"), @@ -833,32 +850,39 @@ func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T // HealthEndpoint not set - will use default /.well-known/alive which returns 404 } - httpService := cbConfig.AddOption(&service) + httpSvc := cbConfig.AddOption(&svc) // First request fails - circuit opens - _, err := httpService.Get(t.Context(), "invalid", nil) + resp, err := httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) + require.NoError(t, resp.Body.Close()) // Second request fails - circuit is now open - _, err = httpService.Get(t.Context(), "invalid", nil) + resp, err = httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) assert.Equal(t, ErrCircuitOpen, err) + require.NoError(t, resp.Body.Close()) // Third request should also fail - circuit cannot recover because /.well-known/alive returns 404 - _, err = httpService.Get(t.Context(), "success", nil) + resp, err = httpSvc.Get(t.Context(), "success", nil) require.Error(t, err) assert.Equal(t, ErrCircuitOpen, err) + require.NoError(t, resp.Body.Close()) } func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { server := testServer() defer server.Close() - mockMetric := &mockMetrics{} - mockMetric.On("RecordHistogram", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(nil) + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) - service := httpService{ + mockMetric.EXPECT().RecordHistogram(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewCounter(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + + svc := httpService{ Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "health"}}, url: server.URL, Tracer: otel.Tracer("gofr-http-client"), @@ -866,28 +890,35 @@ func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { Metrics: mockMetric, } - // Circuit breaker configuration with custom health endpoint and timeout + // Circuit breaker configuration cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, + Threshold: 1, + Interval: 1, + } + + // Health config with custom endpoint and timeout + healthConfig := HealthConfig{ HealthEndpoint: "health", - HealthTimeout: 10, // 10 seconds timeout + Timeout: 10, // 10 seconds timeout } - httpService := cbConfig.AddOption(&service) + // Apply circuit breaker first, then health config + httpSvc := cbConfig.AddOption(&svc) + httpSvc = healthConfig.AddOption(httpSvc) // First request fails - circuit opens - _, err := httpService.Get(t.Context(), "invalid", nil) + resp, err := httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) + require.NoError(t, resp.Body.Close()) // Second request fails - circuit is now open - _, err = httpService.Get(t.Context(), "invalid", nil) + resp, err = httpSvc.Get(t.Context(), "invalid", nil) require.Error(t, err) + require.NoError(t, resp.Body.Close()) // Circuit should recover using custom health endpoint - resp, err := httpService.Get(t.Context(), "success", nil) + resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) assert.NotNil(t, resp) _ = resp.Body.Close() } - diff --git a/pkg/gofr/service/health_config.go b/pkg/gofr/service/health_config.go index 23c920db85..ad6ed7512c 100644 --- a/pkg/gofr/service/health_config.go +++ b/pkg/gofr/service/health_config.go @@ -13,6 +13,10 @@ func (h *HealthConfig) AddOption(svc HTTP) HTTP { h.Timeout = defaultTimeout } + // If the service chain contains a circuit breaker, update it to use this health endpoint + // This ensures the circuit breaker uses the same health endpoint for recovery checks + updateCircuitBreakerHealthConfig(svc, h.HealthEndpoint, h.Timeout) + return &customHealthService{ healthEndpoint: h.HealthEndpoint, timeout: h.Timeout, @@ -20,6 +24,25 @@ func (h *HealthConfig) AddOption(svc HTTP) HTTP { } } +// updateCircuitBreakerHealthConfig traverses the HTTP service chain to ensure that when a HealthConfig is applied, the circuit breaker +// automatically uses the same health endpoint for its recovery checks. +func updateCircuitBreakerHealthConfig(h HTTP, endpoint string, timeout int) { + switch v := h.(type) { + case *circuitBreaker: + v.setHealthConfig(endpoint, timeout) + case *retryProvider: + updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) + case *authProvider: + updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) + case *rateLimiter: + updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) + case *customHeader: + updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) + case *customHealthService: + updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) + } +} + type customHealthService struct { healthEndpoint string timeout int From dfcb30c3a69e875dd37286cfe2ba77957f3050e6 Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 13:28:33 +0530 Subject: [PATCH 3/9] minor refactorations and fixes --- docs/advanced-guide/circuit-breaker/page.md | 5 - pkg/gofr/service/circuit_breaker.go | 6 +- pkg/gofr/service/circuit_breaker_test.go | 226 +++++++++----------- 3 files changed, 106 insertions(+), 131 deletions(-) diff --git a/docs/advanced-guide/circuit-breaker/page.md b/docs/advanced-guide/circuit-breaker/page.md index 629eebe0fd..2a546dee60 100644 --- a/docs/advanced-guide/circuit-breaker/page.md +++ b/docs/advanced-guide/circuit-breaker/page.md @@ -74,11 +74,6 @@ func main() { Threshold: 4, // Time interval at which circuit breaker will hit the health endpoint. Interval: 1 * time.Second, - // Custom health endpoint for circuit breaker recovery (optional) - // Use this when the downstream service doesn't expose /.well-known/alive - HealthEndpoint: "health", - // Timeout for health check requests in seconds (optional, defaults to 5) - HealthTimeout: 10, }, ) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index f57ee543ce..d9f50d8b4d 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -82,7 +82,7 @@ func (cb *circuitBreaker) executeWithCircuitBreaker(ctx context.Context, f func( } result, err := f(ctx) - if err != nil || (result != nil && result.StatusCode > 500) { + if err != nil || (result != nil && result.StatusCode >= 500) { cb.handleFailure() } else { cb.resetFailureCount() @@ -111,10 +111,8 @@ func (cb *circuitBreaker) healthCheck(ctx context.Context) bool { var resp *Health if cb.healthEndpoint != "" { - // Use the custom health endpoint configured in CircuitBreakerConfig resp = cb.HTTP.getHealthResponseForEndpoint(ctx, cb.healthEndpoint, cb.healthTimeout) } else { - // Fall back to the default health check (/.well-known/alive) resp = cb.HTTP.HealthCheck(ctx) } @@ -173,8 +171,6 @@ func (cb *circuitBreaker) resetFailureCount() { } // setHealthConfig updates the circuit breaker's health endpoint and timeout. -// This is called by HealthConfig.AddOption when it wraps a circuit breaker, -// ensuring the circuit breaker uses the same health endpoint for recovery checks. func (cb *circuitBreaker) setHealthConfig(endpoint string, timeout int) { cb.mu.Lock() defer cb.mu.Unlock() diff --git a/pkg/gofr/service/circuit_breaker_test.go b/pkg/gofr/service/circuit_breaker_test.go index 8be9f90de8..93a859049c 100644 --- a/pkg/gofr/service/circuit_breaker_test.go +++ b/pkg/gofr/service/circuit_breaker_test.go @@ -739,36 +739,18 @@ func (*customTransport) RoundTrip(r *http.Request) (*http.Response, error) { return nil, testutil.CustomError{ErrorMessage: "cb error"} } -// customHealthEndpointTransport simulates a service that doesn't have /.well-known/alive -// but has a custom health endpoint like /health or /breeds. -type customHealthEndpointTransport struct { - healthEndpoint string -} - -func (t *customHealthEndpointTransport) RoundTrip(r *http.Request) (*http.Response, error) { - // Custom health endpoint returns OK - if r.URL.Path == "/"+t.healthEndpoint || r.URL.Path == "/success" { - return &http.Response{ - Body: io.NopCloser(bytes.NewBufferString("OK")), - StatusCode: http.StatusOK, - Request: r, - }, nil - } - - // Default /.well-known/alive returns 404 (simulating service without GoFr default endpoint) - if r.URL.Path == "/.well-known/alive" { - return &http.Response{ - Body: io.NopCloser(bytes.NewBufferString("Not Found")), - StatusCode: http.StatusNotFound, - Request: r, - }, nil - } - - return nil, testutil.CustomError{ErrorMessage: "cb error"} -} - func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail (triggers circuit breaker), 200 for /health and /success + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/health": + w.WriteHeader(http.StatusOK) + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -779,51 +761,53 @@ func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - // Initialize HTTP service with custom transport that only responds to custom health endpoint - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, - url: server.URL, - name: "test-service", - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - // Circuit breaker configuration - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - } - - // Health config with custom endpoint - this automatically updates the circuit breaker - healthConfig := HealthConfig{ - HealthEndpoint: "breeds", - } + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + &HealthConfig{HealthEndpoint: "health", Timeout: 5}, + ) - // Apply circuit breaker first, then health config - httpSvc := cbConfig.AddOption(&svc) - httpSvc = healthConfig.AddOption(httpSvc) + // First request returns 502 - failure count becomes 1, threshold not exceeded (1 > 1 is false) + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) + // Second request returns 502 - failure count becomes 2, exceeds threshold (2 > 1) + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) + // Third request - circuit is still open, returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Third request should succeed as circuit recovers using custom health endpoint + // Wait for interval to pass so circuit can attempt recovery via /health endpoint + time.Sleep(500 * time.Millisecond) + + // Fourth request - circuit should recover via /health endpoint and succeed resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) - assert.NotNil(t, resp) - _ = resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() } func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail (triggers circuit breaker), + // 404 for /.well-known/alive (default health endpoint missing) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/alive": + w.WriteHeader(http.StatusNotFound) // Default health endpoint not available + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -834,44 +818,51 @@ func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - // Initialize HTTP service with custom transport that doesn't have /.well-known/alive - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, - url: server.URL, - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - // Circuit breaker configuration WITHOUT custom health endpoint - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - // HealthEndpoint not set - will use default /.well-known/alive which returns 404 - } + // No HealthConfig - will use default /.well-known/alive which returns 404 + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + ) - httpSvc := cbConfig.AddOption(&svc) + // First request returns 502 - failure count becomes 1, threshold not exceeded + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) + // Second request returns 502 - failure count becomes 2, exceeds threshold + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) + // Third request - circuit is still open + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Third request should also fail - circuit cannot recover because /.well-known/alive returns 404 + // Wait for interval to pass + time.Sleep(500 * time.Millisecond) + + // Fourth request should also fail - circuit cannot recover because /.well-known/alive returns 404 resp, err = httpSvc.Get(t.Context(), "success", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) } func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail, 200 for /health and other paths + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/health": + w.WriteHeader(http.StatusOK) + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -882,43 +873,36 @@ func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "health"}}, - url: server.URL, - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - // Circuit breaker configuration - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - } - - // Health config with custom endpoint and timeout - healthConfig := HealthConfig{ - HealthEndpoint: "health", - Timeout: 10, // 10 seconds timeout - } + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + &HealthConfig{HealthEndpoint: "health", Timeout: 10}, + ) - // Apply circuit breaker first, then health config - httpSvc := cbConfig.AddOption(&svc) - httpSvc = healthConfig.AddOption(httpSvc) + // First request returns 502 - failure count becomes 1, threshold not exceeded + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) + // Second request returns 502 - failure count becomes 2, exceeds threshold + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) + // Third request - circuit is still open + resp, err = httpSvc.Get(t.Context(), "fail", nil) + require.ErrorIs(t, err, ErrCircuitOpen) require.NoError(t, resp.Body.Close()) - // Circuit should recover using custom health endpoint + // Wait for interval to pass so circuit can attempt recovery + time.Sleep(500 * time.Millisecond) + + // Fourth request - circuit should recover using custom health endpoint resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) - assert.NotNil(t, resp) - _ = resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() } From 05e3e5bf8f2a19cdda46491b8ea3dd37db09ab65 Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 13:28:33 +0530 Subject: [PATCH 4/9] fix tests --- docs/advanced-guide/circuit-breaker/page.md | 5 - pkg/gofr/service/circuit_breaker.go | 6 +- pkg/gofr/service/circuit_breaker_test.go | 245 ++++++++++---------- pkg/gofr/service/health_config_test.go | 193 +++++++++++++++ 4 files changed, 319 insertions(+), 130 deletions(-) create mode 100644 pkg/gofr/service/health_config_test.go diff --git a/docs/advanced-guide/circuit-breaker/page.md b/docs/advanced-guide/circuit-breaker/page.md index 629eebe0fd..2a546dee60 100644 --- a/docs/advanced-guide/circuit-breaker/page.md +++ b/docs/advanced-guide/circuit-breaker/page.md @@ -74,11 +74,6 @@ func main() { Threshold: 4, // Time interval at which circuit breaker will hit the health endpoint. Interval: 1 * time.Second, - // Custom health endpoint for circuit breaker recovery (optional) - // Use this when the downstream service doesn't expose /.well-known/alive - HealthEndpoint: "health", - // Timeout for health check requests in seconds (optional, defaults to 5) - HealthTimeout: 10, }, ) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index f57ee543ce..d9f50d8b4d 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -82,7 +82,7 @@ func (cb *circuitBreaker) executeWithCircuitBreaker(ctx context.Context, f func( } result, err := f(ctx) - if err != nil || (result != nil && result.StatusCode > 500) { + if err != nil || (result != nil && result.StatusCode >= 500) { cb.handleFailure() } else { cb.resetFailureCount() @@ -111,10 +111,8 @@ func (cb *circuitBreaker) healthCheck(ctx context.Context) bool { var resp *Health if cb.healthEndpoint != "" { - // Use the custom health endpoint configured in CircuitBreakerConfig resp = cb.HTTP.getHealthResponseForEndpoint(ctx, cb.healthEndpoint, cb.healthTimeout) } else { - // Fall back to the default health check (/.well-known/alive) resp = cb.HTTP.HealthCheck(ctx) } @@ -173,8 +171,6 @@ func (cb *circuitBreaker) resetFailureCount() { } // setHealthConfig updates the circuit breaker's health endpoint and timeout. -// This is called by HealthConfig.AddOption when it wraps a circuit breaker, -// ensuring the circuit breaker uses the same health endpoint for recovery checks. func (cb *circuitBreaker) setHealthConfig(endpoint string, timeout int) { cb.mu.Lock() defer cb.mu.Unlock() diff --git a/pkg/gofr/service/circuit_breaker_test.go b/pkg/gofr/service/circuit_breaker_test.go index 8be9f90de8..a3d7721702 100644 --- a/pkg/gofr/service/circuit_breaker_test.go +++ b/pkg/gofr/service/circuit_breaker_test.go @@ -739,36 +739,18 @@ func (*customTransport) RoundTrip(r *http.Request) (*http.Response, error) { return nil, testutil.CustomError{ErrorMessage: "cb error"} } -// customHealthEndpointTransport simulates a service that doesn't have /.well-known/alive -// but has a custom health endpoint like /health or /breeds. -type customHealthEndpointTransport struct { - healthEndpoint string -} - -func (t *customHealthEndpointTransport) RoundTrip(r *http.Request) (*http.Response, error) { - // Custom health endpoint returns OK - if r.URL.Path == "/"+t.healthEndpoint || r.URL.Path == "/success" { - return &http.Response{ - Body: io.NopCloser(bytes.NewBufferString("OK")), - StatusCode: http.StatusOK, - Request: r, - }, nil - } - - // Default /.well-known/alive returns 404 (simulating service without GoFr default endpoint) - if r.URL.Path == "/.well-known/alive" { - return &http.Response{ - Body: io.NopCloser(bytes.NewBufferString("Not Found")), - StatusCode: http.StatusNotFound, - Request: r, - }, nil - } - - return nil, testutil.CustomError{ErrorMessage: "cb error"} -} - func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail (triggers circuit breaker), 200 for /health and /success + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/health": + w.WriteHeader(http.StatusOK) + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -779,51 +761,59 @@ func TestCircuitBreaker_CustomHealthEndpoint_Recovery(t *testing.T) { mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - // Initialize HTTP service with custom transport that only responds to custom health endpoint - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, - url: server.URL, - name: "test-service", - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + &HealthConfig{HealthEndpoint: "health", Timeout: 5}, + ) - // Circuit breaker configuration - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - } + // First request returns 502 - failure count becomes 1, threshold not exceeded (1 > 1 is false) + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() - // Health config with custom endpoint - this automatically updates the circuit breaker - healthConfig := HealthConfig{ - HealthEndpoint: "breeds", + // Second request returns 502 - failure count becomes 2, exceeds threshold (2 > 1) + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() } - // Apply circuit breaker first, then health config - httpSvc := cbConfig.AddOption(&svc) - httpSvc = healthConfig.AddOption(httpSvc) + require.ErrorIs(t, err, ErrCircuitOpen) - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - require.NoError(t, resp.Body.Close()) + // Third request - circuit is still open, returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() + } - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) - require.NoError(t, resp.Body.Close()) + require.ErrorIs(t, err, ErrCircuitOpen) - // Third request should succeed as circuit recovers using custom health endpoint + // Wait for interval to pass so circuit can attempt recovery via /health endpoint + time.Sleep(1 * time.Second) + + // Fourth request - circuit should recover via /health endpoint and succeed resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) - assert.NotNil(t, resp) - _ = resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() } func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail (triggers circuit breaker), + // 404 for /.well-known/alive (default health endpoint missing) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/alive": + w.WriteHeader(http.StatusNotFound) // Default health endpoint not available + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -834,44 +824,60 @@ func TestCircuitBreaker_DefaultHealthEndpoint_NoRecoveryWhenMissing(t *testing.T mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - // Initialize HTTP service with custom transport that doesn't have /.well-known/alive - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "breeds"}}, - url: server.URL, - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + // No HealthConfig - will use default /.well-known/alive which returns 404 + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + ) - // Circuit breaker configuration WITHOUT custom health endpoint - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - // HealthEndpoint not set - will use default /.well-known/alive which returns 404 + // First request returns 502 - failure count becomes 1, threshold not exceeded + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() + + // Second request returns 502 - failure count becomes 2, exceeds threshold + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() } - httpSvc := cbConfig.AddOption(&svc) + require.ErrorIs(t, err, ErrCircuitOpen) + + // Third request - circuit is still open + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() + } - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - require.NoError(t, resp.Body.Close()) + require.ErrorIs(t, err, ErrCircuitOpen) - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) - require.NoError(t, resp.Body.Close()) + // Wait for interval to pass + time.Sleep(500 * time.Millisecond) - // Third request should also fail - circuit cannot recover because /.well-known/alive returns 404 + // Fourth request should also fail - circuit cannot recover because /.well-known/alive returns 404 resp, err = httpSvc.Get(t.Context(), "success", nil) - require.Error(t, err) - assert.Equal(t, ErrCircuitOpen, err) - require.NoError(t, resp.Body.Close()) + if resp != nil && resp.Body != nil { + resp.Body.Close() + } + + require.ErrorIs(t, err, ErrCircuitOpen) } func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { - server := testServer() + // Server that returns 502 for /fail, 200 for /health and other paths + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/health": + w.WriteHeader(http.StatusOK) + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() ctrl := gomock.NewController(t) @@ -882,43 +888,42 @@ func TestCircuitBreaker_HealthEndpointWithTimeout(t *testing.T) { mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - svc := httpService{ - Client: &http.Client{Transport: &customHealthEndpointTransport{healthEndpoint: "health"}}, - url: server.URL, - Tracer: otel.Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + httpSvc := NewHTTPService(server.URL, + logging.NewMockLogger(logging.DEBUG), + mockMetric, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + &HealthConfig{HealthEndpoint: "health", Timeout: 10}, + ) - // Circuit breaker configuration - cbConfig := CircuitBreakerConfig{ - Threshold: 1, - Interval: 1, - } + // First request returns 502 - failure count becomes 1, threshold not exceeded + resp, err := httpSvc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() - // Health config with custom endpoint and timeout - healthConfig := HealthConfig{ - HealthEndpoint: "health", - Timeout: 10, // 10 seconds timeout + // Second request returns 502 - failure count becomes 2, exceeds threshold + // Circuit opens and returns ErrCircuitOpen + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() } - // Apply circuit breaker first, then health config - httpSvc := cbConfig.AddOption(&svc) - httpSvc = healthConfig.AddOption(httpSvc) + require.ErrorIs(t, err, ErrCircuitOpen) + + // Third request - circuit is still open + resp, err = httpSvc.Get(t.Context(), "fail", nil) + if resp != nil && resp.Body != nil { + resp.Body.Close() + } - // First request fails - circuit opens - resp, err := httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - require.NoError(t, resp.Body.Close()) + require.ErrorIs(t, err, ErrCircuitOpen) - // Second request fails - circuit is now open - resp, err = httpSvc.Get(t.Context(), "invalid", nil) - require.Error(t, err) - require.NoError(t, resp.Body.Close()) + // Wait for interval to pass so circuit can attempt recovery + time.Sleep(500 * time.Millisecond) - // Circuit should recover using custom health endpoint + // Fourth request - circuit should recover using custom health endpoint resp, err = httpSvc.Get(t.Context(), "success", nil) require.NoError(t, err) - assert.NotNil(t, resp) - _ = resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() } diff --git a/pkg/gofr/service/health_config_test.go b/pkg/gofr/service/health_config_test.go new file mode 100644 index 0000000000..206387bea1 --- /dev/null +++ b/pkg/gofr/service/health_config_test.go @@ -0,0 +1,193 @@ +package service + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + + "gofr.dev/pkg/gofr/logging" +) + +func TestUpdateCircuitBreakerHealthConfig_DirectCircuitBreaker(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + + updateCircuitBreakerHealthConfig(cb, "custom-health", 15) + + assert.Equal(t, "custom-health", cb.healthEndpoint) + assert.Equal(t, 15, cb.healthTimeout) +} + +func TestUpdateCircuitBreakerHealthConfig_ThroughRetryProvider(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + retry := &retryProvider{HTTP: cb, maxRetries: 3} + + updateCircuitBreakerHealthConfig(retry, "health-endpoint", 20) + + assert.Equal(t, "health-endpoint", cb.healthEndpoint) + assert.Equal(t, 20, cb.healthTimeout) +} + +func TestUpdateCircuitBreakerHealthConfig_ThroughRateLimiter(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + rl := &rateLimiter{HTTP: cb} + + updateCircuitBreakerHealthConfig(rl, "status", 30) + + assert.Equal(t, "status", cb.healthEndpoint) + assert.Equal(t, 30, cb.healthTimeout) +} + +func TestUpdateCircuitBreakerHealthConfig_ThroughAuthProvider(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + auth := &authProvider{HTTP: cb} + + updateCircuitBreakerHealthConfig(auth, "ping", 25) + + assert.Equal(t, "ping", cb.healthEndpoint) + assert.Equal(t, 25, cb.healthTimeout) +} + +func TestUpdateCircuitBreakerHealthConfig_ThroughCustomHeader(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + ch := &customHeader{HTTP: cb} + + updateCircuitBreakerHealthConfig(ch, "ready", 10) + + assert.Equal(t, "ready", cb.healthEndpoint) + assert.Equal(t, 10, cb.healthTimeout) +} + +func TestHealthConfig_AddOption_UpdatesCircuitBreaker(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + + healthConfig := HealthConfig{ + HealthEndpoint: "breeds", + Timeout: 10, + } + + result := healthConfig.AddOption(cb) + + // Verify circuit breaker was updated with health config + assert.Equal(t, "breeds", cb.healthEndpoint) + assert.Equal(t, 10, cb.healthTimeout) + + // Verify result is a customHealthService wrapping the circuit breaker + customHealth, ok := result.(*customHealthService) + assert.True(t, ok) + assert.Equal(t, "breeds", customHealth.healthEndpoint) + assert.Equal(t, 10, customHealth.timeout) +} + +func TestHealthConfig_AddOption_DefaultTimeout(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + + // HealthConfig without explicit timeout + healthConfig := HealthConfig{ + HealthEndpoint: "health", + } + + result := healthConfig.AddOption(cb) + + // Verify default timeout (5) is used + assert.Equal(t, "health", cb.healthEndpoint) + assert.Equal(t, defaultTimeout, cb.healthTimeout) + + customHealth, ok := result.(*customHealthService) + assert.True(t, ok) + assert.Equal(t, defaultTimeout, customHealth.timeout) +} + +func TestUpdateCircuitBreakerHealthConfig_DeepNestedChain(t *testing.T) { + ctrl := gomock.NewController(t) + mockMetric := NewMockMetrics(ctrl) + + mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + + svc := &httpService{ + Logger: logging.NewMockLogger(logging.DEBUG), + Metrics: mockMetric, + } + + // Create a deeply nested chain: customHeader -> rateLimiter -> retryProvider -> circuitBreaker -> httpService + cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + retry := &retryProvider{HTTP: cb, maxRetries: 3} + rl := &rateLimiter{HTTP: retry} + ch := &customHeader{HTTP: rl} + + updateCircuitBreakerHealthConfig(ch, "deep-health", 42) + + assert.Equal(t, "deep-health", cb.healthEndpoint) + assert.Equal(t, 42, cb.healthTimeout) +} From ffd5ada13c8982f8968cd10e7ac8581cac208ef8 Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 14:36:44 +0530 Subject: [PATCH 5/9] resolve linters --- pkg/gofr/service/circuit_breaker.go | 4 - pkg/gofr/service/health_config_test.go | 177 +++++++++---------------- pkg/gofr/service/metrics_helper.go | 12 -- 3 files changed, 66 insertions(+), 127 deletions(-) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index d9f50d8b4d..a028924a6b 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -187,10 +187,6 @@ func (cb *CircuitBreakerConfig) AddOption(h HTTP) HTTP { circuitBreaker.serviceName = httpSvc.name if circuitBreaker.metrics != nil { - registerGauge(circuitBreaker.metrics, "app_http_circuit_breaker_state", - "Current state of the circuit breaker (0 for Closed, 1 for Open)") - - // Initialize the gauge to 0 (Closed) circuitBreaker.metrics.SetGauge("app_http_circuit_breaker_state", 0, "service", circuitBreaker.serviceName) } } diff --git a/pkg/gofr/service/health_config_test.go b/pkg/gofr/service/health_config_test.go index 206387bea1..feeec7b1eb 100644 --- a/pkg/gofr/service/health_config_test.go +++ b/pkg/gofr/service/health_config_test.go @@ -1,139 +1,96 @@ package service import ( + "net/http/httptest" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "gofr.dev/pkg/gofr/logging" ) -func TestUpdateCircuitBreakerHealthConfig_DirectCircuitBreaker(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) - - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() - - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) +func setupMockMetrics(t *testing.T) *MockMetrics { + t.Helper() - updateCircuitBreakerHealthConfig(cb, "custom-health", 15) - - assert.Equal(t, "custom-health", cb.healthEndpoint) - assert.Equal(t, 15, cb.healthTimeout) -} - -func TestUpdateCircuitBreakerHealthConfig_ThroughRetryProvider(t *testing.T) { ctrl := gomock.NewController(t) mockMetric := NewMockMetrics(ctrl) + mockMetric.EXPECT().RecordHistogram(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().NewCounter(gomock.Any(), gomock.Any()).AnyTimes() mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric.EXPECT().SetGauge(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) - retry := &retryProvider{HTTP: cb, maxRetries: 3} - - updateCircuitBreakerHealthConfig(retry, "health-endpoint", 20) - - assert.Equal(t, "health-endpoint", cb.healthEndpoint) - assert.Equal(t, 20, cb.healthTimeout) + return mockMetric } -func TestUpdateCircuitBreakerHealthConfig_ThroughRateLimiter(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) +func TestUpdateCircuitBreakerHealthConfig_DirectCircuitBreaker(t *testing.T) { + server := httptest.NewServer(nil) + defer server.Close() - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric := setupMockMetrics(t) - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, + &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + ) - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) - rl := &rateLimiter{HTTP: cb} + cb, ok := svc.(*circuitBreaker) + require.True(t, ok) - updateCircuitBreakerHealthConfig(rl, "status", 30) + updateCircuitBreakerHealthConfig(cb, "custom-health", 15) - assert.Equal(t, "status", cb.healthEndpoint) - assert.Equal(t, 30, cb.healthTimeout) + assert.Equal(t, "custom-health", cb.healthEndpoint) + assert.Equal(t, 15, cb.healthTimeout) } -func TestUpdateCircuitBreakerHealthConfig_ThroughAuthProvider(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) - - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() - - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } - - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) - auth := &authProvider{HTTP: cb} - - updateCircuitBreakerHealthConfig(auth, "ping", 25) - - assert.Equal(t, "ping", cb.healthEndpoint) - assert.Equal(t, 25, cb.healthTimeout) -} +func TestUpdateCircuitBreakerHealthConfig_ThroughRetryProvider(t *testing.T) { + server := httptest.NewServer(nil) + defer server.Close() -func TestUpdateCircuitBreakerHealthConfig_ThroughCustomHeader(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) + mockMetric := setupMockMetrics(t) - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, + &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + &RetryConfig{MaxRetries: 3}, + ) - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + retry, ok := svc.(*retryProvider) + require.True(t, ok) - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) - ch := &customHeader{HTTP: cb} + cb, ok := retry.HTTP.(*circuitBreaker) + require.True(t, ok) - updateCircuitBreakerHealthConfig(ch, "ready", 10) + updateCircuitBreakerHealthConfig(svc, "health-endpoint", 20) - assert.Equal(t, "ready", cb.healthEndpoint) - assert.Equal(t, 10, cb.healthTimeout) + assert.Equal(t, "health-endpoint", cb.healthEndpoint) + assert.Equal(t, 20, cb.healthTimeout) } func TestHealthConfig_AddOption_UpdatesCircuitBreaker(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) + server := httptest.NewServer(nil) + defer server.Close() - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric := setupMockMetrics(t) - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, + &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + ) - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + cb, ok := svc.(*circuitBreaker) + require.True(t, ok) healthConfig := HealthConfig{ HealthEndpoint: "breeds", Timeout: 10, } - result := healthConfig.AddOption(cb) + result := healthConfig.AddOption(svc) - // Verify circuit breaker was updated with health config assert.Equal(t, "breeds", cb.healthEndpoint) assert.Equal(t, 10, cb.healthTimeout) - // Verify result is a customHealthService wrapping the circuit breaker customHealth, ok := result.(*customHealthService) assert.True(t, ok) assert.Equal(t, "breeds", customHealth.healthEndpoint) @@ -141,26 +98,24 @@ func TestHealthConfig_AddOption_UpdatesCircuitBreaker(t *testing.T) { } func TestHealthConfig_AddOption_DefaultTimeout(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) + server := httptest.NewServer(nil) + defer server.Close() - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric := setupMockMetrics(t) - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, + &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + ) - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) + cb, ok := svc.(*circuitBreaker) + require.True(t, ok) - // HealthConfig without explicit timeout healthConfig := HealthConfig{ HealthEndpoint: "health", } - result := healthConfig.AddOption(cb) + result := healthConfig.AddOption(svc) - // Verify default timeout (5) is used assert.Equal(t, "health", cb.healthEndpoint) assert.Equal(t, defaultTimeout, cb.healthTimeout) @@ -170,23 +125,23 @@ func TestHealthConfig_AddOption_DefaultTimeout(t *testing.T) { } func TestUpdateCircuitBreakerHealthConfig_DeepNestedChain(t *testing.T) { - ctrl := gomock.NewController(t) - mockMetric := NewMockMetrics(ctrl) + server := httptest.NewServer(nil) + defer server.Close() - mockMetric.EXPECT().NewGauge(gomock.Any(), gomock.Any()).AnyTimes() + mockMetric := setupMockMetrics(t) - svc := &httpService{ - Logger: logging.NewMockLogger(logging.DEBUG), - Metrics: mockMetric, - } + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, + &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + &RetryConfig{MaxRetries: 3}, + ) + + retry, ok := svc.(*retryProvider) + require.True(t, ok) - // Create a deeply nested chain: customHeader -> rateLimiter -> retryProvider -> circuitBreaker -> httpService - cb := NewCircuitBreaker(CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, svc) - retry := &retryProvider{HTTP: cb, maxRetries: 3} - rl := &rateLimiter{HTTP: retry} - ch := &customHeader{HTTP: rl} + cb, ok := retry.HTTP.(*circuitBreaker) + require.True(t, ok) - updateCircuitBreakerHealthConfig(ch, "deep-health", 42) + updateCircuitBreakerHealthConfig(svc, "deep-health", 42) assert.Equal(t, "deep-health", cb.healthEndpoint) assert.Equal(t, 42, cb.healthTimeout) diff --git a/pkg/gofr/service/metrics_helper.go b/pkg/gofr/service/metrics_helper.go index f9d1bde887..3d9be03816 100644 --- a/pkg/gofr/service/metrics_helper.go +++ b/pkg/gofr/service/metrics_helper.go @@ -20,15 +20,3 @@ func registerCounter(m Metrics, name, desc string) { m.NewCounter(name, desc) registeredMetrics[name] = true } - -func registerGauge(m Metrics, name, desc string) { - metricsMu.Lock() - defer metricsMu.Unlock() - - if registeredMetrics[name] { - return - } - - m.NewGauge(name, desc) - registeredMetrics[name] = true -} From 4cfb689e39f3123e5591aefb39613ffd0b52cef6 Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 15:28:26 +0530 Subject: [PATCH 6/9] remove file metrics_healper.go --- pkg/gofr/service/metrics_helper.go | 22 ---------------------- pkg/gofr/service/retry.go | 4 ---- 2 files changed, 26 deletions(-) delete mode 100644 pkg/gofr/service/metrics_helper.go diff --git a/pkg/gofr/service/metrics_helper.go b/pkg/gofr/service/metrics_helper.go deleted file mode 100644 index 3d9be03816..0000000000 --- a/pkg/gofr/service/metrics_helper.go +++ /dev/null @@ -1,22 +0,0 @@ -package service - -import "sync" - -var ( - //nolint:gochecknoglobals // Global map to track registered metrics - registeredMetrics = make(map[string]bool) - //nolint:gochecknoglobals // Mutex to protect the global map - metricsMu sync.Mutex -) - -func registerCounter(m Metrics, name, desc string) { - metricsMu.Lock() - defer metricsMu.Unlock() - - if registeredMetrics[name] { - return - } - - m.NewCounter(name, desc) - registeredMetrics[name] = true -} diff --git a/pkg/gofr/service/retry.go b/pkg/gofr/service/retry.go index 03f57731ce..b3387b28c3 100644 --- a/pkg/gofr/service/retry.go +++ b/pkg/gofr/service/retry.go @@ -18,10 +18,6 @@ func (r *RetryConfig) AddOption(h HTTP) HTTP { if httpSvc := extractHTTPService(h); httpSvc != nil { rp.metrics = httpSvc.Metrics rp.serviceName = httpSvc.name - - if rp.metrics != nil { - registerCounter(rp.metrics, "app_http_retry_count", "Total number of retry events") - } } return rp From 7bed3df631fbe84940e9b621430687c1f1c707ac Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Mon, 12 Jan 2026 16:00:53 +0530 Subject: [PATCH 7/9] resolve review comments --- pkg/gofr/service/circuit_breaker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index a028924a6b..63a7504be4 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -82,7 +82,7 @@ func (cb *circuitBreaker) executeWithCircuitBreaker(ctx context.Context, f func( } result, err := f(ctx) - if err != nil || (result != nil && result.StatusCode >= 500) { + if err != nil || (result != nil && result.StatusCode > 500) { cb.handleFailure() } else { cb.resetFailureCount() From 02de51aa760e6aaea1130ff165beed44d523a9dd Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Fri, 16 Jan 2026 12:20:38 +0530 Subject: [PATCH 8/9] refactor: move health config to httpService parent for proper encapsulation and decoupling options --- pkg/gofr/service/circuit_breaker.go | 32 ++--- pkg/gofr/service/health_config.go | 27 +--- pkg/gofr/service/health_config_test.go | 183 ++++++++++++++++++------- pkg/gofr/service/new.go | 5 + pkg/gofr/service/new_test.go | 12 +- 5 files changed, 155 insertions(+), 104 deletions(-) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index 63a7504be4..3dccf3f94c 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -37,11 +37,6 @@ type circuitBreaker struct { metrics Metrics serviceName string - // healthEndpoint is the custom endpoint to use for health checks during circuit recovery. - healthEndpoint string - // healthTimeout is the timeout in seconds for health check requests. - healthTimeout int - HTTP } @@ -50,11 +45,10 @@ type circuitBreaker struct { //nolint:revive // Allow returning unexported types as intended. func NewCircuitBreaker(config CircuitBreakerConfig, h HTTP) *circuitBreaker { cb := &circuitBreaker{ - state: ClosedState, - threshold: config.Threshold, - interval: config.Interval, - healthTimeout: defaultTimeout, - HTTP: h, + state: ClosedState, + threshold: config.Threshold, + interval: config.Interval, + HTTP: h, } // Perform asynchronous health checks @@ -104,14 +98,12 @@ func (cb *circuitBreaker) isOpen() bool { return cb.state == OpenState } -// healthCheck performs the health check for the circuit breaker. -// If a custom healthEndpoint is configured, it uses that endpoint. -// Otherwise, it falls back to the default /.well-known/alive endpoint. func (cb *circuitBreaker) healthCheck(ctx context.Context) bool { var resp *Health - if cb.healthEndpoint != "" { - resp = cb.HTTP.getHealthResponseForEndpoint(ctx, cb.healthEndpoint, cb.healthTimeout) + // Read health config from parent httpService if available + if httpSvc := extractHTTPService(cb.HTTP); httpSvc != nil && httpSvc.healthEndpoint != "" { + resp = cb.HTTP.getHealthResponseForEndpoint(ctx, httpSvc.healthEndpoint, httpSvc.healthTimeout) } else { resp = cb.HTTP.HealthCheck(ctx) } @@ -170,15 +162,6 @@ func (cb *circuitBreaker) resetFailureCount() { cb.failureCount = 0 } -// setHealthConfig updates the circuit breaker's health endpoint and timeout. -func (cb *circuitBreaker) setHealthConfig(endpoint string, timeout int) { - cb.mu.Lock() - defer cb.mu.Unlock() - - cb.healthEndpoint = endpoint - cb.healthTimeout = timeout -} - func (cb *CircuitBreakerConfig) AddOption(h HTTP) HTTP { circuitBreaker := NewCircuitBreaker(*cb, h) @@ -187,6 +170,7 @@ func (cb *CircuitBreakerConfig) AddOption(h HTTP) HTTP { circuitBreaker.serviceName = httpSvc.name if circuitBreaker.metrics != nil { + // Initialize the gauge to 0 (Closed) - gauge is already registered in container.go circuitBreaker.metrics.SetGauge("app_http_circuit_breaker_state", 0, "service", circuitBreaker.serviceName) } } diff --git a/pkg/gofr/service/health_config.go b/pkg/gofr/service/health_config.go index ad6ed7512c..d775772bd5 100644 --- a/pkg/gofr/service/health_config.go +++ b/pkg/gofr/service/health_config.go @@ -13,9 +13,11 @@ func (h *HealthConfig) AddOption(svc HTTP) HTTP { h.Timeout = defaultTimeout } - // If the service chain contains a circuit breaker, update it to use this health endpoint - // This ensures the circuit breaker uses the same health endpoint for recovery checks - updateCircuitBreakerHealthConfig(svc, h.HealthEndpoint, h.Timeout) + // Set health config on the parent httpService so other options can access it + if httpSvc := extractHTTPService(svc); httpSvc != nil { + httpSvc.healthEndpoint = h.HealthEndpoint + httpSvc.healthTimeout = h.Timeout + } return &customHealthService{ healthEndpoint: h.HealthEndpoint, @@ -24,25 +26,6 @@ func (h *HealthConfig) AddOption(svc HTTP) HTTP { } } -// updateCircuitBreakerHealthConfig traverses the HTTP service chain to ensure that when a HealthConfig is applied, the circuit breaker -// automatically uses the same health endpoint for its recovery checks. -func updateCircuitBreakerHealthConfig(h HTTP, endpoint string, timeout int) { - switch v := h.(type) { - case *circuitBreaker: - v.setHealthConfig(endpoint, timeout) - case *retryProvider: - updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) - case *authProvider: - updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) - case *rateLimiter: - updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) - case *customHeader: - updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) - case *customHealthService: - updateCircuitBreakerHealthConfig(v.HTTP, endpoint, timeout) - } -} - type customHealthService struct { healthEndpoint string timeout int diff --git a/pkg/gofr/service/health_config_test.go b/pkg/gofr/service/health_config_test.go index feeec7b1eb..cfede6f248 100644 --- a/pkg/gofr/service/health_config_test.go +++ b/pkg/gofr/service/health_config_test.go @@ -1,6 +1,7 @@ package service import ( + "net/http" "net/http/httptest" "testing" "time" @@ -26,7 +27,7 @@ func setupMockMetrics(t *testing.T) *MockMetrics { return mockMetric } -func TestUpdateCircuitBreakerHealthConfig_DirectCircuitBreaker(t *testing.T) { +func TestHealthConfig_AddOption_SetsParentHealthEndpoint(t *testing.T) { server := httptest.NewServer(nil) defer server.Close() @@ -36,113 +37,189 @@ func TestUpdateCircuitBreakerHealthConfig_DirectCircuitBreaker(t *testing.T) { &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, ) - cb, ok := svc.(*circuitBreaker) - require.True(t, ok) + healthConfig := HealthConfig{ + HealthEndpoint: "breeds", + Timeout: 10, + } - updateCircuitBreakerHealthConfig(cb, "custom-health", 15) + result := healthConfig.AddOption(svc) + + // Verify httpService parent has health config set + httpSvc := extractHTTPService(svc) + require.NotNil(t, httpSvc) + assert.Equal(t, "breeds", httpSvc.healthEndpoint) + assert.Equal(t, 10, httpSvc.healthTimeout) - assert.Equal(t, "custom-health", cb.healthEndpoint) - assert.Equal(t, 15, cb.healthTimeout) + // Verify customHealthService is returned + customHealth, ok := result.(*customHealthService) + assert.True(t, ok) + assert.Equal(t, "breeds", customHealth.healthEndpoint) + assert.Equal(t, 10, customHealth.timeout) } -func TestUpdateCircuitBreakerHealthConfig_ThroughRetryProvider(t *testing.T) { +func TestHealthConfig_AddOption_DefaultTimeout(t *testing.T) { server := httptest.NewServer(nil) defer server.Close() mockMetric := setupMockMetrics(t) - svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, - &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, - &RetryConfig{MaxRetries: 3}, - ) + svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric) - retry, ok := svc.(*retryProvider) - require.True(t, ok) + healthConfig := HealthConfig{ + HealthEndpoint: "health", + // Timeout not set - should use default + } - cb, ok := retry.HTTP.(*circuitBreaker) - require.True(t, ok) + result := healthConfig.AddOption(svc) - updateCircuitBreakerHealthConfig(svc, "health-endpoint", 20) + // Verify default timeout is used + httpSvc := extractHTTPService(svc) + require.NotNil(t, httpSvc) + assert.Equal(t, "health", httpSvc.healthEndpoint) + assert.Equal(t, defaultTimeout, httpSvc.healthTimeout) - assert.Equal(t, "health-endpoint", cb.healthEndpoint) - assert.Equal(t, 20, cb.healthTimeout) + customHealth, ok := result.(*customHealthService) + assert.True(t, ok) + assert.Equal(t, defaultTimeout, customHealth.timeout) } -func TestHealthConfig_AddOption_UpdatesCircuitBreaker(t *testing.T) { +func TestHealthConfig_AddOption_WithRetryAndCircuitBreaker(t *testing.T) { server := httptest.NewServer(nil) defer server.Close() mockMetric := setupMockMetrics(t) + // Create service with circuit breaker and retry svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + &RetryConfig{MaxRetries: 3}, ) - cb, ok := svc.(*circuitBreaker) - require.True(t, ok) - healthConfig := HealthConfig{ - HealthEndpoint: "breeds", - Timeout: 10, + HealthEndpoint: "status", + Timeout: 15, } result := healthConfig.AddOption(svc) - assert.Equal(t, "breeds", cb.healthEndpoint) - assert.Equal(t, 10, cb.healthTimeout) + // Verify httpService parent has health config set + httpSvc := extractHTTPService(svc) + require.NotNil(t, httpSvc) + assert.Equal(t, "status", httpSvc.healthEndpoint) + assert.Equal(t, 15, httpSvc.healthTimeout) + // Verify customHealthService wraps the chain customHealth, ok := result.(*customHealthService) assert.True(t, ok) - assert.Equal(t, "breeds", customHealth.healthEndpoint) - assert.Equal(t, 10, customHealth.timeout) + assert.Equal(t, "status", customHealth.healthEndpoint) } -func TestHealthConfig_AddOption_DefaultTimeout(t *testing.T) { - server := httptest.NewServer(nil) +func TestCircuitBreaker_UsesParentHealthEndpoint(t *testing.T) { + // Server that returns 502 for /fail, 200 for /custom-health + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/custom-health": + w.WriteHeader(http.StatusOK) + case "/fail": + w.WriteHeader(http.StatusBadGateway) + case "/.well-known/alive": + w.WriteHeader(http.StatusNotFound) // Default endpoint not available + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() mockMetric := setupMockMetrics(t) + // Create service with circuit breaker AND health config svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, - &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, + &HealthConfig{HealthEndpoint: "custom-health", Timeout: 5}, ) - cb, ok := svc.(*circuitBreaker) - require.True(t, ok) - - healthConfig := HealthConfig{ - HealthEndpoint: "health", + // First request returns 502 - failure count becomes 1 + resp, err := svc.Get(t.Context(), "fail", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + resp.Body.Close() + + // Second request - circuit opens and returns ErrCircuitOpen + resp, err = svc.Get(t.Context(), "fail", nil) + if err != nil { + require.ErrorIs(t, err, ErrCircuitOpen) + return } - result := healthConfig.AddOption(svc) + defer resp.Body.Close() - assert.Equal(t, "health", cb.healthEndpoint) - assert.Equal(t, defaultTimeout, cb.healthTimeout) + require.ErrorIs(t, err, ErrCircuitOpen) - customHealth, ok := result.(*customHealthService) - assert.True(t, ok) - assert.Equal(t, defaultTimeout, customHealth.timeout) + // Wait for interval to pass + time.Sleep(500 * time.Millisecond) + + // Circuit should recover using /custom-health (from parent httpService) + resp, err = svc.Get(t.Context(), "success", nil) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() } -func TestUpdateCircuitBreakerHealthConfig_DeepNestedChain(t *testing.T) { - server := httptest.NewServer(nil) +func TestCircuitBreaker_UsesDefaultHealthEndpoint_WhenNoHealthConfig(t *testing.T) { + // Server where default health endpoint returns 404 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/alive": + w.WriteHeader(http.StatusNotFound) // Default endpoint not available + case "/fail": + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusOK) + } + })) defer server.Close() mockMetric := setupMockMetrics(t) + // Create service with circuit breaker but NO health config svc := NewHTTPService(server.URL, logging.NewMockLogger(logging.DEBUG), mockMetric, - &CircuitBreakerConfig{Threshold: 3, Interval: time.Second}, - &RetryConfig{MaxRetries: 3}, + &CircuitBreakerConfig{Threshold: 1, Interval: 200 * time.Millisecond}, ) - retry, ok := svc.(*retryProvider) - require.True(t, ok) + // First request returns 502 + resp, err := svc.Get(t.Context(), "fail", nil) + if err != nil { + require.ErrorIs(t, err, ErrCircuitOpen) + return + } + + defer resp.Body.Close() + + require.NoError(t, err) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) - cb, ok := retry.HTTP.(*circuitBreaker) - require.True(t, ok) + // Second request - circuit opens + resp, err = svc.Get(t.Context(), "fail", nil) + if err != nil { + require.ErrorIs(t, err, ErrCircuitOpen) + return + } + + defer resp.Body.Close() + + require.ErrorIs(t, err, ErrCircuitOpen) + + // Wait for interval + time.Sleep(500 * time.Millisecond) + + // Circuit should NOT recover because /.well-known/alive returns 404 + resp, err = svc.Get(t.Context(), "success", nil) + if err != nil { + require.Error(t, err) + return + } - updateCircuitBreakerHealthConfig(svc, "deep-health", 42) + defer resp.Body.Close() - assert.Equal(t, "deep-health", cb.healthEndpoint) - assert.Equal(t, 42, cb.healthTimeout) + require.ErrorIs(t, err, ErrCircuitOpen) } diff --git a/pkg/gofr/service/new.go b/pkg/gofr/service/new.go index 88a093ba6b..7881a8fd78 100644 --- a/pkg/gofr/service/new.go +++ b/pkg/gofr/service/new.go @@ -22,6 +22,11 @@ type httpService struct { name string Logger Metrics + + // healthEndpoint is the custom endpoint for health checks (shared across options) + healthEndpoint string + // healthTimeout is the timeout in seconds for health check requests + healthTimeout int } type HTTP interface { diff --git a/pkg/gofr/service/new_test.go b/pkg/gofr/service/new_test.go index 9f05171e3d..a739482af6 100644 --- a/pkg/gofr/service/new_test.go +++ b/pkg/gofr/service/new_test.go @@ -87,11 +87,13 @@ func TestHTTPService_createAndSendRequest(t *testing.T) { })) service := &httpService{ - Client: http.DefaultClient, - url: server.URL, - Tracer: trace.NewTracerProvider().Tracer("gofr-http-client"), - Logger: logging.NewMockLogger(logging.INFO), - Metrics: metrics, + Client: http.DefaultClient, + url: server.URL, + Tracer: trace.NewTracerProvider().Tracer("gofr-http-client"), + Logger: logging.NewMockLogger(logging.INFO), + Metrics: metrics, + healthEndpoint: "", + healthTimeout: 0, } metrics.EXPECT().RecordHistogram(gomock.Any(), "app_http_service_response", gomock.Any(), "path", server.URL, From 204125544ddf592fd3e18fce05a36976f896c0e3 Mon Sep 17 00:00:00 2001 From: Umang01-hash Date: Fri, 16 Jan 2026 13:41:56 +0530 Subject: [PATCH 9/9] refactor circuit breaker healthCheck --- pkg/gofr/service/circuit_breaker.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/gofr/service/circuit_breaker.go b/pkg/gofr/service/circuit_breaker.go index 3dccf3f94c..0d3eec791c 100644 --- a/pkg/gofr/service/circuit_breaker.go +++ b/pkg/gofr/service/circuit_breaker.go @@ -99,15 +99,14 @@ func (cb *circuitBreaker) isOpen() bool { } func (cb *circuitBreaker) healthCheck(ctx context.Context) bool { - var resp *Health - - // Read health config from parent httpService if available if httpSvc := extractHTTPService(cb.HTTP); httpSvc != nil && httpSvc.healthEndpoint != "" { - resp = cb.HTTP.getHealthResponseForEndpoint(ctx, httpSvc.healthEndpoint, httpSvc.healthTimeout) - } else { - resp = cb.HTTP.HealthCheck(ctx) + resp := cb.HTTP.getHealthResponseForEndpoint(ctx, httpSvc.healthEndpoint, httpSvc.healthTimeout) + + return resp.Status == serviceUp } + resp := cb.HTTP.HealthCheck(ctx) + return resp.Status == serviceUp }