From 721ccc4b0b86a44302fc45fa2a3d6a497a35a5b7 Mon Sep 17 00:00:00 2001 From: cesnietor <> Date: Tue, 23 Jan 2024 17:28:59 -0800 Subject: [PATCH 1/2] Fail request when error occurs during download --- api/admin_inspect.go | 2 +- api/user_objects.go | 32 ++++++++++++++++---------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api/admin_inspect.go b/api/admin_inspect.go index c59eb5c0ed..6774cd6553 100644 --- a/api/admin_inspect.go +++ b/api/admin_inspect.go @@ -120,7 +120,7 @@ func processInspectResponse(params *inspectApi.InspectParams, k []byte, r io.Rea _, err := io.Copy(w, r) if err != nil { - LogError("Unable to write all the data: %v", err) + LogError("unable to write all the data: %v", err) } } } diff --git a/api/user_objects.go b/api/user_objects.go index b3f225dab3..8420d0bb66 100644 --- a/api/user_objects.go +++ b/api/user_objects.go @@ -448,7 +448,8 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl // override filename is set decodeOverride, err := base64.StdEncoding.DecodeString(*params.OverrideFileName) if err != nil { - return + ErrorWithContext(ctx, fmt.Errorf("unable to decode OverrideFileName: %v", err)) + panic(err) } overrideName := string(decodeOverride) @@ -472,17 +473,15 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl stat, err := resp.Stat() if err != nil { minErr := minio.ToErrorResponse(err) - rw.WriteHeader(minErr.StatusCode) - ErrorWithContext(ctx, fmt.Errorf("Failed to get Stat() response from server for %s (version %s): %v", prefix, opts.VersionID, minErr.Error())) - return + ErrorWithContext(ctx, fmt.Errorf("failed to get Stat() response from server for %s (version %s): %v", prefix, opts.VersionID, minErr.Error())) + panic(err) } // if we are getting a Range Request (video) handle that specially ranges, err := parseRange(params.HTTPRequest.Header.Get("Range"), stat.Size) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to parse range header input %s: %v", params.HTTPRequest.Header.Get("Range"), err)) - rw.WriteHeader(400) - return + ErrorWithContext(ctx, fmt.Errorf("unable to parse range header input %s: %v", params.HTTPRequest.Header.Get("Range"), err)) + panic(err) } contentType := stat.ContentType rw.Header().Set("X-XSS-Protection", "1; mode=block") @@ -510,9 +509,8 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl _, err = resp.Seek(start, io.SeekStart) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to seek at offset %d: %v", start, err)) - rw.WriteHeader(400) - return + ErrorWithContext(ctx, fmt.Errorf("unable to seek at offset %d: %v", start, err)) + panic(err) } rw.Header().Set("Accept-Ranges", "bytes") @@ -524,8 +522,8 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl rw.Header().Set("Content-Length", fmt.Sprintf("%d", length)) _, err = io.Copy(rw, io.LimitReader(resp, length)) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to write all data to client: %v", err)) - return + ErrorWithContext(ctx, fmt.Errorf("unable to write all data to client: %v", err)) + panic(err) } }), nil } @@ -610,8 +608,8 @@ func getDownloadFolderResponse(session *models.Principal, params objectApi.Downl encodedPrefix := SanitizeEncodedPrefix(params.Prefix) decodedPrefix, err := base64.StdEncoding.DecodeString(encodedPrefix) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to parse encoded prefix %s: %v", encodedPrefix, err)) - return + ErrorWithContext(ctx, fmt.Errorf("unable to parse encoded prefix %s: %v", encodedPrefix, err)) + panic(err) } prefixPath = string(decodedPrefix) @@ -632,7 +630,8 @@ func getDownloadFolderResponse(session *models.Principal, params objectApi.Downl // Copy the stream _, err := io.Copy(rw, resp) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to write all the requested data: %v", err)) + ErrorWithContext(ctx, fmt.Errorf("unable to write all the requested data: %v", err)) + panic(err) } }), nil } @@ -754,7 +753,8 @@ func getMultipleFilesDownloadResponse(session *models.Principal, params objectAp // Copy the stream _, err := io.Copy(rw, resp) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("Unable to write all the requested data: %v", err)) + ErrorWithContext(ctx, fmt.Errorf("unable to write all the requested data: %v", err)) + panic(err) } }), nil } From 6b3041da5d42ec0e98002e0f3e8360096706a8e3 Mon Sep 17 00:00:00 2001 From: cesnietor <> Date: Wed, 24 Jan 2024 17:07:23 -0800 Subject: [PATCH 2/2] Handle error in frontend --- api/user_objects.go | 37 ++++++++++++------- .../Buckets/ListBuckets/Objects/utils.ts | 13 ++++--- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/api/user_objects.go b/api/user_objects.go index 8420d0bb66..b78a315044 100644 --- a/api/user_objects.go +++ b/api/user_objects.go @@ -448,8 +448,9 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl // override filename is set decodeOverride, err := base64.StdEncoding.DecodeString(*params.OverrideFileName) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("unable to decode OverrideFileName: %v", err)) - panic(err) + fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to decode OverrideFileName: %v", err)) + http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusBadRequest) + return } overrideName := string(decodeOverride) @@ -473,15 +474,17 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl stat, err := resp.Stat() if err != nil { minErr := minio.ToErrorResponse(err) - ErrorWithContext(ctx, fmt.Errorf("failed to get Stat() response from server for %s (version %s): %v", prefix, opts.VersionID, minErr.Error())) - panic(err) + fmtError := ErrorWithContext(ctx, fmt.Errorf("failed to get Stat() response from server for %s (version %s): %v", prefix, opts.VersionID, minErr.Error())) + http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError) + return } // if we are getting a Range Request (video) handle that specially ranges, err := parseRange(params.HTTPRequest.Header.Get("Range"), stat.Size) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("unable to parse range header input %s: %v", params.HTTPRequest.Header.Get("Range"), err)) - panic(err) + fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to parse range header input %s: %v", params.HTTPRequest.Header.Get("Range"), err)) + http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError) + return } contentType := stat.ContentType rw.Header().Set("X-XSS-Protection", "1; mode=block") @@ -509,8 +512,9 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl _, err = resp.Seek(start, io.SeekStart) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("unable to seek at offset %d: %v", start, err)) - panic(err) + fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to seek at offset %d: %v", start, err)) + http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError) + return } rw.Header().Set("Accept-Ranges", "bytes") @@ -523,7 +527,9 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl _, err = io.Copy(rw, io.LimitReader(resp, length)) if err != nil { ErrorWithContext(ctx, fmt.Errorf("unable to write all data to client: %v", err)) - panic(err) + // You can't change headers after you already started writing the body. + // Handle incomplete write in client. + return } }), nil } @@ -608,8 +614,9 @@ func getDownloadFolderResponse(session *models.Principal, params objectApi.Downl encodedPrefix := SanitizeEncodedPrefix(params.Prefix) decodedPrefix, err := base64.StdEncoding.DecodeString(encodedPrefix) if err != nil { - ErrorWithContext(ctx, fmt.Errorf("unable to parse encoded prefix %s: %v", encodedPrefix, err)) - panic(err) + fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to parse encoded prefix %s: %v", encodedPrefix, err)) + http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError) + return } prefixPath = string(decodedPrefix) @@ -631,7 +638,9 @@ func getDownloadFolderResponse(session *models.Principal, params objectApi.Downl _, err := io.Copy(rw, resp) if err != nil { ErrorWithContext(ctx, fmt.Errorf("unable to write all the requested data: %v", err)) - panic(err) + // You can't change headers after you already started writing the body. + // Handle incomplete write in client. + return } }), nil } @@ -754,7 +763,9 @@ func getMultipleFilesDownloadResponse(session *models.Principal, params objectAp _, err := io.Copy(rw, resp) if err != nil { ErrorWithContext(ctx, fmt.Errorf("unable to write all the requested data: %v", err)) - panic(err) + // You can't change headers after you already started writing the body. + // Handle incomplete write in client. + return } }), nil } diff --git a/web-app/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts b/web-app/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts index 6cb778e563..626bea53b4 100644 --- a/web-app/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts +++ b/web-app/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts @@ -21,7 +21,7 @@ import store from "../../../../../store"; import { ContentType, PermissionResource } from "api/consoleApi"; import { api } from "../../../../../api"; import { setErrorSnackMessage } from "../../../../../systemSlice"; - +import { StatusCodes } from "http-status-codes"; const downloadWithLink = (href: string, downloadFileName: string) => { const link = document.createElement("a"); link.href = href; @@ -78,6 +78,7 @@ export const download = ( toastCallback: () => void, ) => { let basename = document.baseURI.replace(window.location.origin, ""); + let bytesLoaded = 0; const state = store.getState(); const anonymousMode = state.system.anonymousMode; @@ -106,7 +107,7 @@ export const download = ( "progress", function (evt) { let percentComplete = Math.round((evt.loaded / fileSize) * 100); - + bytesLoaded = evt.loaded; if (progressCallback) { progressCallback(percentComplete); } @@ -116,8 +117,10 @@ export const download = ( req.responseType = "blob"; req.onreadystatechange = () => { - if (req.readyState === 4) { - if (req.status === 200) { + if (req.readyState === XMLHttpRequest.DONE) { + let completeDownload = bytesLoaded === fileSize; + + if (req.status === StatusCodes.OK && completeDownload) { const rspHeader = req.getResponseHeader("Content-Disposition"); let filename = "download"; @@ -143,7 +146,7 @@ export const download = ( return; } } - errorCallback(`Unexpected response status code (${req.status}).`); + errorCallback(`Unexpected response, download incomplete.`); } } };