From 3a0ca7f57f7e5970ca4f65079a97534d1a592147 Mon Sep 17 00:00:00 2001 From: User Date: Sat, 18 Apr 2026 13:16:05 +0800 Subject: [PATCH] fix: concurrency safety and API correctness from code review B3 (HIGH): sora_generation_service.go - Add panic recovery to parallel S3 URL fetching goroutines. Without recovery, a panic in GetAccessURL would skip wg.Done() causing wg.Wait() to hang indefinitely. B2 (MEDIUM): subscription_service.go:549 - Replace bare goroutine with safego.Go() for consistent panic recovery pattern. All other async calls in this file already use safego. B4 (MEDIUM): admin/sora_handler.go - Change ClearUserStorage response from 200 no-op to 410 Gone. The per-user storage quota was fully removed; returning success was misleading to callers. --- .../internal/handler/admin/sora_handler.go | 24 +++++++++++-------- .../service/sora_generation_service.go | 9 +++++++ .../internal/service/subscription_service.go | 4 ++-- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/backend/internal/handler/admin/sora_handler.go b/backend/internal/handler/admin/sora_handler.go index 7ee24630..a154d2aa 100644 --- a/backend/internal/handler/admin/sora_handler.go +++ b/backend/internal/handler/admin/sora_handler.go @@ -1,6 +1,7 @@ package admin import ( + "net/http" "strconv" "github.com/Wei-Shaw/sub2api/internal/pkg/pagination" @@ -167,8 +168,9 @@ func (h *SoraHandler) ListGenerations(c *gin.Context) { // ClearUserStorage 清除用户的 Sora 存储空间(已弃用)。 // // Deprecated: Per-user storage tracking has been removed. -// This endpoint now returns a success no-op. It will be removed in a future version. -// Clients should stop calling this endpoint. +// This endpoint now returns 410 Gone. Per-user Sora storage quota tracking was +// fully removed in the Sora storage refactoring. Storage management is now +// handled at the system-default level via SoraQuotaService. // // DELETE /api/v1/admin/sora/users/:id/storage func (h *SoraHandler) ClearUserStorage(c *gin.Context) { @@ -178,18 +180,20 @@ func (h *SoraHandler) ClearUserStorage(c *gin.Context) { return } - // 重置用户的存储使用量 - // NOTE: Per-user SoraStorageUsedBytes field removed. - // Storage clearing now handled at the SoraGenerationService level if needed. - _, err = h.userRepo.GetByID(c.Request.Context(), userID) - if err != nil { + // Verify user exists before responding + ctx := c.Request.Context() + if _, err := h.userRepo.GetByID(ctx, userID); err != nil { response.ErrorFrom(c, err) return } - // TODO: Implement storage cleanup via SoraGenerationService c.Header("Deprecation", "true") c.Header("Sunset", "2026-12-31") - c.Header("Warning", `299 - "Deprecated API: use SoraGenerationService for storage management"`) - response.Success(c, gin.H{"message": "User Sora storage cleared (no-op: per-user tracking removed)", "deprecated": true}) + c.Header("Warning", `299 - "Gone: per-user storage tracking removed, see SoraQuotaService"`) + c.JSON(http.StatusGone, gin.H{ + "error": "This endpoint is no longer available", + "message": "Per-user Sora storage quota tracking has been removed. Storage is now managed at system level.", + "sunset": "2026-12-31", + "deprecated": true, + }) } diff --git a/backend/internal/service/sora_generation_service.go b/backend/internal/service/sora_generation_service.go index 22d5b519..9112a782 100644 --- a/backend/internal/service/sora_generation_service.go +++ b/backend/internal/service/sora_generation_service.go @@ -308,6 +308,15 @@ func (s *SoraGenerationService) ResolveMediaURLs(ctx context.Context, gen *SoraG wg.Add(1) go func(i int, objectKey string) { defer wg.Done() + defer func() { + if r := recover(); r != nil { + errMu.Lock() + if firstErr == nil { + firstErr = fmt.Errorf("goroutine panic fetching S3 URL: %v", r) + } + errMu.Unlock() + } + }() url, err := s.s3Storage.GetAccessURL(ctx, objectKey) if err != nil { errMu.Lock() diff --git a/backend/internal/service/subscription_service.go b/backend/internal/service/subscription_service.go index 0a304b20..d070ae05 100644 --- a/backend/internal/service/subscription_service.go +++ b/backend/internal/service/subscription_service.go @@ -546,11 +546,11 @@ func (s *SubscriptionService) ExtendSubscription(ctx context.Context, subscripti s.InvalidateSubCache(sub.UserID, sub.GroupID) if s.billingCacheService != nil { userID, groupID := sub.UserID, sub.GroupID - go func() { + safego.Go("service.billing-cache-invalidate", func() { cacheCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() _ = s.billingCacheService.InvalidateSubscription(cacheCtx, userID, groupID) - }() + }, nil) } return s.userSubRepo.GetByID(ctx, subscriptionID)