From 9137f863558ea95bcd458cf3d6ad7c111a403ca7 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 2 Aug 2026 00:01:08 +0100 Subject: [PATCH] Classify license server transport failures as retryable Transport-level failures from httpClient.Do (connection refused/reset, DNS, TLS, timeouts) were wrapped as plain errors, so they fell through userFriendlyActivationError's LicenseServerError branch to the generic contact-support fallback instead of the retryable temporarily-unavailable message. Users behind flaky networks got a dead-end message, and the 2026-08-01 rc.6 paid-runtime gate failure was misdiagnosed the same way. Classify Do failures at the client layer as a retryable LicenseServerError (code license_server_unreachable, StatusCode 0 so revocation/suspension/ migration status gates never match) with the cause reachable via Unwrap so the status poller's context.Canceled check keeps working. The cloud-paid transport boundary contract now names this classification. Tests pin the classification, the cancellation chain, and the end-to-end user-facing retryable message. --- .../v6/internal/subsystems/cloud-paid.md | 7 +++ internal/api/license_handlers_test.go | 26 ++++++++++ pkg/licensing/activation_types.go | 9 ++++ pkg/licensing/activation_types_test.go | 26 ++++++++++ pkg/licensing/license_server_client.go | 25 +++++++-- pkg/licensing/license_server_client_test.go | 52 +++++++++++++++++++ 6 files changed, 140 insertions(+), 5 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 8fb05860cf..e754a7a72f 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -829,6 +829,13 @@ the App/AppLayout, routing, and desktop Actions journey tests. may allow plaintext only on direct loopback development targets, and must route outbound license-server fetches through the restricted HTTP client so DNS rebinding or private-network drift cannot silently reopen the boundary. + Outbound transport failures (DNS, connection refused/reset, TLS, timeout) + must classify as a retryable `LicenseServerError` rather than a plain + wrapped error, so activation surfaces present retry guidance instead of a + terminal contact-support fallback. The classification carries no HTTP + status, so revocation, suspension, and migration status gates never match + a request that produced no response, and the underlying cause stays + reachable through `Unwrap` so context-cancellation checks keep working. 10. Add or change encrypted activation persistence through `pkg/licensing/persistence.go` and `pkg/licensing/activation_store.go` That persistence boundary must encrypt new state only with the persistent random key file and the current HKDF-based derivation. Machine ID may diff --git a/internal/api/license_handlers_test.go b/internal/api/license_handlers_test.go index a1f13792d1..c3df057a27 100644 --- a/internal/api/license_handlers_test.go +++ b/internal/api/license_handlers_test.go @@ -1449,6 +1449,32 @@ func TestUserFriendlyActivationError_ServerError(t *testing.T) { } } +func TestUserFriendlyActivationError_TransportErrorRetryable(t *testing.T) { + // A real transport failure (nothing listening on the loopback port) must + // surface the retryable "temporarily unavailable" message, not the + // generic contact-support fallback. Pins the end-to-end path from + // httpClient.Do through the client's error classification to the + // user-facing copy. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + unreachableURL := server.URL + server.Close() + + client := pkglicensing.NewLicenseServerClient(unreachableURL) + _, err := client.Activate(context.Background(), pkglicensing.ActivateInstallationRequest{ + ActivationKey: "ppk_live_test123", + InstanceFingerprint: "fp-123", + }) + if err == nil { + t.Fatal("expected transport error, got nil") + } + + msg := userFriendlyActivationError(fmt.Errorf("activate license: %w", err)) + want := "The license server is temporarily unavailable. Please try again in a few minutes." + if msg != want { + t.Errorf("transport error message = %q, want %q", msg, want) + } +} + // ======================================== // HandleClearLicense tests // ======================================== diff --git a/pkg/licensing/activation_types.go b/pkg/licensing/activation_types.go index d9cd00a5a2..2d2713e293 100644 --- a/pkg/licensing/activation_types.go +++ b/pkg/licensing/activation_types.go @@ -338,8 +338,17 @@ type LicenseServerError struct { Code string `json:"code"` Message string `json:"message"` Retryable bool `json:"retryable"` + + // cause carries the underlying transport error when the request never + // produced an HTTP response, so errors.Is checks (e.g. context.Canceled + // in the status poller) keep working through the classification. + cause error } func (e *LicenseServerError) Error() string { return e.Code + ": " + e.Message } + +func (e *LicenseServerError) Unwrap() error { + return e.cause +} diff --git a/pkg/licensing/activation_types_test.go b/pkg/licensing/activation_types_test.go index e2069dc356..90eb935179 100644 --- a/pkg/licensing/activation_types_test.go +++ b/pkg/licensing/activation_types_test.go @@ -2,6 +2,7 @@ package licensing import ( "encoding/json" + "errors" "testing" "time" ) @@ -451,6 +452,31 @@ func TestLicenseServerError(t *testing.T) { if got != want { t.Errorf("Error() = %q, want %q", got, want) } + if e.Unwrap() != nil { + t.Errorf("server-response error should have no transport cause, got %v", e.Unwrap()) + } +} + +func TestLicenseServerErrorTransportClassification(t *testing.T) { + cause := errors.New("dial tcp 127.0.0.1:9: connect: connection refused") + err := transportError("activate", cause) + + var serverErr *LicenseServerError + if !errors.As(err, &serverErr) { + t.Fatalf("transportError should classify as *LicenseServerError, got %T", err) + } + if !serverErr.Retryable { + t.Errorf("transport failure must be retryable: %+v", serverErr) + } + if serverErr.StatusCode != 0 { + t.Errorf("transport failure must not carry an HTTP status, got %d", serverErr.StatusCode) + } + if serverErr.Code != "license_server_unreachable" { + t.Errorf("Code = %q, want license_server_unreachable", serverErr.Code) + } + if !errors.Is(err, cause) { + t.Error("underlying transport cause must stay reachable through Unwrap") + } } // --- test helpers --- diff --git a/pkg/licensing/license_server_client.go b/pkg/licensing/license_server_client.go index 791646de5d..2d6b4e85d0 100644 --- a/pkg/licensing/license_server_client.go +++ b/pkg/licensing/license_server_client.go @@ -106,6 +106,21 @@ func (c *LicenseServerClient) ready() error { return nil } +// transportError classifies an outbound transport failure (DNS, connection +// refused/reset, TLS, timeout) as a retryable license-server error. The +// request never reached the server, so retrying is always the right guidance; +// StatusCode stays 0 so status-gated consumers (revocation, suspension, +// migration classification) never match. The underlying error remains +// reachable via Unwrap for context-cancellation checks. +func transportError(label string, err error) error { + return &LicenseServerError{ + Code: "license_server_unreachable", + Message: fmt.Sprintf("%s request failed: %v", label, err), + Retryable: true, + cause: err, + } +} + // postActivationRequest POSTs an idempotent JSON request to the license // server and decodes the shared activation response shape. label feeds error // wrapping ("activate", "exchange"). @@ -128,7 +143,7 @@ func (c *LicenseServerClient) postActivationRequest(ctx context.Context, path, l resp, err := c.httpClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("%s request failed: %w", label, err) + return nil, transportError(label, err) } defer resp.Body.Close() @@ -180,7 +195,7 @@ func (c *LicenseServerClient) RefreshGrant(ctx context.Context, installationID, resp, err := refreshClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("refresh request failed: %w", err) + return nil, transportError("refresh", err) } defer resp.Body.Close() @@ -218,7 +233,7 @@ func (c *LicenseServerClient) CheckInstallationStatus(ctx context.Context, insta statusClient.Timeout = 10 * time.Second resp, err := statusClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("installation status request failed: %w", err) + return nil, transportError("installation status", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { @@ -260,7 +275,7 @@ func (c *LicenseServerClient) GetCheckoutSessionResult(ctx context.Context, sess resp, err := c.httpClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("checkout session result request failed: %w", err) + return nil, transportError("checkout session result", err) } defer resp.Body.Close() @@ -296,7 +311,7 @@ func (c *LicenseServerClient) CreateCheckoutPortalHandoff(ctx context.Context, r resp, err := c.httpClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("checkout portal handoff request failed: %w", err) + return nil, transportError("checkout portal handoff", err) } defer resp.Body.Close() diff --git a/pkg/licensing/license_server_client_test.go b/pkg/licensing/license_server_client_test.go index b9119c538b..a5f93150b3 100644 --- a/pkg/licensing/license_server_client_test.go +++ b/pkg/licensing/license_server_client_test.go @@ -3,6 +3,7 @@ package licensing import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -51,6 +52,57 @@ func TestNewLicenseServerClient(t *testing.T) { }) } +func TestClientTransportErrorRetryable(t *testing.T) { + t.Run("connection refused is a retryable server error", func(t *testing.T) { + // Grab a loopback URL with nothing listening on it. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + unreachableURL := server.URL + server.Close() + + client := NewLicenseServerClient(unreachableURL) + _, err := client.Activate(context.Background(), ActivateInstallationRequest{ + ActivationKey: "ppk_live_test123", + InstanceFingerprint: "fp-123", + }) + if err == nil { + t.Fatal("expected transport error, got nil") + } + + var serverErr *LicenseServerError + if !errors.As(err, &serverErr) { + t.Fatalf("transport error should classify as *LicenseServerError, got %T: %v", err, err) + } + if !serverErr.Retryable { + t.Errorf("transport error should be retryable: %+v", serverErr) + } + if serverErr.StatusCode != 0 { + t.Errorf("transport error must not carry an HTTP status, got %d", serverErr.StatusCode) + } + if serverErr.Code != "license_server_unreachable" { + t.Errorf("Code = %q, want license_server_unreachable", serverErr.Code) + } + }) + + t.Run("context cancellation stays detectable through classification", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + defer server.Close() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + client := NewLicenseServerClient(server.URL) + _, err := client.CheckInstallationStatus(ctx, "pit_live_token", InstallationStatusRequest{}) + if err == nil { + t.Fatal("expected error from canceled context, got nil") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("errors.Is(err, context.Canceled) should hold through the transport classification, got %v", err) + } + }) +} + func TestClientActivate(t *testing.T) { t.Run("successful activation", func(t *testing.T) { grantJWT := makeTestGrantJWT(t, &GrantClaims{