Merge pull request #4618 from superman2003/fix/system-update-detach-request-ctx
fix(update): detach in-place update from the HTTP request lifetime
This commit is contained in:
@@ -22,6 +22,27 @@ type SystemHandler struct {
|
||||
lockSvc *service.SystemOperationLockService
|
||||
}
|
||||
|
||||
// systemUpdateTimeout bounds a full in-place update or rollback: the release
|
||||
// manifest fetch plus a large binary download over slow links. It must stay
|
||||
// above the GitHub download client timeout (10 minutes) so the download owns
|
||||
// its own deadline.
|
||||
const systemUpdateTimeout = 15 * time.Minute
|
||||
|
||||
// systemUpdateContext detaches a long-running update/rollback from the HTTP
|
||||
// request lifetime. Browsers and reverse proxies commonly abort idle requests
|
||||
// after 30-60s (axios default, nginx proxy_read_timeout), which canceled
|
||||
// c.Request.Context() mid-download and killed the update with
|
||||
// "download failed: context canceled" (#4504). The swap keeps running after a
|
||||
// client disconnect; a later retry then hits the system operation lock or
|
||||
// reports "Already up to date".
|
||||
func systemUpdateContext(ctx context.Context) (context.Context, context.CancelFunc) {
|
||||
base := context.Background()
|
||||
if ctx != nil {
|
||||
base = context.WithoutCancel(ctx)
|
||||
}
|
||||
return context.WithTimeout(base, systemUpdateTimeout)
|
||||
}
|
||||
|
||||
type systemUpdateService interface {
|
||||
CheckUpdate(ctx context.Context, force bool) (*service.UpdateInfo, error)
|
||||
PerformUpdate(ctx context.Context) error
|
||||
@@ -75,9 +96,12 @@ func (h *SystemHandler) PerformUpdate(c *gin.Context) {
|
||||
release(releaseReason, succeeded)
|
||||
}()
|
||||
|
||||
if err := h.updateSvc.PerformUpdate(ctx); err != nil {
|
||||
updateCtx, cancel := systemUpdateContext(ctx)
|
||||
defer cancel()
|
||||
|
||||
if err := h.updateSvc.PerformUpdate(updateCtx); err != nil {
|
||||
if errors.Is(err, service.ErrNoUpdateAvailable) {
|
||||
info, checkErr := h.updateSvc.CheckUpdate(ctx, false)
|
||||
info, checkErr := h.updateSvc.CheckUpdate(updateCtx, false)
|
||||
if checkErr != nil {
|
||||
releaseReason = "SYSTEM_UPDATE_FAILED"
|
||||
return nil, checkErr
|
||||
@@ -152,7 +176,10 @@ func (h *SystemHandler) Rollback(c *gin.Context) {
|
||||
}()
|
||||
|
||||
if targetVersion != "" {
|
||||
err = h.updateSvc.RollbackToVersion(ctx, targetVersion)
|
||||
// 指定版本回退同样要下载完整二进制,与更新一样和请求生命周期解耦。
|
||||
rollbackCtx, cancel := systemUpdateContext(ctx)
|
||||
defer cancel()
|
||||
err = h.updateSvc.RollbackToVersion(rollbackCtx, targetVersion)
|
||||
} else {
|
||||
err = h.updateSvc.Rollback()
|
||||
}
|
||||
|
||||
@@ -18,18 +18,22 @@ import (
|
||||
)
|
||||
|
||||
type systemHandlerUpdateServiceStub struct {
|
||||
performErr error
|
||||
updateInfo *service.UpdateInfo
|
||||
checkErr error
|
||||
checkForces []bool
|
||||
performCall int
|
||||
rollbackCall int
|
||||
rollbackToCall int
|
||||
rollbackToVersions []string
|
||||
rollbackToErr error
|
||||
rollbackVersions []service.RollbackVersion
|
||||
rollbackVersionsErr error
|
||||
rollbackVersionsCall int
|
||||
performErr error
|
||||
updateInfo *service.UpdateInfo
|
||||
checkErr error
|
||||
checkForces []bool
|
||||
performCall int
|
||||
performCtxErr error
|
||||
performHasDeadline bool
|
||||
rollbackCall int
|
||||
rollbackToCall int
|
||||
rollbackToCtxErr error
|
||||
rollbackToHasDeadline bool
|
||||
rollbackToVersions []string
|
||||
rollbackToErr error
|
||||
rollbackVersions []service.RollbackVersion
|
||||
rollbackVersionsErr error
|
||||
rollbackVersionsCall int
|
||||
}
|
||||
|
||||
func (s *systemHandlerUpdateServiceStub) CheckUpdate(_ context.Context, force bool) (*service.UpdateInfo, error) {
|
||||
@@ -37,8 +41,10 @@ func (s *systemHandlerUpdateServiceStub) CheckUpdate(_ context.Context, force bo
|
||||
return s.updateInfo, s.checkErr
|
||||
}
|
||||
|
||||
func (s *systemHandlerUpdateServiceStub) PerformUpdate(context.Context) error {
|
||||
func (s *systemHandlerUpdateServiceStub) PerformUpdate(ctx context.Context) error {
|
||||
s.performCall++
|
||||
s.performCtxErr = ctx.Err()
|
||||
_, s.performHasDeadline = ctx.Deadline()
|
||||
return s.performErr
|
||||
}
|
||||
|
||||
@@ -52,8 +58,10 @@ func (s *systemHandlerUpdateServiceStub) ListRollbackVersions(context.Context) (
|
||||
return s.rollbackVersions, s.rollbackVersionsErr
|
||||
}
|
||||
|
||||
func (s *systemHandlerUpdateServiceStub) RollbackToVersion(_ context.Context, version string) error {
|
||||
func (s *systemHandlerUpdateServiceStub) RollbackToVersion(ctx context.Context, version string) error {
|
||||
s.rollbackToCall++
|
||||
s.rollbackToCtxErr = ctx.Err()
|
||||
_, s.rollbackToHasDeadline = ctx.Deadline()
|
||||
s.rollbackToVersions = append(s.rollbackToVersions, version)
|
||||
return s.rollbackToErr
|
||||
}
|
||||
@@ -165,6 +173,55 @@ func TestSystemHandlerPerformUpdateFailureStillReturnsInternalError(t *testing.T
|
||||
require.Equal(t, "internal error", body.Message)
|
||||
}
|
||||
|
||||
// TestSystemHandlerPerformUpdateSurvivesClientDisconnect reproduces #4504:
|
||||
// the browser or a reverse proxy (axios 30s default, nginx proxy_read_timeout
|
||||
// 60s) aborts the long-running update request and cancels the request
|
||||
// context. The download must keep running on a detached, bounded context
|
||||
// instead of dying with "download failed: context canceled".
|
||||
func TestSystemHandlerPerformUpdateSurvivesClientDisconnect(t *testing.T) {
|
||||
updateSvc := &systemHandlerUpdateServiceStub{}
|
||||
repo := newMemoryIdempotencyRepoStub()
|
||||
router := newSystemHandlerTestRouter(t, updateSvc, repo)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/system/update", nil)
|
||||
canceledCtx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
req = req.WithContext(canceledCtx)
|
||||
req.Header.Set("Idempotency-Key", "disconnected-update")
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, 1, updateSvc.performCall)
|
||||
require.NoError(t, updateSvc.performCtxErr,
|
||||
"update must not observe the canceled request context")
|
||||
require.True(t, updateSvc.performHasDeadline,
|
||||
"detached update context must still be bounded by a deadline")
|
||||
requireSystemLockStatus(t, repo, service.IdempotencyStatusSucceeded)
|
||||
}
|
||||
|
||||
func TestSystemHandlerRollbackToVersionSurvivesClientDisconnect(t *testing.T) {
|
||||
updateSvc := &systemHandlerUpdateServiceStub{}
|
||||
repo := newMemoryIdempotencyRepoStub()
|
||||
router := newSystemHandlerTestRouter(t, updateSvc, repo)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/system/rollback",
|
||||
strings.NewReader(`{"version":"0.1.146"}`))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
canceledCtx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
req = req.WithContext(canceledCtx)
|
||||
req.Header.Set("Idempotency-Key", "disconnected-rollback")
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, 1, updateSvc.rollbackToCall)
|
||||
require.NoError(t, updateSvc.rollbackToCtxErr,
|
||||
"versioned rollback must not observe the canceled request context")
|
||||
require.True(t, updateSvc.rollbackToHasDeadline,
|
||||
"detached rollback context must still be bounded by a deadline")
|
||||
requireSystemLockStatus(t, repo, service.IdempotencyStatusSucceeded)
|
||||
}
|
||||
|
||||
func TestSystemHandlerRollbackWithoutBodyUsesLegacyBackup(t *testing.T) {
|
||||
updateSvc := &systemHandlerUpdateServiceStub{}
|
||||
repo := newMemoryIdempotencyRepoStub()
|
||||
|
||||
@@ -61,12 +61,22 @@ export async function getRollbackVersions(): Promise<{ versions: RollbackVersion
|
||||
return data
|
||||
}
|
||||
|
||||
/**
|
||||
* In-place update/rollback downloads a full release binary from GitHub, which
|
||||
* can take several minutes on slow links. The global 30s axios timeout would
|
||||
* abort the request mid-download (#4504), so these calls wait as long as the
|
||||
* backend allows (15 minutes server-side).
|
||||
*/
|
||||
const UPDATE_REQUEST_TIMEOUT_MS = 15 * 60 * 1000
|
||||
|
||||
/**
|
||||
* Perform system update
|
||||
* Downloads and applies the latest version
|
||||
*/
|
||||
export async function performUpdate(): Promise<UpdateResult> {
|
||||
const { data } = await apiClient.post<UpdateResult>('/admin/system/update')
|
||||
const { data } = await apiClient.post<UpdateResult>('/admin/system/update', undefined, {
|
||||
timeout: UPDATE_REQUEST_TIMEOUT_MS
|
||||
})
|
||||
return data
|
||||
}
|
||||
|
||||
@@ -77,7 +87,8 @@ export async function performUpdate(): Promise<UpdateResult> {
|
||||
export async function rollback(version?: string): Promise<UpdateResult> {
|
||||
const { data } = await apiClient.post<UpdateResult>(
|
||||
'/admin/system/rollback',
|
||||
version ? { version } : undefined
|
||||
version ? { version } : undefined,
|
||||
{ timeout: UPDATE_REQUEST_TIMEOUT_MS }
|
||||
)
|
||||
return data
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user