upload: Make filedrop error handling more consistent.

The first argument to the error callback is *usually* a string code
from a list in the filedrop source; but sometimes it was the text
the server sent in the HTTP status line, instead.  The latter isn't
predictable, and so it's not possible to write app code that uses it
to handle error consistently.

Instead, use that parameter for the numeric HTTP status code.  This
still isn't totally clean in that sometimes it's internal filedrop
errors, as strings, and sometimes it's HTTP status codes, as numbers;
but at least both of those are things we can sanely handle with a
`switch` statement.

Also pass through `serverResponse`, which for a nice JSON error from
the server will contain meaningful information about the error which
the calling code can use for nice error handling.  And just drop the
HTTP status text, which at best is redundant with the numeric code.

In passing, fix one case where for no obvious reason filedrop was
passing the file object but not the index.

This should be a pure refactor.
This commit is contained in:
Greg Price
2018-01-26 18:50:37 -08:00
committed by Greg Price
parent fe787c617c
commit d053e07760
3 changed files with 11 additions and 11 deletions

View File

@@ -68,7 +68,7 @@ zrequire('upload');
function test(err, file, msg) {
setup_test();
upload.uploadError(err, file);
upload.uploadError(err, null, file);
assert_side_effects(msg);
}
@@ -82,7 +82,7 @@ zrequire('upload');
test('BrowserNotSupported', {}, msg_prefix + msg_1);
test('TooManyFiles', {}, msg_prefix + msg_2);
test('FileTooLarge', {name: 'foobar.txt'}, msg_prefix + msg_3);
test('REQUEST ENTITY TOO LARGE', {}, msg_prefix + msg_4);
test(413, {}, msg_prefix + msg_4);
test('Do-not-match-any-case', {}, msg_prefix + msg_5);
}());

View File

@@ -35,12 +35,12 @@ exports.progressUpdated = function (i, file, progress) {
$("#compose-upload-bar").width(progress + "%");
};
exports.uploadError = function (err, file) {
exports.uploadError = function (error_code, server_response, file) {
var msg;
$("#compose-send-status").addClass("alert-error")
.removeClass("alert-info");
$("#compose-send-button").prop("disabled", false);
switch (err) {
switch (error_code) {
case 'BrowserNotSupported':
msg = i18n.t("File upload is not yet available for your browser.");
break;
@@ -54,7 +54,7 @@ exports.uploadError = function (err, file) {
};
msg = i18n.t('"__file_name__" was too large; the maximum file size is 25MiB.', context);
break;
case 'REQUEST ENTITY TOO LARGE':
case 413: // HTTP status "Request Entity Too Large"
msg = i18n.t("Sorry, the file was too large.");
break;
default:

View File

@@ -76,7 +76,7 @@
beforeEach: empty,
afterAll: empty,
rename: empty,
error: function(err, file, i, status) {
error: function(err, response, file, i) {
alert(err);
},
uploadStarted: empty,
@@ -362,7 +362,7 @@
// Pass any errors to the error option
if (xhr.status < 200 || xhr.status > 299) {
on_error(xhr.statusText, xhr.status);
on_error(xhr.status, serverResponse);
}
};
@@ -380,7 +380,7 @@
if (opts.allowedfiletypes.push && opts.allowedfiletypes.length) {
for(var fileIndex = files.length;fileIndex--;) {
if(!files[fileIndex].type || $.inArray(files[fileIndex].type, opts.allowedfiletypes) < 0) {
opts.error(errors[3], files[fileIndex]);
opts.error(errors[3], null, files[fileIndex], fileIndex);
return false;
}
}
@@ -442,7 +442,7 @@
reader.index = fileIndex;
if (files[fileIndex].size > max_file_size) {
opts.error(errors[2], files[fileIndex], fileIndex);
opts.error(errors[2], null, files[fileIndex], fileIndex);
// Remove from queue
processingQueue.forEach(function(value, key) {
if (value === fileIndex) {
@@ -530,8 +530,8 @@
return result;
}
function on_error(status_text, status) {
opts.error(status_text, file, fileIndex, status);
function on_error(status_code, response) {
opts.error(status_code, response, file, fileIndex);
}
var fileName,