fix(purchase): widen SYB writeback backoff, cover CheckSession, improve message (#330 review)

Address review findings on 01510a8: 1. BLOCKER: sessionRetryBackoff summed to 30min, shorter than the up-to- ~60min gap between a session dying and the next hourly SYB sync refreshing it. Changed to 5m/10m/15m/30m/30m (total 90min across maxSessionRetryAttempts=6), updated the code comment to state the ~90min > one hourly sync period rationale, and added TestSessionRetryBackoffTotalExceedsHourlySyncWindow to guard it. 2. Test gap: the CheckSession probe added inside restoreOrderWritebackClient was only exercised through a fake Factory, never through a real sybclient.Client. Added httptest-backed tests that run restoreOrderWritebackClient against an emulated /am/user/get (matching the envelope shape in sybclient/client.go's `envelope` type): valid session returns a client, mismatched username maps to ErrSessionInvalid, 5xx/timeout map to a non-invalid error — each asserting the syb_session row is left untouched. Added an end-to-end worker test using the real Factory against the invalid-session server, asserting failed/SYB_SESSION_UNAVAILABLE with a scheduled backoff and an intact session row. 3. sessionUnavailableMessage: renamed the default category to "会话恢复失败（网络/其他）" and wrapped every category in an actionable template ("SYB会话不可用（<类别>），将自动重试；如持续失败请恢复登录后重试"), still well under the 300-char column limit and free of raw error text/credentials. Tests: go vet ./app/goauto/purchase/... (clean); go test ./app/goauto/purchase/... (ok, 3.4s, includes the new httptest-backed CheckSession coverage and the backoff-window guard). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTDbDcwbDw1TSAcE6wfh2F
2026-09-21 16:09:34 +08:00
parent 01510a85dc
commit 08b7095cf1
2 changed files with 184 additions and 17 deletions
@@ -2,13 +2,19 @@ package purchase

 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
 	"testing"
 	"time"

 	"github.com/google/uuid"
 	"go-admin/app/goauto/models"
 	"go-admin/app/goauto/sybclient"
+	"go-admin/config"
 	"gorm.io/gorm"
 )

@@ -31,6 +37,12 @@ func TestOrderWritebackSessionFailureSchedulesBoundedRetry(t *testing.T) {
 	if row.ErrorMessage == "" || len(row.ErrorMessage) > 300 {
 		t.Fatalf("error message not recorded safely: %q", row.ErrorMessage)
 	}
+	if !strings.Contains(row.ErrorMessage, "会话缺失/已过期") {
+		t.Fatalf("category missing from message: %q", row.ErrorMessage)
+	}
+	if !strings.Contains(row.ErrorMessage, "将自动重试") || !strings.Contains(row.ErrorMessage, "恢复登录") {
+		t.Fatalf("message is not actionable: %q", row.ErrorMessage)
+	}
 	if row.LeaseExpiresAt == nil || !row.LeaseExpiresAt.After(s.Now()) {
 		t.Fatal("no backoff scheduled for first session-class failure")
 	}
@@ -39,6 +51,23 @@ func TestOrderWritebackSessionFailureSchedulesBoundedRetry(t *testing.T) {
 	}
 }

+// TestSessionRetryBackoffTotalExceedsHourlySyncWindow guards the ticket's
+// blocker: the cumulative auto-retry window must outlast one hourly sync
+// period (up to ~60 minutes from failure to the refresh that fixes it),
+// otherwise attempts run out before the session has a chance to recover.
+func TestSessionRetryBackoffTotalExceedsHourlySyncWindow(t *testing.T) {
+	if len(sessionRetryBackoff) != maxSessionRetryAttempts-1 {
+		t.Fatalf("expected %d backoff steps for %d attempts, got %d", maxSessionRetryAttempts-1, maxSessionRetryAttempts, len(sessionRetryBackoff))
+	}
+	var total time.Duration
+	for _, d := range sessionRetryBackoff {
+		total += d
+	}
+	if total <= time.Hour {
+		t.Fatalf("total backoff %s must exceed one hourly sync period", total)
+	}
+}
+
 func TestOrderWritebackSessionFailureNotReclaimedBeforeBackoffExpires(t *testing.T) {
 	s, _ := orderWritebackFixture(t)
 	if _, err := wbFactoryWorker(s, sybclient.ErrNoSession).RunOnce(context.Background()); err != nil {
@@ -146,8 +175,12 @@ func TestOrderWritebackCheckSessionNetworkErrorIsSessionClassAndNeverDeletesSess
 func seedWritebackSession(t *testing.T, s *Service) {
 	t.Helper()
 	store := sybclient.NewSessionStore(s.DB)
+	// restoreOrderWritebackClient's SessionStore.Load compares against real
+	// wall-clock time.Now(), not the service's mocked s.Now (which fixtures
+	// pin to a fixed past date) — so the session must expire relative to the
+	// real clock or Load reports ErrNoSession even though a row exists.
 	if err := store.Save(context.Background(), sybclient.Session{
-		Username: "syb-writeback-test", UserID: 555, CookiesJSON: `[{"name":"SESSION","value":"x"}]`, ExpiresAt: s.Now().Add(time.Hour),
+		Username: "syb-writeback-test", UserID: 555, CookiesJSON: `[{"name":"SESSION","value":"x"}]`, ExpiresAt: time.Now().Add(time.Hour),
 	}); err != nil {
 		t.Fatal(err)
 	}
@@ -227,3 +260,132 @@ func TestOrderWritebackOtherFailureCodesAreNotAutoRetried(t *testing.T) {
 		t.Fatalf("a non-session failure code was auto-reclaimed: ok=%v err=%v", ok, err)
 	}
 }
+
+// --- restoreOrderWritebackClient against a real sybclient.Client + emulated
+// SYB /am/user/get, so the CheckSession probe added by #330 is actually
+// exercised end to end instead of only through a fake Factory. ---
+
+// sybUserGetServer emulates the one endpoint restoreOrderWritebackClient's
+// CheckSession call depends on, using the real envelope shape documented in
+// sybclient/client.go's `envelope` type and asserted against in
+// sybclient/client_test.go.
+func sybUserGetServer(t *testing.T, handler func(w http.ResponseWriter, r *http.Request)) *httptest.Server {
+	t.Helper()
+	mux := http.NewServeMux()
+	mux.HandleFunc("/am/user/get", handler)
+	srv := httptest.NewServer(mux)
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+// withWritebackSYBConfig points config.ExtConfig.SYB at the given test
+// server for the duration of the test, restoring the previous value
+// afterwards so other tests (and any parallel config reads) are unaffected.
+func withWritebackSYBConfig(t *testing.T, baseURL, username string) {
+	t.Helper()
+	prev := config.ExtConfig.SYB
+	config.ExtConfig.SYB = config.SYB{BaseURL: baseURL, Username: username}
+	t.Cleanup(func() { config.ExtConfig.SYB = prev })
+}
+
+func envelopeOK(w http.ResponseWriter, data any) {
+	body, _ := json.Marshal(data)
+	env, _ := json.Marshal(map[string]any{"status": true, "msg": "获取成功", "data": json.RawMessage(body), "code": nil})
+	w.Header().Set("Content-Type", "application/json")
+	w.Write(env)
+}
+
+func TestRestoreOrderWritebackClientValidSessionReturnsClient(t *testing.T) {
+	s, _ := orderWritebackFixture(t)
+	seedWritebackSession(t, s)
+	srv := sybUserGetServer(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("id") != strconv.FormatInt(555, 10) {
+			t.Fatalf("unexpected id query: %s", r.URL.RawQuery)
+		}
+		envelopeOK(w, map[string]any{"id": 555, "username": "syb-writeback-test"})
+	})
+	withWritebackSYBConfig(t, srv.URL, "syb-writeback-test")
+	client, err := restoreOrderWritebackClient(context.Background(), s.DB)
+	if err != nil || client == nil {
+		t.Fatalf("expected a usable client, got client=%v err=%v", client, err)
+	}
+	assertWritebackSessionUntouched(t, s)
+}
+
+func TestRestoreOrderWritebackClientMismatchedUsernameIsSessionInvalid(t *testing.T) {
+	s, _ := orderWritebackFixture(t)
+	seedWritebackSession(t, s)
+	srv := sybUserGetServer(t, func(w http.ResponseWriter, r *http.Request) {
+		// SYB says the cookie now belongs to a different account (12 §3.5):
+		// treated the same as an explicit logout.
+		envelopeOK(w, map[string]any{"id": 555, "username": "somebody-else"})
+	})
+	withWritebackSYBConfig(t, srv.URL, "syb-writeback-test")
+	_, err := restoreOrderWritebackClient(context.Background(), s.DB)
+	if !errors.Is(err, sybclient.ErrSessionInvalid) {
+		t.Fatalf("expected ErrSessionInvalid, got %v", err)
+	}
+	assertWritebackSessionUntouched(t, s)
+}
+
+func TestRestoreOrderWritebackClient500IsNotSessionInvalid(t *testing.T) {
+	s, _ := orderWritebackFixture(t)
+	seedWritebackSession(t, s)
+	srv := sybUserGetServer(t, func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	})
+	withWritebackSYBConfig(t, srv.URL, "syb-writeback-test")
+	_, err := restoreOrderWritebackClient(context.Background(), s.DB)
+	if err == nil {
+		t.Fatal("expected an error for a 5xx response")
+	}
+	if errors.Is(err, sybclient.ErrSessionInvalid) {
+		t.Fatalf("a 5xx must not be classified as a confirmed logout, got %v", err)
+	}
+	assertWritebackSessionUntouched(t, s)
+}
+
+func TestRestoreOrderWritebackClientTimeoutIsNotSessionInvalid(t *testing.T) {
+	s, _ := orderWritebackFixture(t)
+	seedWritebackSession(t, s)
+	srv := sybUserGetServer(t, func(w http.ResponseWriter, r *http.Request) {
+		<-r.Context().Done() // never respond; the client-side ctx timeout fires first
+	})
+	withWritebackSYBConfig(t, srv.URL, "syb-writeback-test")
+	ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
+	defer cancel()
+	_, err := restoreOrderWritebackClient(ctx, s.DB)
+	if err == nil {
+		t.Fatal("expected an error for a request that never completes")
+	}
+	if errors.Is(err, sybclient.ErrSessionInvalid) {
+		t.Fatalf("a timeout must not be classified as a confirmed logout, got %v", err)
+	}
+	assertWritebackSessionUntouched(t, s)
+}
+
+// TestOrderWritebackWorkerWithRealFactoryOnInvalidSession is the requested
+// end-to-end case: the worker's actual Factory (restoreOrderWritebackClient)
+// against a server that reports the cached session invalid. It must record
+// SYB_SESSION_UNAVAILABLE with a scheduled backoff and must not touch the
+// cached session row.
+func TestOrderWritebackWorkerWithRealFactoryOnInvalidSession(t *testing.T) {
+	s, task := orderWritebackFixture(t)
+	seedWritebackSession(t, s)
+	srv := sybUserGetServer(t, func(w http.ResponseWriter, r *http.Request) {
+		envelopeOK(w, map[string]any{"id": 555, "username": "somebody-else"})
+	})
+	withWritebackSYBConfig(t, srv.URL, "syb-writeback-test")
+	w := &OrderWritebackWorker{DB: s.DB, Now: s.Now, Factory: restoreOrderWritebackClient}
+	if ok, err := w.RunOnce(context.Background()); err != nil || !ok {
+		t.Fatalf("run %v %v", ok, err)
+	}
+	row := loadOrderWritebackByTask(t, s, task.ID)
+	if row.Status != "failed" || row.ErrorCode != "SYB_SESSION_UNAVAILABLE" {
+		t.Fatalf("status=%s code=%s", row.Status, row.ErrorCode)
+	}
+	if row.LeaseExpiresAt == nil {
+		t.Fatal("no backoff scheduled")
+	}
+	assertWritebackSessionUntouched(t, s)
+}
@@ -32,20 +32,24 @@ type OrderWritebackWorker struct {
 var errSessionUserIDMissing = errors.New("SYB 会话记录缺少有效 user id")

 // Bounded auto-retry for session-class writeback failures (#330). A session
-// outage self-heals once GoAutoSYBHourlySync refreshes syb_session, so a
-// short-lived backoff schedule — growing up to ~15 minutes — comfortably
-// spans that hourly cadence without hammering SYB while the session is down.
-// maxSessionRetryAttempts caps the automatic attempts so a session that never
-// recovers still lands back in "failed" for a human instead of retrying
-// forever.
+// outage self-heals once GoAutoSYBHourlySync refreshes syb_session, but that
+// refresh only happens once per hour (at :05) and only fires the run *after*
+// the session is found dead — so the wait from failure to refresh can be
+// close to a full hour. The backoff schedule below sums to ~90 minutes
+// (5+10+15+30+30) across maxSessionRetryAttempts=6 attempts, deliberately
+// longer than one hourly sync period so a session recovered by "the next"
+// hourly run is still caught automatically instead of exhausting attempts
+// first. maxSessionRetryAttempts caps the automatic attempts so a session
+// that never recovers still lands back in "failed" for a human instead of
+// retrying forever.
 const maxSessionRetryAttempts = 6

 var sessionRetryBackoff = []time.Duration{
-	1 * time.Minute,
-	2 * time.Minute,
-	4 * time.Minute,
-	8 * time.Minute,
+	5 * time.Minute,
+	10 * time.Minute,
 	15 * time.Minute,
+	30 * time.Minute,
+	30 * time.Minute,
 }

 // sessionRetryDelay returns the backoff before the next automatic attempt,
@@ -65,18 +69,19 @@ func sessionRetryDelay(attempt int) time.Duration {
 // sessionUnavailableMessage classifies why the cached SYB session could not
 // be used, without ever including cookies, tokens or other credential
 // material (#330 修订1点3). The category — not the raw error text — is what
-// gets persisted to error_message.
+// gets persisted to error_message, wrapped in a fixed, actionable template
+// that stays well under the 300-char column limit.
 func sessionUnavailableMessage(err error) string {
+	category := "会话恢复失败（网络/其他）"
 	switch {
 	case errors.Is(err, sybclient.ErrNoSession):
-		return "会话缺失/已过期"
+		category = "会话缺失/已过期"
 	case errors.Is(err, errSessionUserIDMissing):
-		return "会话记录异常，缺少 user id"
+		category = "会话记录异常，缺少 user id"
 	case errors.Is(err, sybclient.ErrSessionInvalid):
-		return "会话校验失效"
-	default:
-		return "会话校验网络错误"
+		category = "会话校验失效"
 	}
+	return "SYB会话不可用（" + category + "），将自动重试；如持续失败请恢复登录后重试"
 }

 // restoreOrderWritebackClient rebuilds a SYB client from the cached session