jenkins-bot has submitted this change and it was merged.
Change subject: Use httpStatus instead of code as the property on errors
......................................................................
Use httpStatus instead of code as the property on errors
* domino, and probably other libraries, set .code on their errors. We
should use a verbose thing that's hard to clash for when we're explicitly
setting http status codes.
* Can test that now enwiki/WWL_World_Trios_Championship?oldid=678143660
returns a 500 instead of a 5.
Bug: T121611
Change-Id: I60364ebee02d954721110eeec16e0ae80061a938
---
M lib/api/apiUtils.js
M lib/api/routes.js
M lib/config/MWParserEnvironment.js
M lib/logger/LogData.js
M lib/mw/ApiRequest.js
M tests/mockAPI.js
6 files changed, 18 insertions(+), 24 deletions(-)
Approvals:
Subramanya Sastry: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/api/apiUtils.js b/lib/api/apiUtils.js
index 88607fd..e570f69 100644
--- a/lib/api/apiUtils.js
+++ b/lib/api/apiUtils.js
@@ -38,14 +38,14 @@
* @param {Object} args
*/
apiUtils.relativeRedirect = function(args) {
- if (!args.code) {
- args.code = 302; // moved temporarily
+ if (!args.httpStatus) {
+ args.httpStatus = 302; // moved temporarily
}
if (args.res && args.env && args.env.responseSent) {
return;
} else {
- args.res.writeHead(args.code, {
+ args.res.writeHead(args.httpStatus, {
'Location': args.path,
});
args.res.end();
@@ -548,7 +548,7 @@
var dp = obj['data-parsoid'];
if (!dp || !dp.body || dp.body.constructor !== Object || !dp.body.ids) {
var err = new Error('Invalid data-parsoid was provided.');
- err.code = 400;
+ err.httpStatus = 400;
err.suppressLoggingStack = true;
throw err;
}
@@ -560,11 +560,11 @@
* @method
* @param {MWParserEnvironment} env
* @param {String} text
- * @param {Number} [code]
+ * @param {Number} [httpStatus]
*/
-apiUtils.fatalRequest = function(env, text, code) {
+apiUtils.fatalRequest = function(env, text, httpStatus) {
var err = new Error(text);
- err.code = code || 404;
+ err.httpStatus = httpStatus || 404;
err.suppressLoggingStack = true;
env.log('fatal/request', err);
};
diff --git a/lib/api/routes.js b/lib/api/routes.js
index 21d2e67..bb87228 100644
--- a/lib/api/routes.js
+++ b/lib/api/routes.js
@@ -25,12 +25,12 @@
// This helper is only to be used in middleware, before an environment
// is setup. The logger doesn't emit the expected location info.
// You probably want `apiUtils.fatalRequest` instead.
- var errOut = function(res, text, code) {
+ var errOut = function(res, text, httpStatus) {
var err = new Error(text);
- err.code = code || 404;
+ err.httpStatus = httpStatus || 404;
err.suppressLoggingStack = true;
processLogger.log('fatal/request', err);
- apiUtils.sendResponse(res, {}, text, err.code);
+ apiUtils.sendResponse(res, {}, text, err.httpStatus);
};
// Middlewares
@@ -123,7 +123,7 @@
}
res.once('finish', resolve);
apiUtils.setHeader(res, env,
'content-type', 'text/plain;charset=utf-8');
- apiUtils.sendResponse(res, env,
logData.fullMsg(), logData.flatLogObject().code || 500);
+ apiUtils.sendResponse(res, env,
logData.fullMsg(), logData.flatLogObject().httpStatus || 500);
}).catch(function(e) {
console.error(e.stack || e);
res.end();
diff --git a/lib/config/MWParserEnvironment.js
b/lib/config/MWParserEnvironment.js
index cba8436..a96be77 100644
--- a/lib/config/MWParserEnvironment.js
+++ b/lib/config/MWParserEnvironment.js
@@ -124,7 +124,7 @@
this.message = message ||
"Refusing to process the request because the payload is " +
"larger than the server is willing or able to handle.";
- this.code = 413;
+ this.httpStatus = 413;
this.suppressLoggingStack = true;
}
PayloadTooLargeError.prototype = Error.prototype;
diff --git a/lib/logger/LogData.js b/lib/logger/LogData.js
index 4f7a9ca..e2fdb99 100644
--- a/lib/logger/LogData.js
+++ b/lib/logger/LogData.js
@@ -147,8 +147,8 @@
// We return a defined value to avoid generating a
stack above.
stack: o.suppressLoggingStack ? "" : o.stack,
};
- if (o.code) {
- f.code = o.code;
+ if (o.httpStatus) {
+ f.httpStatus = o.httpStatus;
}
return f;
} else if (typeof (o) === 'function') {
diff --git a/lib/mw/ApiRequest.js b/lib/mw/ApiRequest.js
index 07b4888..b8392cd 100644
--- a/lib/mw/ApiRequest.js
+++ b/lib/mw/ApiRequest.js
@@ -199,7 +199,7 @@
Error.captureStackTrace(this, DoesNotExistError);
this.name = "DoesNotExistError";
this.message = message || "Something doesn't exist";
- this.code = 404;
+ this.httpStatus = 404;
this.suppressLoggingStack = true;
}
DoesNotExistError.prototype = Error.prototype;
@@ -212,7 +212,7 @@
Error.captureStackTrace(this, ParserError);
this.name = "ParserError";
this.message = message || "Generic parser error";
- this.code = 500;
+ this.httpStatus = 500;
}
ParserError.prototype = Error.prototype;
@@ -224,7 +224,7 @@
Error.captureStackTrace(this, AccessDeniedError);
this.name = 'AccessDeniedError';
this.message = message || 'Your wiki requires a logged-in account to
access the API.';
- this.code = 401;
+ this.httpStatus = 401;
}
AccessDeniedError.prototype = Error.prototype;
diff --git a/tests/mockAPI.js b/tests/mockAPI.js
index 4fc08b8..f622a25 100644
--- a/tests/mockAPI.js
+++ b/tests/mockAPI.js
@@ -425,13 +425,7 @@
res.end();
} else {
res.setHeader('Content-Type', 'text/plain');
-
- if (err.code) {
- res.status(err.code);
- } else {
- res.status(500);
- }
-
+ res.status(err.httpStatus || 500);
res.write(err.stack || err.toString());
res.end();
}
--
To view, visit https://gerrit.wikimedia.org/r/259626
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I60364ebee02d954721110eeec16e0ae80061a938
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Arlolra <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: GWicke <[email protected]>
Gerrit-Reviewer: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
