Skip to content

Commit

Permalink
Stop/alter some logging (#1764)
Browse files Browse the repository at this point in the history 
* Better error handling for Write Script Online.
* Better error handling for Upload Script. Fixes potential server trips and/or hangings.
* Repo URL change detected on GH for *formidable*... updated README.md linkage.
* Some symmetry in these areas.

Post #1730 #430 #37

Auto-merge
  • Loading branch information
@Martii
Martii committed Sep 22, 2020
1 parent b7fb365 commit 10d8c7a
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 13 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ Outdated dependencies list can also be achieved with `$ npm outdated`
[font-awesomeNPMUrl]: https://www.npmjs.com/package/font-awesome
[font-awesomeNPMVersionImage]: https://img.shields.io/npm/v/font-awesome.svg?style=flat

[formidableGHUrl]: https://github.com/felixge/node-formidable
[formidableDOCUrl]: https://github.com/felixge/node-formidable/blob/master/Readme.md
[formidableGHUrl]: https://github.com/node-formidable/formidable
[formidableDOCUrl]: https://github.com/node-formidable/formidable/blob/master/README.md
[formidableNPMUrl]: https://www.npmjs.com/package/formidable
[formidableNPMVersionImage]: https://img.shields.io/npm/v/formidable.svg?style=flat

Expand Down
11 changes: 10 additions & 1 deletion controllers/scriptStorage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2230,13 +2230,22 @@ exports.webhook = function (aReq, aRes) {

repoManager.loadSyncs(update, function (aErr) {
if (aErr) {
// These currently should be server side errors so always log
console.error(update, aErr);
return;
}

repoManager.loadScripts(update, function (aErr) {

var code = null;
if (aErr) {
console.error(update, aErr);
// Some errors could be user generated or dep generated user error,
// usually ignore those since handled with Sync model and visible
// to end user. We shouldn't be sending non-numeric codes.
code = (aErr instanceof statusError ? aErr.status.code : aErr.code);
if (code && !isNaN(code) && code >= 500) {
console.error(update, aErr);
}
}
});
});
Expand Down
141 changes: 131 additions & 10 deletions controllers/user.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1781,14 +1781,59 @@ exports.uploadScript = function (aReq, aRes, aNext) {

form = new formidable.IncomingForm();
form.parse(aReq, function (aErr, aFields, aFiles) {
// WARNING: No err handling
var msg = null;

if (aErr) {
if (aErr) {
msg = 'Unknown error when form parsing at `uploadScript`.'
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, 'Please contact Development.'].join(' ')
});
console.error(aErr);
return;
}
}

if (!aFields || (aFields && !aFields.uploadScript)) {
msg = '`uploadScript` field is missing.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: msg
});
console.error(msg);
return;
}

if (aFields.uploadScript !== 'true') {
msg = '`uploadScript` field is invalid :=';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, aFields.uploadScript].join(' ')
});
console.error([msg, aFields.uploadScript].join(' '));
return;
}

// TODO: Maybe add more fields validation

if (!aFiles) {
msg = 'Upload Script is missing File.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: msg
});
console.error(msg);
return;
}

//
var isLib = aReq.params.isLib;
var script = aFiles.script;
var stream = null;
var bufs = [];
var authedUser = aReq.session.user;
var msg = null;

// Reject missing files
if (!script) {
Expand Down Expand Up @@ -1818,36 +1863,71 @@ exports.uploadScript = function (aReq, aRes, aNext) {

// Reject huge file
if (script.size > settings.maximum_upload_script_size) {
msg = util.format('Selected file size is larger than maximum (%s bytes).',
settings.maximum_upload_script_size)
statusCodePage(aReq, aRes, aNext, {
statusCode: 400,
statusMessage: 'Selected file is too big.'
statusMessage: msg
});
return;
}

stream = fs.createReadStream(script.path);

stream.on('error', function(aErr) {
msg = 'Upload Script failed.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: msg
});
console.error(aErr);
return;
});

stream.on('data', function (aData) {
bufs.push(aData);
});

stream.on('end', function () {
User.findOne({ _id: authedUser._id }, function (aErr, aUser) {
// WARNING: No err handling
var msg = null;

if (aErr) {
msg = 'Unknown error when finding User at processing Script stream.'
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, 'Please contact Development.'].join(' ')
});
console.error(aErr);
return;
}

if (!aUser) {
msg = 'No user found.'
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: msg
});
return;
}

var bufferConcat = Buffer.concat(bufs);

scriptStorage.getMeta(bufs, function (aMeta) {
var msg = null;

if (!isLib && !!scriptStorage.findMeta(aMeta, 'UserLibrary')) {
msg = 'UserLibrary metadata block found while attempting to upload as a UserScript.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 400,
statusMessage:
'UserLibrary metadata block found while attempting to upload as a UserScript.'
statusMessage: msg
});
return;
} else if (isLib && !!!scriptStorage.findMeta(aMeta, 'UserLibrary')) {
msg = 'UserLibrary metadata block missing.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 400,
statusMessage: 'UserLibrary metadata block missing.'
statusMessage: msg
});
return;
}
Expand Down Expand Up @@ -1902,9 +1982,30 @@ exports.submitSource = function (aReq, aRes, aNext) {
function storeScript(aMeta, aSource) {

User.findOne({ _id: authedUser._id }, function (aErr, aUser) {
// WARNING: No err handling
var msg = null;

if (aErr) {
msg = 'Unknown error when finding User at `submitSource`.'
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, 'Please contact Development.'].join(' ')
});
console.error(aErr);
return;
}

if (!aUser) {
msg = 'No user found.'
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: msg
});
return;
}

scriptStorage.storeScript(aUser, aMeta, aSource, false, function (aErr, aScript) {
var msg = null;

var redirectUri = aScript
? ((aScript.isLib ? '/libs/' : '/scripts/') +
encodeURIComponent(helpers.cleanFilename(aScript.author)) +
Expand All @@ -1921,9 +2022,10 @@ exports.submitSource = function (aReq, aRes, aNext) {
}

if (!aScript) {
msg = 'No script found.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500, // NOTE: Watch point
statusMessage: 'No script'
statusCode: 500,
statusMessage: msg
});
return;
}
Expand Down Expand Up @@ -1955,7 +2057,26 @@ exports.submitSource = function (aReq, aRes, aNext) {
aScript.fork = fork;

aScript.save(function (aErr, aScript) {
// WARNING: No err handling
var msg = null;

if (aErr) {
msg = 'Unknown error when saving at `submitSource`.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, 'Please contact Development'].join(' ')
});
console.error(aErr);
return;
}
if (!aScript) {
msg = 'No script handle when saving at `submitSource`.';
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: [msg, 'Please contact Development'].join(' ')
});
console.error(msg)
return;
}

aRes.redirect(redirectUri);
});
Expand Down

0 comments on commit 10d8c7a

Please sign in to comment.