filer: do not 404 a TUS session on a transient chunk-load failure (#11153)

* filer: do not 404 a TUS session on a transient chunk-load failure

readTusSessionInfo already proved the session exists before
loadTusSessionChunks is called, so a failure there is a read failure,
not evidence the session is gone: a volume-server timeout or a
canceled request context surfaces through ListDirectoryEntries the
same way a missing session would.

Every such error was mapped to writeTusSessionNotFound, answering 404
to HEAD/PATCH and 204 to DELETE. A spec-compliant TUS client trusts
that and discards the session, orphaning every chunk it had committed
until the 24h expiry sweep, or forever if it never issues a DELETE.

Only an error matching filer_pb.ErrNotFound is now reported as not
found; anything else answers 500 so the client retries against the
same session instead of abandoning it.

* test: cover a TUS session's transient chunk-load failure

Adds a listErr hook to the in-memory test store, alongside the
existing commitErr/deleteErr, to simulate a store or RPC failure from
ListDirectoryEntries.

HEAD, PATCH and DELETE against a live session all answer with a
server error instead of a not-found status when the chunk listing
fails transiently, and the session is left on disk untouched. A
listing failure that genuinely means not found, filer_pb.ErrNotFound,
still answers 404 (204 for DELETE).
This commit is contained in:
Chris Lu
2026-09-04 16:39:24 -07:00
committed by GitHub
parent ed9d58873e
commit cda43f1976
3 changed files with 96 additions and 0 deletions
@@ -29,6 +29,7 @@ type renameTestStore struct {
findCalls map[string]int
commitErr error
deleteErr error
listErr error // simulates a transient store/RPC failure from a directory listing
findDelay time.Duration // optional: widen check-then-act windows in tests
}
@@ -107,6 +108,11 @@ func (s *renameTestStore) DeleteFolderChildren(_ context.Context, p util.FullPat
func (s *renameTestStore) listDirectoryEntries(dirPath util.FullPath, startFileName string, includeStartFile bool, limit int64, prefix string, eachEntryFunc filer.ListEachEntryFunc) (string, error) {
s.mu.Lock()
if s.listErr != nil {
err := s.listErr
s.mu.Unlock()
return "", err
}
var entries []*filer.Entry
for path, entry := range s.entries {
if path == string(dirPath) {
+4
View File
@@ -99,6 +99,10 @@ func (fs *FilerServer) tusHandler(w http.ResponseWriter, r *http.Request) {
}
if err := fs.loadTusSessionChunks(ctx, session); err != nil {
glog.Errorf("Failed to load TUS session %s chunks: %v", uploadID, err)
if !errors.Is(err, filer_pb.ErrNotFound) {
http.Error(w, "Failed to load upload state", http.StatusInternalServerError)
return
}
writeTusSessionNotFound(w, r.Method)
return
}
@@ -0,0 +1,86 @@
package weed_server
import (
"context"
"net/http"
"net/http/httptest"
"testing"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/seaweedfs/seaweedfs/weed/util"
)
// TestFilerServer_tusHandler_TransientChunkLoadFailure reproduces #11151: a
// transient failure listing a session's chunk directory (volume timeout,
// canceled context) must not be reported as 404/204, since readTusSessionInfo
// already proved the session exists. A false "not found" makes a compliant
// client discard a live session and orphan every chunk it committed.
func TestFilerServer_tusHandler_TransientChunkLoadFailure(t *testing.T) {
tests := []struct {
name string
method string
}{
{"HEAD", http.MethodHead},
{"PATCH", http.MethodPatch},
{"DELETE", http.MethodDelete},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fs, store := newTusTestServer(t, map[string]string{tusTestUploadID: "/buckets/data/file.bin"})
store.listErr = context.Canceled
headers := map[string]string{"Authorization": "Bearer " + signFilerToken(t, tusTestWriteKey, nil, nil)}
if tt.method == http.MethodHead {
headers["Authorization"] = "Bearer " + signFilerToken(t, tusTestReadKey, nil, nil)
}
if tt.method == http.MethodPatch {
headers["Content-Type"] = "application/offset+octet-stream"
headers["Upload-Offset"] = "0"
}
req := tusRequest(tt.method, "/.tus/.uploads/"+tusTestUploadID, headers, "")
rec := httptest.NewRecorder()
fs.tusHandler(rec, req)
if rec.Code == http.StatusNotFound || rec.Code == http.StatusNoContent {
t.Fatalf("%s on a transient chunk-load failure = %d, want a server error, not a not-found status", tt.method, rec.Code)
}
if rec.Code < 500 {
t.Errorf("%s on a transient chunk-load failure = %d, want a 5xx status", tt.method, rec.Code)
}
store.listErr = nil
if _, err := store.FindEntry(context.Background(), util.FullPath(fs.tusSessionInfoPath(tusTestUploadID))); err != nil {
t.Errorf("session removed after a transient %s failure: %v", tt.method, err)
}
})
}
}
// TestFilerServer_tusHandler_ChunkLoadNotFoundStillNotFound verifies a chunk
// listing failure that genuinely means "not found" is still reported as such,
// so the fix for #11151 does not mask a real absence.
func TestFilerServer_tusHandler_ChunkLoadNotFoundStillNotFound(t *testing.T) {
fs, store := newTusTestServer(t, map[string]string{tusTestUploadID: "/buckets/data/file.bin"})
store.listErr = filer_pb.ErrNotFound
req := tusRequest(http.MethodHead, "/.tus/.uploads/"+tusTestUploadID, map[string]string{
"Authorization": "Bearer " + signFilerToken(t, tusTestReadKey, nil, nil),
}, "")
rec := httptest.NewRecorder()
fs.tusHandler(rec, req)
if rec.Code != http.StatusNotFound {
t.Fatalf("HEAD on a genuinely missing session = %d, want %d", rec.Code, http.StatusNotFound)
}
deleteReq := tusRequest(http.MethodDelete, "/.tus/.uploads/"+tusTestUploadID, map[string]string{
"Authorization": "Bearer " + signFilerToken(t, tusTestWriteKey, nil, nil),
}, "")
deleteRec := httptest.NewRecorder()
fs.tusHandler(deleteRec, deleteReq)
if deleteRec.Code != http.StatusNoContent {
t.Fatalf("DELETE on a genuinely missing session = %d, want %d", deleteRec.Code, http.StatusNoContent)
}
}