From b782fd7b98532197036f6ff7e321dda83af6a88a Mon Sep 17 00:00:00 2001
From: chrislu <chris.lu@gmail.com>
Date: Fri, 5 Dec 2025 13:05:02 -0800
Subject: [PATCH] refactor: Use idiomatic Go error handling in fetchFileContent

Changed fetchFileContent to return (string, error) instead of
(content string, reason string) for idiomatic Go error handling.
This enables error wrapping and standard 'if err != nil' checks.

Also improved error messages to be more descriptive for debugging,
including the HTTP status code and response body on non-200 responses.
---
 weed/admin/handlers/file_browser_handlers.go | 30 +++++++++++---------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/weed/admin/handlers/file_browser_handlers.go b/weed/admin/handlers/file_browser_handlers.go
index 02cf7bf77..ee214ae40 100644
--- a/weed/admin/handlers/file_browser_handlers.go
+++ b/weed/admin/handlers/file_browser_handlers.go
@@ -470,28 +470,27 @@ func (h *FileBrowserHandlers) validateAndCleanFilePath(filePath string) (string,
 	return cleanPath, nil
 }
 
-// fetchFileContent fetches file content from the filer and returns the content and an error reason.
-// If the fetch is successful, reason will be empty string.
-func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Duration) (content string, reason string) {
+// fetchFileContent fetches file content from the filer and returns the content or an error.
+func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Duration) (string, error) {
 	filerAddress := h.adminServer.GetFilerAddress()
 	if filerAddress == "" {
-		return "", "Filer address not configured"
+		return "", fmt.Errorf("filer address not configured")
 	}
 
 	if err := h.validateFilerAddress(filerAddress); err != nil {
-		return "", "Invalid filer address configuration"
+		return "", fmt.Errorf("invalid filer address configuration: %w", err)
 	}
 
 	cleanFilePath, err := h.validateAndCleanFilePath(filePath)
 	if err != nil {
-		return "", "Invalid file path"
+		return "", err
 	}
 
 	// Create the file URL with proper scheme based on TLS configuration
 	fileURL := fmt.Sprintf("%s%s", filerAddress, cleanFilePath)
 	fileURL, err = h.httpClient.NormalizeHttpScheme(fileURL)
 	if err != nil {
-		return "", "Failed to construct file URL"
+		return "", fmt.Errorf("failed to construct file URL: %w", err)
 	}
 
 	// lgtm[go/ssrf]
@@ -503,20 +502,21 @@ func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Dur
 	}
 	resp, err := clientWithTimeout.Get(fileURL)
 	if err != nil {
-		return "", "Failed to fetch file from filer"
+		return "", fmt.Errorf("failed to fetch file from filer: %w", err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return "", "Failed to fetch file from filer"
+		body, _ := io.ReadAll(resp.Body)
+		return "", fmt.Errorf("filer returned status %d: %s", resp.StatusCode, string(body))
 	}
 
 	contentBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return "", "Failed to read file content"
+		return "", fmt.Errorf("failed to read file content: %w", err)
 	}
 
-	return string(contentBytes), ""
+	return string(contentBytes), nil
 }
 
 // DownloadFile handles file download requests by proxying through the Admin UI server
@@ -682,8 +682,12 @@ func (h *FileBrowserHandlers) ViewFile(c *gin.Context) {
 			reason = "File too large for viewing (>1MB)"
 		} else {
 			// Fetch file content from filer
-			content, reason = h.fetchFileContent(filePath, 30*time.Second)
-			viewable = (reason == "")
+			var err error
+			content, err = h.fetchFileContent(filePath, 30*time.Second)
+			if err != nil {
+				reason = err.Error()
+			}
+			viewable = (err == nil)
 		}
 	} else {
 		// Not a text file, but might be viewable as image or PDF