fix(remote): avoid replaying non-idempotent requests (#238)

* fix(remote): avoid replaying non-idempotent requests

* fix(remote): classify retryable transport failures

commit: f37fe67a5887e4cfde360a9cf21960427c629a3d
author: bnema · 52d ago · 2026-08-31 10:46 UTC
committer: GitHub · 52d ago · 2026-08-31 10:46 UTC
parent: 30c748c
signature: GPG UnverifiedB5690EEEBB952194 The key is unknown
refs: tag: v2.31.1

2 files · +251 -37

M internal/adapters/in/cli/remote/client.go +68 -7


@@ -266,6 +266,18 @@ func (c Client) request(ctx context.Context, method, path string, body any) (h
266	266		return resp, nil
267	267		}
268	268
	269	+	type requestTransportError struct {
	270	+	err error
	271	+	}
	272	+
	273	+	func (e *requestTransportError) Error() string {
	274	+	return e.err.Error()
	275	+	}
	276	+
	277	+	func (e *requestTransportError) Unwrap() error {
	278	+	return e.err
	279	+	}
	280	+
269	281		// doRequest builds and executes a single HTTP request to the admin API.
270	282		func (c Client) doRequest(ctx context.Context, method, path string, jsonBody []byte) (http.Response, error) {
271	283		reqURL := c.baseURL + "/admin" + path

@@ -293,7 +305,9 @@ func (c *Client) doRequest(ctx context.Context, method, path string, jsonBody []
293	305
294	306		resp, err := c.httpClient.Do(req)
295	307		if err != nil {
296		-	return nil, err
	308	+	return nil, &requestTransportError{
	309	+	err: fmt.Errorf("%s %s transport request: %w", method, path, err),
	310	+	}
297	311		}
298	312
299	313		return resp, nil

@@ -333,6 +347,28 @@ func (e *HTTPError) Error() string {
333	347		return e.Status + ": " + e.Body
334	348		}
335	349
	350	+	// OutcomeUnknownError reports that a non-idempotent request may have completed
	351	+	// even though the client did not receive a definitive response.
	352	+	type OutcomeUnknownError struct {
	353	+	Method string
	354	+	Path string
	355	+	Err error
	356	+	}
	357	+
	358	+	func (e *OutcomeUnknownError) Error() string {
	359	+	return fmt.Sprintf(
	360	+	"%s %s outcome unknown: request may have completed; inspect current state before retrying: %v",
	361	+	e.Method,
	362	+	e.Path,
	363	+	e.Err,
	364	+	)
	365	+	}
	366	+
	367	+	// Unwrap returns the transport or HTTP error that made the outcome ambiguous.
	368	+	func (e *OutcomeUnknownError) Unwrap() error {
	369	+	return e.Err
	370	+	}
	371	+
336	372		func parseErrorResponse(resp *http.Response, body []byte) error {
337	373		msg := string(body)
338	374		structured := false

@@ -360,12 +396,25 @@ func parseErrorResponse(resp *http.Response, body []byte) error {
360	396		}
361	397
362	398		func isRetryableRequestError(err error) bool {
363		-	return !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded)
	399	+	var transportErr *requestTransportError
	400	+	return errors.As(err, &transportErr) &&
	401	+	!errors.Is(err, context.Canceled) &&
	402	+	!errors.Is(err, context.DeadlineExceeded)
	403	+	}
	404	+
	405	+	func isIdempotentMethod(method string) bool {
	406	+	switch method {
	407	+	case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace,
	408	+	http.MethodPut, http.MethodDelete:
	409	+	return true
	410	+	default:
	411	+	return false
	412	+	}
364	413		}
365	414
366	415		func isRetryableStatus(status int) bool {
367		-	// Only retry gateway errors; retrying 500 on non-idempotent POSTs
368		-	// (deploy, restart, reload) can cause duplicate state changes.
	416	+	// Gateway errors can be transient; requestWithRetry separately ensures the
	417	+	// method is safe to replay before attempting another request.
369	418		return status == 502 \|\| status == 503 \|\| status == 504
370	419		}
371	420

@@ -376,14 +425,23 @@ func retryDelay(attempt int) time.Duration {
376	425		return retryBaseDelay * time.Duration(1<<(attempt-1))
377	426		}
378	427
379		-	// requestWithRetry performs an HTTP request and retries transient failures.
380		-	// Retries occur on transport errors and gateway errors (502, 503, 504).
	428	+	// requestWithRetry performs an HTTP request and retries transient failures
	429	+	// only when the HTTP method is idempotent. A transient failure from a
	430	+	// non-idempotent request is returned as OutcomeUnknownError without replaying it.
381	431		func (c Client) requestWithRetry(ctx context.Context, method, path string, body any) (http.Response, error) {
382	432		var lastErr error
	433	+	idempotent := isIdempotentMethod(method)
383	434
384	435		for attempt := 1; attempt <= retryMaxAttempts; attempt++ {
385	436		resp, err := c.request(ctx, method, path, body)
386	437		if err != nil {
	438	+	if !idempotent {
	439	+	var transportErr *requestTransportError
	440	+	if errors.As(err, &transportErr) {
	441	+	return nil, &OutcomeUnknownError{Method: method, Path: path, Err: err}
	442	+	}
	443	+	return nil, err
	444	+	}
387	445		lastErr = err
388	446		if attempt == retryMaxAttempts \|\| !isRetryableRequestError(err) {
389	447		return nil, err

@@ -396,6 +454,9 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, body
396	454		respBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodySize))
397	455		_ = resp.Body.Close()
398	456		lastErr = parseErrorResponse(resp, respBody)
	457	+	if !idempotent {
	458	+	return nil, &OutcomeUnknownError{Method: method, Path: path, Err: lastErr}
	459	+	}
399	460		if attempt == retryMaxAttempts {
400	461		return nil, lastErr
401	462		}

@@ -718,7 +779,7 @@ func (c Client) GetTrafficStatus(ctx context.Context) (dto.TrafficStatusRespon
718	779
719	780		// GetStatus returns the Gordon server status.
720	781		func (c Client) GetStatus(ctx context.Context) (Status, error) {
721		-	resp, err := c.request(ctx, http.MethodGet, "/status", nil)
	782	+	resp, err := c.requestWithRetry(ctx, http.MethodGet, "/status", nil)
722	783		if err != nil {
723	784		return nil, err
724	785		}


@@ -266,6 +266,18 @@ func (c Client) request(ctx context.Context, method, path string, body any) (h
266		return resp, nil	266		return resp, nil
267		}	267		}
268			268
			269	+	type requestTransportError struct {
			270	+	err error
			271	+	}
			272	+
			273	+	func (e *requestTransportError) Error() string {
			274	+	return e.err.Error()
			275	+	}
			276	+
			277	+	func (e *requestTransportError) Unwrap() error {
			278	+	return e.err
			279	+	}
			280	+
269		// doRequest builds and executes a single HTTP request to the admin API.	281		// doRequest builds and executes a single HTTP request to the admin API.
270		func (c Client) doRequest(ctx context.Context, method, path string, jsonBody []byte) (http.Response, error) {	282		func (c Client) doRequest(ctx context.Context, method, path string, jsonBody []byte) (http.Response, error) {
271		reqURL := c.baseURL + "/admin" + path	283		reqURL := c.baseURL + "/admin" + path

@@ -293,7 +305,9 @@ func (c *Client) doRequest(ctx context.Context, method, path string, jsonBody []
293			305
294		resp, err := c.httpClient.Do(req)	306		resp, err := c.httpClient.Do(req)
295		if err != nil {	307		if err != nil {
296	-	return nil, err	308	+	return nil, &requestTransportError{
			309	+	err: fmt.Errorf("%s %s transport request: %w", method, path, err),
			310	+	}
297		}	311		}
298			312
299		return resp, nil	313		return resp, nil

@@ -333,6 +347,28 @@ func (e *HTTPError) Error() string {
333		return e.Status + ": " + e.Body	347		return e.Status + ": " + e.Body
334		}	348		}
335			349
			350	+	// OutcomeUnknownError reports that a non-idempotent request may have completed
			351	+	// even though the client did not receive a definitive response.
			352	+	type OutcomeUnknownError struct {
			353	+	Method string
			354	+	Path string
			355	+	Err error
			356	+	}
			357	+
			358	+	func (e *OutcomeUnknownError) Error() string {
			359	+	return fmt.Sprintf(
			360	+	"%s %s outcome unknown: request may have completed; inspect current state before retrying: %v",
			361	+	e.Method,
			362	+	e.Path,
			363	+	e.Err,
			364	+	)
			365	+	}
			366	+
			367	+	// Unwrap returns the transport or HTTP error that made the outcome ambiguous.
			368	+	func (e *OutcomeUnknownError) Unwrap() error {
			369	+	return e.Err
			370	+	}
			371	+
336		func parseErrorResponse(resp *http.Response, body []byte) error {	372		func parseErrorResponse(resp *http.Response, body []byte) error {
337		msg := string(body)	373		msg := string(body)
338		structured := false	374		structured := false

@@ -360,12 +396,25 @@ func parseErrorResponse(resp *http.Response, body []byte) error {
360		}	396		}
361			397
362		func isRetryableRequestError(err error) bool {	398		func isRetryableRequestError(err error) bool {
363	-	return !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded)	399	+	var transportErr *requestTransportError
			400	+	return errors.As(err, &transportErr) &&
			401	+	!errors.Is(err, context.Canceled) &&
			402	+	!errors.Is(err, context.DeadlineExceeded)
			403	+	}
			404	+
			405	+	func isIdempotentMethod(method string) bool {
			406	+	switch method {
			407	+	case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace,
			408	+	http.MethodPut, http.MethodDelete:
			409	+	return true
			410	+	default:
			411	+	return false
			412	+	}
364		}	413		}
365			414
366		func isRetryableStatus(status int) bool {	415		func isRetryableStatus(status int) bool {
367	-	// Only retry gateway errors; retrying 500 on non-idempotent POSTs	416	+	// Gateway errors can be transient; requestWithRetry separately ensures the
368	-	// (deploy, restart, reload) can cause duplicate state changes.	417	+	// method is safe to replay before attempting another request.
369		return status == 502 \|\| status == 503 \|\| status == 504	418		return status == 502 \|\| status == 503 \|\| status == 504
370		}	419		}
371			420

@@ -376,14 +425,23 @@ func retryDelay(attempt int) time.Duration {
376		return retryBaseDelay * time.Duration(1<<(attempt-1))	425		return retryBaseDelay * time.Duration(1<<(attempt-1))
377		}	426		}
378			427
379	-	// requestWithRetry performs an HTTP request and retries transient failures.	428	+	// requestWithRetry performs an HTTP request and retries transient failures
380	-	// Retries occur on transport errors and gateway errors (502, 503, 504).	429	+	// only when the HTTP method is idempotent. A transient failure from a
			430	+	// non-idempotent request is returned as OutcomeUnknownError without replaying it.
381		func (c Client) requestWithRetry(ctx context.Context, method, path string, body any) (http.Response, error) {	431		func (c Client) requestWithRetry(ctx context.Context, method, path string, body any) (http.Response, error) {
382		var lastErr error	432		var lastErr error
			433	+	idempotent := isIdempotentMethod(method)
383			434
384		for attempt := 1; attempt <= retryMaxAttempts; attempt++ {	435		for attempt := 1; attempt <= retryMaxAttempts; attempt++ {
385		resp, err := c.request(ctx, method, path, body)	436		resp, err := c.request(ctx, method, path, body)
386		if err != nil {	437		if err != nil {
			438	+	if !idempotent {
			439	+	var transportErr *requestTransportError
			440	+	if errors.As(err, &transportErr) {
			441	+	return nil, &OutcomeUnknownError{Method: method, Path: path, Err: err}
			442	+	}
			443	+	return nil, err
			444	+	}
387		lastErr = err	445		lastErr = err
388		if attempt == retryMaxAttempts \|\| !isRetryableRequestError(err) {	446		if attempt == retryMaxAttempts \|\| !isRetryableRequestError(err) {
389		return nil, err	447		return nil, err

@@ -396,6 +454,9 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, body
396		respBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodySize))	454		respBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodySize))
397		_ = resp.Body.Close()	455		_ = resp.Body.Close()
398		lastErr = parseErrorResponse(resp, respBody)	456		lastErr = parseErrorResponse(resp, respBody)
			457	+	if !idempotent {
			458	+	return nil, &OutcomeUnknownError{Method: method, Path: path, Err: lastErr}
			459	+	}
399		if attempt == retryMaxAttempts {	460		if attempt == retryMaxAttempts {
400		return nil, lastErr	461		return nil, lastErr
401		}	462		}

@@ -718,7 +779,7 @@ func (c Client) GetTrafficStatus(ctx context.Context) (dto.TrafficStatusRespon
718			779
719		// GetStatus returns the Gordon server status.	780		// GetStatus returns the Gordon server status.
720		func (c Client) GetStatus(ctx context.Context) (Status, error) {	781		func (c Client) GetStatus(ctx context.Context) (Status, error) {
721	-	resp, err := c.request(ctx, http.MethodGet, "/status", nil)	782	+	resp, err := c.requestWithRetry(ctx, http.MethodGet, "/status", nil)
722		if err != nil {	783		if err != nil {
723		return nil, err	784		return nil, err
724		}	785		}

M internal/adapters/in/cli/remote/client_test.go +183 -30


@@ -13,11 +13,11 @@ import (
13	13		"testing"
14	14		"time"
15	15
16		-	"github.com/bnema/gordon/internal/domain"
17	16		"github.com/stretchr/testify/assert"
18	17		"github.com/stretchr/testify/require"
19	18
20	19		"github.com/bnema/gordon/internal/adapters/dto"
	20	+	"github.com/bnema/gordon/internal/domain"
21	21		)
22	22
23	23		func withFastRetry(t *testing.T) {

@@ -32,57 +32,72 @@ func withFastRetry(t *testing.T) {
32	32		})
33	33		}
34	34
35		-	func TestClientRestartRetriesOn5xx(t *testing.T) {
	35	+	func TestClientRestartDoesNotRetryGatewayResponseAfterSideEffect(t *testing.T) {
36	36		withFastRetry(t)
37	37
38		-	var attempts int32
	38	+	var mutations int32
39	39		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
40	40		require.Equal(t, "/admin/restart/test.example.com", r.URL.Path)
41	41		require.Equal(t, http.MethodPost, r.Method)
42		-	current := atomic.AddInt32(&attempts, 1)
43		-	if current < 3 {
44		-	w.WriteHeader(http.StatusServiceUnavailable)
45		-	_, _ = w.Write([]byte(`{"error":"temporary outage"}`))
46		-	return
47		-	}
48		-	w.Header().Set("Content-Type", "application/json")
49		-	_, _ = w.Write([]byte(`{"status":"restarted","domain":"test.example.com"}`))
	42	+	atomic.AddInt32(&mutations, 1)
	43	+	w.WriteHeader(http.StatusServiceUnavailable)
	44	+	_, _ = w.Write([]byte(`{"error":"upstream response unavailable"}`))
50	45		}))
51	46		defer srv.Close()
52	47
53	48		client := NewClient(srv.URL)
54	49		result, err := client.Restart(context.Background(), "test.example.com", false)
55		-	require.NoError(t, err)
56		-	require.NotNil(t, result)
57		-	assert.Equal(t, "test.example.com", result.Domain)
58		-	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))
	50	+
	51	+	require.Nil(t, result)
	52	+	var outcomeErr *OutcomeUnknownError
	53	+	require.ErrorAs(t, err, &outcomeErr)
	54	+	assert.Contains(t, err.Error(), "inspect current state before retrying")
	55	+	assert.EqualValues(t, 1, atomic.LoadInt32(&mutations))
	56	+	}
	57	+
	58	+	func TestClientRestartDoesNotRetryAmbiguousTransportFailure(t *testing.T) {
	59	+	withFastRetry(t)
	60	+
	61	+	var mutations int32
	62	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	63	+	atomic.AddInt32(&mutations, 1)
	64	+	conn, _, err := w.(http.Hijacker).Hijack()
	65	+	require.NoError(t, err)
	66	+	require.NoError(t, conn.Close())
	67	+	}))
	68	+	defer srv.Close()
	69	+
	70	+	client := NewClient(srv.URL)
	71	+	result, err := client.Restart(context.Background(), "test.example.com", true)
	72	+
	73	+	require.Nil(t, result)
	74	+	var outcomeErr *OutcomeUnknownError
	75	+	require.ErrorAs(t, err, &outcomeErr)
	76	+	assert.EqualValues(t, 1, atomic.LoadInt32(&mutations))
59	77		}
60	78
61		-	func TestClientReloadRetriesOn5xx(t *testing.T) {
	79	+	func TestClientReloadDoesNotRetryGatewayResponse(t *testing.T) {
62	80		withFastRetry(t)
63	81
64	82		var attempts int32
65	83		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
66	84		require.Equal(t, "/admin/reload", r.URL.Path)
67	85		require.Equal(t, http.MethodPost, r.Method)
68		-	current := atomic.AddInt32(&attempts, 1)
69		-	if current < 2 {
70		-	w.WriteHeader(http.StatusBadGateway)
71		-	_, _ = w.Write([]byte(`upstream down`))
72		-	return
73		-	}
74		-	w.Header().Set("Content-Type", "application/json")
75		-	_, _ = w.Write([]byte(`{"status":"reloaded"}`))
	86	+	atomic.AddInt32(&attempts, 1)
	87	+	w.WriteHeader(http.StatusBadGateway)
	88	+	_, _ = w.Write([]byte(`upstream down`))
76	89		}))
77	90		defer srv.Close()
78	91
79	92		client := NewClient(srv.URL)
80	93		err := client.Reload(context.Background())
81		-	require.NoError(t, err)
82		-	assert.EqualValues(t, 2, atomic.LoadInt32(&attempts))
	94	+
	95	+	var outcomeErr *OutcomeUnknownError
	96	+	require.ErrorAs(t, err, &outcomeErr)
	97	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
83	98		}
84	99
85		-	func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) {
	100	+	func TestClientDeployDoesNotRetryGatewayResponse(t *testing.T) {
86	101		withFastRetry(t)
87	102
88	103		var attempts int32

@@ -96,9 +111,147 @@ func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) {
96	111		defer srv.Close()
97	112
98	113		client := NewClient(srv.URL)
99		-	_, err := client.Deploy(context.Background(), "test.example.com")
100		-	require.Error(t, err)
101		-	assert.Contains(t, err.Error(), "502 Bad Gateway: upstream unavailable")
	114	+	result, err := client.Deploy(context.Background(), "test.example.com")
	115	+
	116	+	require.Nil(t, result)
	117	+	var outcomeErr *OutcomeUnknownError
	118	+	require.ErrorAs(t, err, &outcomeErr)
	119	+	assert.ErrorContains(t, err, "502 Bad Gateway: upstream unavailable")
	120	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
	121	+	}
	122	+
	123	+	func TestClientDeployConfirmedRejectionIsNotOutcomeUnknown(t *testing.T) {
	124	+	var attempts int32
	125	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	126	+	atomic.AddInt32(&attempts, 1)
	127	+	w.WriteHeader(http.StatusUnprocessableEntity)
	128	+	_, _ = w.Write([]byte(`{"error":"invalid deployment"}`))
	129	+	}))
	130	+	defer srv.Close()
	131	+
	132	+	client := NewClient(srv.URL)
	133	+	result, err := client.Deploy(context.Background(), "test.example.com")
	134	+
	135	+	require.Nil(t, result)
	136	+	var outcomeErr *OutcomeUnknownError
	137	+	assert.NotErrorAs(t, err, &outcomeErr)
	138	+	var httpErr *HTTPError
	139	+	require.ErrorAs(t, err, &httpErr)
	140	+	assert.Equal(t, http.StatusUnprocessableEntity, httpErr.StatusCode)
	141	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
	142	+	}
	143	+
	144	+	func TestClientDeployAuthenticationRejectionIsNotOutcomeUnknown(t *testing.T) {
	145	+	var deployAttempts int32
	146	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	147	+	if r.URL.Path == "/auth/token" {
	148	+	w.WriteHeader(http.StatusForbidden)
	149	+	_, _ = w.Write([]byte(`{"error":"invalid credentials"}`))
	150	+	return
	151	+	}
	152	+	atomic.AddInt32(&deployAttempts, 1)
	153	+	w.WriteHeader(http.StatusNoContent)
	154	+	}))
	155	+	defer srv.Close()
	156	+
	157	+	client := NewClient(srv.URL, WithToken("invalid-token"))
	158	+	result, err := client.Deploy(context.Background(), "test.example.com")
	159	+
	160	+	require.Nil(t, result)
	161	+	var outcomeErr *OutcomeUnknownError
	162	+	assert.NotErrorAs(t, err, &outcomeErr)
	163	+	assert.ErrorContains(t, err, "ephemeral token exchange: 403 Forbidden")
	164	+	assert.Zero(t, atomic.LoadInt32(&deployAttempts))
	165	+	}
	166	+
	167	+	func TestClientGetStatusDoesNotRetryAuthenticationRejection(t *testing.T) {
	168	+	withFastRetry(t)
	169	+
	170	+	var tokenAttempts int32
	171	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	172	+	require.Equal(t, "/auth/token", r.URL.Path)
	173	+	atomic.AddInt32(&tokenAttempts, 1)
	174	+	w.WriteHeader(http.StatusForbidden)
	175	+	_, _ = w.Write([]byte(`{"error":"invalid credentials"}`))
	176	+	}))
	177	+	defer srv.Close()
	178	+
	179	+	client := NewClient(srv.URL, WithToken("invalid-token"))
	180	+	status, err := client.GetStatus(context.Background())
	181	+
	182	+	require.Nil(t, status)
	183	+	assert.ErrorContains(t, err, "ephemeral token exchange: 403 Forbidden")
	184	+	assert.EqualValues(t, 1, atomic.LoadInt32(&tokenAttempts))
	185	+	}
	186	+
	187	+	func TestClientGetStatusRetriesTransientGatewayResponse(t *testing.T) {
	188	+	withFastRetry(t)
	189	+
	190	+	var attempts int32
	191	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	192	+	require.Equal(t, "/admin/status", r.URL.Path)
	193	+	require.Equal(t, http.MethodGet, r.Method)
	194	+	if atomic.AddInt32(&attempts, 1) < 3 {
	195	+	w.WriteHeader(http.StatusGatewayTimeout)
	196	+	_, _ = w.Write([]byte(`{"error":"upstream timeout"}`))
	197	+	return
	198	+	}
	199	+	w.Header().Set("Content-Type", "application/json")
	200	+	_, _ = w.Write([]byte(`{"routes":2,"container_status":{}}`))
	201	+	}))
	202	+	defer srv.Close()
	203	+
	204	+	client := NewClient(srv.URL)
	205	+	status, err := client.GetStatus(context.Background())
	206	+
	207	+	require.NoError(t, err)
	208	+	require.NotNil(t, status)
	209	+	assert.Equal(t, 2, status.Routes)
	210	+	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))
	211	+	}
	212	+
	213	+	func TestClientGetStatusRetriesTransientTransportFailure(t *testing.T) {
	214	+	withFastRetry(t)
	215	+
	216	+	var attempts int32
	217	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	218	+	if atomic.AddInt32(&attempts, 1) < 3 {
	219	+	conn, _, err := w.(http.Hijacker).Hijack()
	220	+	require.NoError(t, err)
	221	+	require.NoError(t, conn.Close())
	222	+	return
	223	+	}
	224	+	w.Header().Set("Content-Type", "application/json")
	225	+	_, _ = w.Write([]byte(`{"routes":2,"container_status":{}}`))
	226	+	}))
	227	+	defer srv.Close()
	228	+
	229	+	client := NewClient(srv.URL)
	230	+	status, err := client.GetStatus(context.Background())
	231	+
	232	+	require.NoError(t, err)
	233	+	require.NotNil(t, status)
	234	+	assert.Equal(t, 2, status.Routes)
	235	+	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))
	236	+	}
	237	+
	238	+	func TestClientGetStatusReportsTransportContextAfterRetryExhaustion(t *testing.T) {
	239	+	withFastRetry(t)
	240	+
	241	+	var attempts int32
	242	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	243	+	atomic.AddInt32(&attempts, 1)
	244	+	conn, _, err := w.(http.Hijacker).Hijack()
	245	+	require.NoError(t, err)
	246	+	require.NoError(t, conn.Close())
	247	+	}))
	248	+	defer srv.Close()
	249	+
	250	+	client := NewClient(srv.URL)
	251	+	status, err := client.GetStatus(context.Background())
	252	+
	253	+	require.Nil(t, status)
	254	+	assert.ErrorContains(t, err, "GET /status transport request")
102	255		assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts))
103	256		}
104	257


@@ -13,11 +13,11 @@ import (
13		"testing"	13		"testing"
14		"time"	14		"time"
15			15
16	-	"github.com/bnema/gordon/internal/domain"
17		"github.com/stretchr/testify/assert"	16		"github.com/stretchr/testify/assert"
18		"github.com/stretchr/testify/require"	17		"github.com/stretchr/testify/require"
19			18
20		"github.com/bnema/gordon/internal/adapters/dto"	19		"github.com/bnema/gordon/internal/adapters/dto"
			20	+	"github.com/bnema/gordon/internal/domain"
21		)	21		)
22			22
23		func withFastRetry(t *testing.T) {	23		func withFastRetry(t *testing.T) {

@@ -32,57 +32,72 @@ func withFastRetry(t *testing.T) {
32		})	32		})
33		}	33		}
34			34
35	-	func TestClientRestartRetriesOn5xx(t *testing.T) {	35	+	func TestClientRestartDoesNotRetryGatewayResponseAfterSideEffect(t *testing.T) {
36		withFastRetry(t)	36		withFastRetry(t)
37			37
38	-	var attempts int32	38	+	var mutations int32
39		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {	39		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
40		require.Equal(t, "/admin/restart/test.example.com", r.URL.Path)	40		require.Equal(t, "/admin/restart/test.example.com", r.URL.Path)
41		require.Equal(t, http.MethodPost, r.Method)	41		require.Equal(t, http.MethodPost, r.Method)
42	-	current := atomic.AddInt32(&attempts, 1)	42	+	atomic.AddInt32(&mutations, 1)
43	-	if current < 3 {	43	+	w.WriteHeader(http.StatusServiceUnavailable)
44	-	w.WriteHeader(http.StatusServiceUnavailable)	44	+	_, _ = w.Write([]byte(`{"error":"upstream response unavailable"}`))
45	-	_, _ = w.Write([]byte(`{"error":"temporary outage"}`))
46	-	return
47	-	}
48	-	w.Header().Set("Content-Type", "application/json")
49	-	_, _ = w.Write([]byte(`{"status":"restarted","domain":"test.example.com"}`))
50		}))	45		}))
51		defer srv.Close()	46		defer srv.Close()
52			47
53		client := NewClient(srv.URL)	48		client := NewClient(srv.URL)
54		result, err := client.Restart(context.Background(), "test.example.com", false)	49		result, err := client.Restart(context.Background(), "test.example.com", false)
55	-	require.NoError(t, err)	50	+
56	-	require.NotNil(t, result)	51	+	require.Nil(t, result)
57	-	assert.Equal(t, "test.example.com", result.Domain)	52	+	var outcomeErr *OutcomeUnknownError
58	-	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))	53	+	require.ErrorAs(t, err, &outcomeErr)
			54	+	assert.Contains(t, err.Error(), "inspect current state before retrying")
			55	+	assert.EqualValues(t, 1, atomic.LoadInt32(&mutations))
			56	+	}
			57	+
			58	+	func TestClientRestartDoesNotRetryAmbiguousTransportFailure(t *testing.T) {
			59	+	withFastRetry(t)
			60	+
			61	+	var mutations int32
			62	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
			63	+	atomic.AddInt32(&mutations, 1)
			64	+	conn, _, err := w.(http.Hijacker).Hijack()
			65	+	require.NoError(t, err)
			66	+	require.NoError(t, conn.Close())
			67	+	}))
			68	+	defer srv.Close()
			69	+
			70	+	client := NewClient(srv.URL)
			71	+	result, err := client.Restart(context.Background(), "test.example.com", true)
			72	+
			73	+	require.Nil(t, result)
			74	+	var outcomeErr *OutcomeUnknownError
			75	+	require.ErrorAs(t, err, &outcomeErr)
			76	+	assert.EqualValues(t, 1, atomic.LoadInt32(&mutations))
59		}	77		}
60			78
61	-	func TestClientReloadRetriesOn5xx(t *testing.T) {	79	+	func TestClientReloadDoesNotRetryGatewayResponse(t *testing.T) {
62		withFastRetry(t)	80		withFastRetry(t)
63			81
64		var attempts int32	82		var attempts int32
65		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {	83		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
66		require.Equal(t, "/admin/reload", r.URL.Path)	84		require.Equal(t, "/admin/reload", r.URL.Path)
67		require.Equal(t, http.MethodPost, r.Method)	85		require.Equal(t, http.MethodPost, r.Method)
68	-	current := atomic.AddInt32(&attempts, 1)	86	+	atomic.AddInt32(&attempts, 1)
69	-	if current < 2 {	87	+	w.WriteHeader(http.StatusBadGateway)
70	-	w.WriteHeader(http.StatusBadGateway)	88	+	_, _ = w.Write([]byte(`upstream down`))
71	-	_, _ = w.Write([]byte(`upstream down`))
72	-	return
73	-	}
74	-	w.Header().Set("Content-Type", "application/json")
75	-	_, _ = w.Write([]byte(`{"status":"reloaded"}`))
76		}))	89		}))
77		defer srv.Close()	90		defer srv.Close()
78			91
79		client := NewClient(srv.URL)	92		client := NewClient(srv.URL)
80		err := client.Reload(context.Background())	93		err := client.Reload(context.Background())
81	-	require.NoError(t, err)	94	+
82	-	assert.EqualValues(t, 2, atomic.LoadInt32(&attempts))	95	+	var outcomeErr *OutcomeUnknownError
			96	+	require.ErrorAs(t, err, &outcomeErr)
			97	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
83		}	98		}
84			99
85	-	func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) {	100	+	func TestClientDeployDoesNotRetryGatewayResponse(t *testing.T) {
86		withFastRetry(t)	101		withFastRetry(t)
87			102
88		var attempts int32	103		var attempts int32

@@ -96,9 +111,147 @@ func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) {
96		defer srv.Close()	111		defer srv.Close()
97			112
98		client := NewClient(srv.URL)	113		client := NewClient(srv.URL)
99	-	_, err := client.Deploy(context.Background(), "test.example.com")	114	+	result, err := client.Deploy(context.Background(), "test.example.com")
100	-	require.Error(t, err)	115	+
101	-	assert.Contains(t, err.Error(), "502 Bad Gateway: upstream unavailable")	116	+	require.Nil(t, result)
			117	+	var outcomeErr *OutcomeUnknownError
			118	+	require.ErrorAs(t, err, &outcomeErr)
			119	+	assert.ErrorContains(t, err, "502 Bad Gateway: upstream unavailable")
			120	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
			121	+	}
			122	+
			123	+	func TestClientDeployConfirmedRejectionIsNotOutcomeUnknown(t *testing.T) {
			124	+	var attempts int32
			125	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
			126	+	atomic.AddInt32(&attempts, 1)
			127	+	w.WriteHeader(http.StatusUnprocessableEntity)
			128	+	_, _ = w.Write([]byte(`{"error":"invalid deployment"}`))
			129	+	}))
			130	+	defer srv.Close()
			131	+
			132	+	client := NewClient(srv.URL)
			133	+	result, err := client.Deploy(context.Background(), "test.example.com")
			134	+
			135	+	require.Nil(t, result)
			136	+	var outcomeErr *OutcomeUnknownError
			137	+	assert.NotErrorAs(t, err, &outcomeErr)
			138	+	var httpErr *HTTPError
			139	+	require.ErrorAs(t, err, &httpErr)
			140	+	assert.Equal(t, http.StatusUnprocessableEntity, httpErr.StatusCode)
			141	+	assert.EqualValues(t, 1, atomic.LoadInt32(&attempts))
			142	+	}
			143	+
			144	+	func TestClientDeployAuthenticationRejectionIsNotOutcomeUnknown(t *testing.T) {
			145	+	var deployAttempts int32
			146	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			147	+	if r.URL.Path == "/auth/token" {
			148	+	w.WriteHeader(http.StatusForbidden)
			149	+	_, _ = w.Write([]byte(`{"error":"invalid credentials"}`))
			150	+	return
			151	+	}
			152	+	atomic.AddInt32(&deployAttempts, 1)
			153	+	w.WriteHeader(http.StatusNoContent)
			154	+	}))
			155	+	defer srv.Close()
			156	+
			157	+	client := NewClient(srv.URL, WithToken("invalid-token"))
			158	+	result, err := client.Deploy(context.Background(), "test.example.com")
			159	+
			160	+	require.Nil(t, result)
			161	+	var outcomeErr *OutcomeUnknownError
			162	+	assert.NotErrorAs(t, err, &outcomeErr)
			163	+	assert.ErrorContains(t, err, "ephemeral token exchange: 403 Forbidden")
			164	+	assert.Zero(t, atomic.LoadInt32(&deployAttempts))
			165	+	}
			166	+
			167	+	func TestClientGetStatusDoesNotRetryAuthenticationRejection(t *testing.T) {
			168	+	withFastRetry(t)
			169	+
			170	+	var tokenAttempts int32
			171	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			172	+	require.Equal(t, "/auth/token", r.URL.Path)
			173	+	atomic.AddInt32(&tokenAttempts, 1)
			174	+	w.WriteHeader(http.StatusForbidden)
			175	+	_, _ = w.Write([]byte(`{"error":"invalid credentials"}`))
			176	+	}))
			177	+	defer srv.Close()
			178	+
			179	+	client := NewClient(srv.URL, WithToken("invalid-token"))
			180	+	status, err := client.GetStatus(context.Background())
			181	+
			182	+	require.Nil(t, status)
			183	+	assert.ErrorContains(t, err, "ephemeral token exchange: 403 Forbidden")
			184	+	assert.EqualValues(t, 1, atomic.LoadInt32(&tokenAttempts))
			185	+	}
			186	+
			187	+	func TestClientGetStatusRetriesTransientGatewayResponse(t *testing.T) {
			188	+	withFastRetry(t)
			189	+
			190	+	var attempts int32
			191	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			192	+	require.Equal(t, "/admin/status", r.URL.Path)
			193	+	require.Equal(t, http.MethodGet, r.Method)
			194	+	if atomic.AddInt32(&attempts, 1) < 3 {
			195	+	w.WriteHeader(http.StatusGatewayTimeout)
			196	+	_, _ = w.Write([]byte(`{"error":"upstream timeout"}`))
			197	+	return
			198	+	}
			199	+	w.Header().Set("Content-Type", "application/json")
			200	+	_, _ = w.Write([]byte(`{"routes":2,"container_status":{}}`))
			201	+	}))
			202	+	defer srv.Close()
			203	+
			204	+	client := NewClient(srv.URL)
			205	+	status, err := client.GetStatus(context.Background())
			206	+
			207	+	require.NoError(t, err)
			208	+	require.NotNil(t, status)
			209	+	assert.Equal(t, 2, status.Routes)
			210	+	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))
			211	+	}
			212	+
			213	+	func TestClientGetStatusRetriesTransientTransportFailure(t *testing.T) {
			214	+	withFastRetry(t)
			215	+
			216	+	var attempts int32
			217	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
			218	+	if atomic.AddInt32(&attempts, 1) < 3 {
			219	+	conn, _, err := w.(http.Hijacker).Hijack()
			220	+	require.NoError(t, err)
			221	+	require.NoError(t, conn.Close())
			222	+	return
			223	+	}
			224	+	w.Header().Set("Content-Type", "application/json")
			225	+	_, _ = w.Write([]byte(`{"routes":2,"container_status":{}}`))
			226	+	}))
			227	+	defer srv.Close()
			228	+
			229	+	client := NewClient(srv.URL)
			230	+	status, err := client.GetStatus(context.Background())
			231	+
			232	+	require.NoError(t, err)
			233	+	require.NotNil(t, status)
			234	+	assert.Equal(t, 2, status.Routes)
			235	+	assert.EqualValues(t, 3, atomic.LoadInt32(&attempts))
			236	+	}
			237	+
			238	+	func TestClientGetStatusReportsTransportContextAfterRetryExhaustion(t *testing.T) {
			239	+	withFastRetry(t)
			240	+
			241	+	var attempts int32
			242	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
			243	+	atomic.AddInt32(&attempts, 1)
			244	+	conn, _, err := w.(http.Hijacker).Hijack()
			245	+	require.NoError(t, err)
			246	+	require.NoError(t, conn.Close())
			247	+	}))
			248	+	defer srv.Close()
			249	+
			250	+	client := NewClient(srv.URL)
			251	+	status, err := client.GetStatus(context.Background())
			252	+
			253	+	require.Nil(t, status)
			254	+	assert.ErrorContains(t, err, "GET /status transport request")
102		assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts))	255		assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts))
103		}	256		}
104			257