From 71f8a6e18946f7de9da05e818a305e71fa88ff84 Mon Sep 17 00:00:00 2001
From: chrislu <chris.lu@gmail.com>
Date: Thu, 20 Nov 2025 13:21:05 -0800
Subject: [PATCH] fix: handle partial results correctly in
 LookupVolumeIdsWithFallback callers

Two callers were discarding partial results by checking err before processing
the result map. While these are currently single-volume lookups (so partial
results aren't possible), the code was fragile and would break if we ever
batched multiple volumes together.

Changes:
- Check result map FIRST, then conditionally check error
- If volume is found in result, use it (ignore errors about other volumes)
- If volume is NOT found and err != nil, include error context with %w
- Add defensive comments explaining the pattern for future maintainers

This makes the code:
1. Correct for future batched lookups
2. More informative (preserves underlying error details)
3. Consistent with filer_grpc_server.go which already handles this correctly

Example: If looking up ["1", "2", "999"] and only 999 fails, callers
looking for volumes 1 or 2 will succeed instead of failing unnecessarily.
---
 weed/wdclient/filer_client.go  | 13 +++++++++----
 weed/wdclient/vidmap_client.go | 13 +++++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/weed/wdclient/filer_client.go b/weed/wdclient/filer_client.go
index 3f665d775..bb5242c44 100644
--- a/weed/wdclient/filer_client.go
+++ b/weed/wdclient/filer_client.go
@@ -113,14 +113,19 @@ func (fc *FilerClient) GetLookupFileIdFunction() LookupFileIdFunctionType {
 
 		// First try the cache using LookupVolumeIdsWithFallback
 		vidLocations, err := fc.LookupVolumeIdsWithFallback(ctx, []string{volumeIdStr})
-		if err != nil {
-			return nil, fmt.Errorf("LookupVolume %s failed: %v", fileId, err)
-		}
-
+		
+		// Check for partial results first (important for multi-volume batched lookups)
 		locations, found := vidLocations[volumeIdStr]
 		if !found || len(locations) == 0 {
+			// Volume not found - return specific error with context from lookup if available
+			if err != nil {
+				return nil, fmt.Errorf("volume %s not found for fileId %s: %w", volumeIdStr, fileId, err)
+			}
 			return nil, fmt.Errorf("volume %s not found for fileId %s", volumeIdStr, fileId)
 		}
+		
+		// Volume found successfully - ignore any errors about other volumes
+		// (not relevant for single-volume lookup, but defensive for future batching)
 
 		// Build URLs with publicUrl preference
 		for _, loc := range locations {
diff --git a/weed/wdclient/vidmap_client.go b/weed/wdclient/vidmap_client.go
index 0457c246b..5e5d6f7b2 100644
--- a/weed/wdclient/vidmap_client.go
+++ b/weed/wdclient/vidmap_client.go
@@ -78,14 +78,19 @@ func (vc *vidMapClient) LookupFileIdWithFallback(ctx context.Context, fileId str
 
 	// Use shared lookup logic with batching and singleflight
 	vidLocations, err := vc.LookupVolumeIdsWithFallback(ctx, []string{volumeId})
-	if err != nil {
-		return nil, fmt.Errorf("LookupVolume %s failed: %v", fileId, err)
-	}
-
+	
+	// Check for partial results first (important for multi-volume batched lookups)
 	locations, found := vidLocations[volumeId]
 	if !found || len(locations) == 0 {
+		// Volume not found - return specific error with context from lookup if available
+		if err != nil {
+			return nil, fmt.Errorf("volume %s not found for fileId %s: %w", volumeId, fileId, err)
+		}
 		return nil, fmt.Errorf("volume %s not found for fileId %s", volumeId, fileId)
 	}
+	
+	// Volume found successfully - ignore any errors about other volumes
+	// (not relevant for single-volume lookup, but defensive for future batching)
 
 	// Build HTTP URLs from locations, preferring same data center
 	var sameDcUrls, otherDcUrls []string