Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

morebits: more robust failure handling in wiki.api and wiki.page

To answer the comment on line 2570/2606, which I am removing, we certainly do need the failure callbacks.
They are crucial for tracking the completion of individual tasks in a batch operation.
Without failure callbacks, a failing task looks the same as one that hasn't completed yet, meaning the batch operation can hang if there are too many individual failures.

As for wiki.api's error handling, I think it's fine; UncleDouggie wrote that comment back in e8cc372, and only he knows what he had in mind for wiki.api.

Also fixing a long-standing bug - "invalid title" errors were never caught properly.
  • Loading branch information...
commit 66409277811786f4de2a61af6870338eff1b2038 1 parent b299d3f
@atlight atlight authored
Showing with 77 additions and 42 deletions.
  1. +77 −42 morebits.js
View
119 morebits.js
@@ -1399,15 +1399,14 @@ Morebits.wiki.api.prototype = {
},
returnError: function() {
-
+ this.statelem.error( this.errorText );
+
// invoke failure callback if one was supplied
if (this.onError) {
// set the callback context to this.parent for new code and supply the API object
// as the first argument to the callback for legacy code
this.onError.call( this.parent, this );
- } else {
- this.statelem.error( this.errorText );
}
// don't complete the action so that the error remains displayed
},
@@ -1689,6 +1688,8 @@ Morebits.wiki.page = function(pageName, currentAction) {
stabilizeProcessApi: null
};
+ var emptyFunction = function() { };
+
/**
* Public interface accessors
*/
@@ -1829,11 +1830,12 @@ Morebits.wiki.page = function(pageName, currentAction) {
this.load = function(onSuccess, onFailure) {
ctx.onLoadSuccess = onSuccess;
- ctx.onLoadFailure = onFailure;
+ ctx.onLoadFailure = onFailure || emptyFunction;
// Need to be able to do something after the page loads
if (!onSuccess) {
ctx.statusElement.error("Internal error: no onSuccess callback provided to load()!");
+ ctx.onLoadFailure(this);
return;
}
@@ -1862,7 +1864,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
ctx.loadQuery.inprop = 'protection';
}
- ctx.loadApi = new Morebits.wiki.api("Retrieving page...", ctx.loadQuery, fnLoadSuccess, ctx.statusElement);
+ ctx.loadApi = new Morebits.wiki.api("Retrieving page...", ctx.loadQuery, fnLoadSuccess, ctx.statusElement, ctx.onLoadFailure);
ctx.loadApi.setParent(this);
ctx.loadApi.post();
};
@@ -1870,12 +1872,17 @@ Morebits.wiki.page = function(pageName, currentAction) {
// Save updated .pageText to Wikipedia
// Only valid after successful .load()
this.save = function(onSuccess, onFailure) {
+ ctx.onSaveSuccess = onSuccess;
+ ctx.onSaveFailure = onFailure || emptyFunction;
+
if (!ctx.pageLoaded) {
ctx.statusElement.error("Internal error: attempt to save a page that has not been loaded!");
+ ctx.onSaveFailure(this);
return;
}
if (!ctx.editSummary) {
ctx.statusElement.error("Internal error: edit summary not set before save!");
+ ctx.onSaveFailure(this);
return;
}
@@ -1883,11 +1890,10 @@ Morebits.wiki.page = function(pageName, currentAction) {
(ctx.fullyProtected === 'infinity' ? '" (protected indefinitely)' : ('" (protection expiring ' + ctx.fullyProtected + ')')) +
'. \n\nClick OK to proceed with the edit, or Cancel to skip this edit.')) {
ctx.statusElement.error("Edit to fully protected page was aborted.");
+ ctx.onSaveFailure(this);
return;
}
- ctx.onSaveSuccess = onSuccess;
- ctx.onSaveFailure = onFailure;
ctx.retries = 0;
var query = {
@@ -1945,15 +1951,15 @@ Morebits.wiki.page = function(pageName, currentAction) {
this.append = function(onSuccess, onFailure) {
ctx.editMode = 'append';
ctx.onSaveSuccess = onSuccess;
- ctx.onSaveFailure = onFailure;
- this.load(fnAutoSave, onFailure);
+ ctx.onSaveFailure = onFailure || emptyFunction;
+ this.load(fnAutoSave, ctx.onSaveFailure);
};
this.prepend = function(onSuccess, onFailure) {
ctx.editMode = 'prepend';
ctx.onSaveSuccess = onSuccess;
- ctx.onSaveFailure = onFailure;
- this.load(fnAutoSave, onFailure);
+ ctx.onSaveFailure = onFailure || emptyFunction;
+ this.load(fnAutoSave, ctx.onSaveFailure);
};
this.lookupCreator = function(onSuccess) {
@@ -2009,29 +2015,34 @@ Morebits.wiki.page = function(pageName, currentAction) {
};
this.revert = function(onSuccess, onFailure) {
+ ctx.onSaveSuccess = onSuccess;
+ ctx.onSaveFailure = onFailure || emptyFunction;
+
if (!ctx.revertOldID) {
ctx.statusElement.error("Internal error: revision ID to revert to was not set before revert!");
+ ctx.onSaveFailure(this);
return;
}
+
ctx.editMode = 'revert';
- ctx.onSaveSuccess = onSuccess;
- ctx.onSaveFailure = onFailure;
- this.load(fnAutoSave, onFailure);
+ this.load(fnAutoSave, ctx.onSaveFailure);
};
this.move = function(onSuccess, onFailure) {
+ ctx.onMoveSuccess = onSuccess;
+ ctx.onMoveFailure = onFailure || emptyFunction;
+
if (!ctx.editSummary) {
ctx.statusElement.error("Internal error: move reason not set before move (use setEditSummary function)!");
+ ctx.onMoveFailure(this);
return;
}
if (!ctx.moveDestination) {
ctx.statusElement.error("Internal error: destination page name was not set before move!");
+ ctx.onMoveFailure(this);
return;
}
- ctx.onMoveSuccess = onSuccess;
- ctx.onMoveFailure = onFailure;
-
var query = {
action: 'query',
prop: 'info',
@@ -2045,26 +2056,28 @@ Morebits.wiki.page = function(pageName, currentAction) {
query.inprop = 'protection';
}
- ctx.moveApi = new Morebits.wiki.api("retrieving move token...", query, fnProcessMove, ctx.statusElement);
+ ctx.moveApi = new Morebits.wiki.api("retrieving move token...", query, fnProcessMove, ctx.statusElement, ctx.onMoveFailure);
ctx.moveApi.setParent(this);
ctx.moveApi.post();
};
// |delete| is a reserved word in some flavours of JS
this.deletePage = function(onSuccess, onFailure) {
+ ctx.onDeleteSuccess = onSuccess;
+ ctx.onDeleteFailure = onFailure || emptyFunction;
+
// if a non-admin tries to do this, don't bother
if (!Morebits.userIsInGroup('sysop')) {
ctx.statusElement.error("Cannot delete page: only admins can do that");
+ ctx.onDeleteFailure(this);
return;
}
if (!ctx.editSummary) {
ctx.statusElement.error("Internal error: delete reason not set before delete (use setEditSummary function)!");
+ ctx.onDeleteFailure(this);
return;
}
- ctx.onDeleteSuccess = onSuccess;
- ctx.onDeleteFailure = onFailure;
-
var query = {
action: 'query',
prop: 'info',
@@ -2076,29 +2089,32 @@ Morebits.wiki.page = function(pageName, currentAction) {
query.redirects = ''; // follow all redirects
}
- ctx.deleteApi = new Morebits.wiki.api("retrieving delete token...", query, fnProcessDelete, ctx.statusElement);
+ ctx.deleteApi = new Morebits.wiki.api("retrieving delete token...", query, fnProcessDelete, ctx.statusElement, ctx.onDeleteFailure);
ctx.deleteApi.setParent(this);
ctx.deleteApi.post();
};
this.protect = function(onSuccess, onFailure) {
+ ctx.onProtectSuccess = onSuccess;
+ ctx.onProtectFailure = onFailure || emptyFunction;
+
// if a non-admin tries to do this, don't bother
if (!Morebits.userIsInGroup('sysop')) {
ctx.statusElement.error("Cannot protect page: only admins can do that");
+ ctx.onProtectFailure(this);
return;
}
if (!ctx.protectEdit && !ctx.protectMove && !ctx.protectCreate) {
ctx.statusElement.error("Internal error: you must set edit and/or move and/or create protection before calling protect()!");
+ ctx.onProtectFailure(this);
return;
}
if (!ctx.editSummary) {
ctx.statusElement.error("Internal error: protection reason not set before protect (use setEditSummary function)!");
+ ctx.onProtectFailure(this);
return;
}
- ctx.onProtectSuccess = onSuccess;
- ctx.onProtectFailure = onFailure;
-
var query = {
action: 'query',
prop: 'info',
@@ -2110,7 +2126,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
query.redirects = ''; // follow all redirects
}
- ctx.protectApi = new Morebits.wiki.api("retrieving protect token...", query, fnProcessProtect, ctx.statusElement);
+ ctx.protectApi = new Morebits.wiki.api("retrieving protect token...", query, fnProcessProtect, ctx.statusElement, ctx.onProtectFailure);
ctx.protectApi.setParent(this);
ctx.protectApi.post();
};
@@ -2119,23 +2135,26 @@ Morebits.wiki.page = function(pageName, currentAction) {
// only works where $wgFlaggedRevsProtection = true (i.e. where FlaggedRevs
// settings appear on the wiki's "protect" tab)
this.stabilize = function(onSuccess, onFailure) {
+ ctx.onStabilizeSuccess = onSuccess;
+ ctx.onStabilizeFailure = onFailure || emptyFunction;
+
// if a non-admin tries to do this, don't bother
if (!Morebits.userIsInGroup('sysop')) {
ctx.statusElement.error("Cannot apply FlaggedRevs settings: only admins can do that");
+ ctx.onStabilizeFailure(this);
return;
}
if (!ctx.flaggedRevs) {
ctx.statusElement.error("Internal error: you must set flaggedRevs before calling stabilize()!");
+ ctx.onStabilizeFailure(this);
return;
}
if (!ctx.editSummary) {
ctx.statusElement.error("Internal error: reason not set before calling stabilize() (use setEditSummary function)!");
+ ctx.onStabilizeFailure(this);
return;
}
- ctx.onStabilizeSuccess = onSuccess;
- ctx.onStabilizeFailure = onFailure;
-
var query = {
action: 'query',
prop: 'info|flagged',
@@ -2146,7 +2165,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
query.redirects = ''; // follow all redirects
}
- ctx.stabilizeApi = new Morebits.wiki.api("retrieving stabilize token...", query, fnProcessStabilize, ctx.statusElement);
+ ctx.stabilizeApi = new Morebits.wiki.api("retrieving stabilize token...", query, fnProcessStabilize, ctx.statusElement, ctx.onStabilizeFailure);
ctx.stabilizeApi.setParent(this);
ctx.stabilizeApi.post();
};
@@ -2166,7 +2185,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
var fnLoadSuccess = function() {
var xml = ctx.loadApi.getXML();
- if ( !fnCheckPageName(xml) ) {
+ if ( !fnCheckPageName(xml, ctx.onLoadFailure) ) {
return; // abort
}
@@ -2191,12 +2210,14 @@ Morebits.wiki.page = function(pageName, currentAction) {
if (!ctx.editToken)
{
ctx.statusElement.error("Failed to retrieve edit token.");
+ ctx.onLoadFailure(this);
return;
}
ctx.loadTime = $(xml).find('page').attr('starttimestamp');
if (!ctx.loadTime)
{
ctx.statusElement.error("Failed to retrieve start timestamp.");
+ ctx.onLoadFailure(this);
return;
}
ctx.lastEditTime = $(xml).find('page').attr('touched');
@@ -2205,6 +2226,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
ctx.revertCurID = $(xml).find('rev').attr('revid');
if (!ctx.revertCurID) {
ctx.statusElement.error("Failed to retrieve current revision ID.");
+ ctx.onLoadFailure(this);
return;
}
ctx.revertUser = $(xml).find('rev').attr('user');
@@ -2213,6 +2235,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
ctx.revertUser = "<username hidden>";
} else {
ctx.statusElement.error("Failed to retrieve user who made the revision.");
+ ctx.onLoadFailure(this);
return;
}
}
@@ -2227,11 +2250,15 @@ Morebits.wiki.page = function(pageName, currentAction) {
};
// helper function to parse the page name returned from the API
- var fnCheckPageName = function(xml) {
-
+ var fnCheckPageName = function(xml, onFailure) {
+ if (!onFailure) {
+ onFailure = emptyFunction;
+ }
+
// check for invalid titles
- if ( $(xml).find('page').attr('invalid') ) {
- ctx.statusElement.error("Attempt to edit a page with invalid title: " + ctx.pageName);
+ if ( $(xml).find('page').attr('invalid') === "" ) {
+ ctx.statusElement.error("The page title is invalid: " + ctx.pageName);
+ onFailure(this);
return false; // abort
}
@@ -2248,6 +2275,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
else {
// could be a circular redirect or other problem
ctx.statusElement.error("Could not resolve redirects for: " + ctx.pageName);
+ onFailure(this);
// force error to stay on the screen
++Morebits.wiki.numberOfActionsLeft;
@@ -2295,6 +2323,8 @@ Morebits.wiki.page = function(pageName, currentAction) {
// force error to stay on the screen
++Morebits.wiki.numberOfActionsLeft;
+
+ ctx.onSaveFailure(this);
};
// callback from saveApi.post()
@@ -2371,6 +2401,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
if ($(xml).find('page').attr('missing') === "") {
ctx.statusElement.error("Cannot move the page, because it no longer exists");
+ ctx.onMoveFailure(this);
return;
}
@@ -2381,6 +2412,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
(editprot.attr('expiry') === 'infinity' ? '" (protected indefinitely)' : ('" (protection expiring ' + editprot.attr('expiry') + ')')) +
'. \n\nClick OK to proceed with the move, or Cancel to skip this move.')) {
ctx.statusElement.error("Move of fully protected page was aborted.");
+ ctx.onMoveFailure(this);
return;
}
}
@@ -2388,6 +2420,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
var moveToken = $(xml).find('page').attr('movetoken');
if (!moveToken) {
ctx.statusElement.error("Failed to retrieve move token.");
+ ctx.onMoveFailure(this);
return;
}
@@ -2421,6 +2454,7 @@ Morebits.wiki.page = function(pageName, currentAction) {
if ($(xml).find('page').attr('missing') === "") {
ctx.statusElement.error("Cannot delete the page, because it no longer exists");
+ ctx.onDeleteFailure(this);
return;
}
@@ -2430,12 +2464,14 @@ Morebits.wiki.page = function(pageName, currentAction) {
(editprot.attr('expiry') === 'infinity' ? '" (protected indefinitely)' : ('" (protection expiring ' + editprot.attr('expiry') + ')')) +
'. \n\nClick OK to proceed with the deletion, or Cancel to skip this deletion.')) {
ctx.statusElement.error("Deletion of fully protected page was aborted.");
+ ctx.onDeleteFailure(this);
return;
}
var deleteToken = $(xml).find('page').attr('deletetoken');
if (!deleteToken) {
ctx.statusElement.error("Failed to retrieve delete token.");
+ ctx.onDeleteFailure(this);
return;
}
@@ -2460,23 +2496,21 @@ Morebits.wiki.page = function(pageName, currentAction) {
var missing = ($(xml).find('page').attr('missing') === "");
if (((ctx.protectEdit || ctx.protectMove) && missing)) {
ctx.statusElement.error("Cannot protect the page, because it no longer exists");
+ ctx.onProtectFailure(this);
return;
}
if (ctx.protectCreate && !missing) {
ctx.statusElement.error("Cannot create protect the page, because it already exists");
+ ctx.onProtectFailure(this);
return;
}
- // cascading protection not possible on edit<sysop
- // XXX fix this logic - I can't wrap my head around it
- //if (ctx.protectCascade && (editprot && editprot.attr('level') !== 'sysop') && (ctx.protectEdit && ctx.protectEdit.level !== 'sysop')) {
- // ctx.statusElement.error("Internal error: cascading protection requires sysop-level edit protection!");
- // return;
- //}
+ // TODO cascading protection not possible on edit<sysop
var protectToken = $(xml).find('page').attr('protecttoken');
if (!protectToken) {
ctx.statusElement.error("Failed to retrieve protect token.");
+ ctx.onProtectFailure(this);
return;
}
@@ -2539,12 +2573,14 @@ Morebits.wiki.page = function(pageName, currentAction) {
var missing = ($(xml).find('page').attr('missing') === "");
if (missing) {
ctx.statusElement.error("Cannot protect the page, because it no longer exists");
+ ctx.onStabilizeFailure(this);
return;
}
var stabilizeToken = $(xml).find('page').attr('edittoken');
if (!stabilizeToken) {
ctx.statusElement.error("Failed to retrieve stabilize token.");
+ ctx.onStabilizeFailure(this);
return;
}
@@ -2567,7 +2603,6 @@ Morebits.wiki.page = function(pageName, currentAction) {
}; // end Morebits.wiki.page
/** Morebits.wiki.page TODO: (XXX)
- * - Do we need the onFailure callbacks? How do we know when to call them? Timeouts? Enhance Morebits.wiki.api for failures?
* - Should we retry loads also?
* - Need to reset current action before the save?
* - Deal with action.completed stuff
Please sign in to comment.
Something went wrong with that request. Please try again.