Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail request when error occurs during download #3214

Merged
merged 5 commits into from
Jan 26, 2024
Merged

Fail request when error occurs during download #3214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
721ccc4
Fail request when error occurs during download
Jan 24, 2024
6b3041d
Handle error in frontend
Jan 25, 2024
2a896fd
Merge branch 'master' into send-error-on-download-error
prakashsvmx Jan 25, 2024
baca065
Merge branch 'master' into send-error-on-download-error
cesnietor Jan 26, 2024
7b1b0f8
Merge branch 'master' into send-error-on-download-error
bexsoft Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/admin_inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
31 changes: 21 additions & 10 deletions api/user_objects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ func getDownloadObjectResponse(session *models.Principal, params objectApi.Downl
// override filename is set
decodeOverride, err := base64.StdEncoding.DecodeString(*params.OverrideFileName)
if err != nil {
fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to decode OverrideFileName: %v", err))
http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusBadRequest)
return
}

Expand All @@ -472,16 +474,16 @@ 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()))
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))
rw.WriteHeader(400)
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
Expand Down Expand Up @@ -510,8 +512,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)
fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to seek at offset %d: %v", start, err))
http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError)
return
}

Expand All @@ -524,7 +526,9 @@ 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))
ErrorWithContext(ctx, fmt.Errorf("unable to write all data to client: %v", err))
// You can't change headers after you already started writing the body.
// Handle incomplete write in client.
return
}
}), nil
Expand Down Expand Up @@ -610,7 +614,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))
fmtError := ErrorWithContext(ctx, fmt.Errorf("unable to parse encoded prefix %s: %v", encodedPrefix, err))
http.Error(rw, fmtError.APIError.DetailedMessage, http.StatusInternalServerError)
return
}

Expand All @@ -632,7 +637,10 @@ 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))
// You can't change headers after you already started writing the body.
// Handle incomplete write in client.
return
}
}), nil
}
Expand Down Expand Up @@ -754,7 +762,10 @@ 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))
// You can't change headers after you already started writing the body.
// Handle incomplete write in client.
return
}
}), nil
}
Expand Down
13 changes: 8 additions & 5 deletions web-app/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
Expand All @@ -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";
Expand All @@ -143,7 +146,7 @@ export const download = (
return;
}
}
errorCallback(`Unexpected response status code (${req.status}).`);
errorCallback(`Unexpected response, download incomplete.`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the status code does not help debugging, can we try to add more information here ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status by itself was also not very helpful. You'll need to see the logs for now. We were not exposing the errors before either.
Exposing detailed errors is in our backlog.

}
}
};
Expand Down
Loading