wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-11-24 06:24:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Update save failure logging for EditAttemptStep schema, simplify code

Browse source 
* Remove custom internal events in ArticleTarget for every error type.
  The indirection was just making it harder to figure out what data
  goes where.

* Centralize the actual logging in ArticleTarget, instead of doing it
  in a dozen methods.

* Directly use the error code from the API for 'save_failure_message'.
  Previously we'd lose the original error code and generate a new one
  in the event indirection stuff, except for 'responseUnknown'.

* Update 'save_failure_type' map. Remove unused error codes, update
  the ones that changed, and sort in the order in which the types are
  listed on the schema page.

Bug: T272162
Change-Id: Ied602c456f4b0e7e9bb135e3200bec5ce65641ba
This commit is contained in:
Bartosz Dziewoński 2021-01-15 20:06:20 +01:00
parent f246a0584c
commit df43a1ef96
2 changed files with 50 additions and 141 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

144
modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js
View file

@ -94,10 +94,6 @@ OO.inheritClass( ve.init.mw.ArticleTarget, ve.init.mw.Target );
/* Events */
/**
* @event editConflict
*/
/**
* @event save
* @param {Object} data Save data from the API, see ve.init.mw.ArticleTarget#saveComplete
@ -113,64 +109,8 @@ OO.inheritClass( ve.init.mw.ArticleTarget, ve.init.mw.Target );
*/
/**
* @event saveErrorEmpty
* Fired when save API returns no data object
*/
/**
* @event saveErrorSpamBlacklist
* Fired when save is considered spam or blacklisted
* TODO: Move this to the extension
*/
/**
* @event saveErrorAbuseFilter
* Fired when AbuseFilter throws warnings
* TODO: Move this to the extension
*/
/**
* @event saveErrorBadToken
* @param {boolean} willRetry Whether an automatic retry will occur
* Fired on save if we have to fetch a new edit token.
* This is mainly for analytical purposes.
*/
/**
* @event saveErrorNewUser
* Fired when user is logged in as a new user
*/
/**
* @event saveErrorCaptcha
* Fired when saveError indicates captcha field is required
* TODO: Move this to the extension
*/
/**
* @event saveErrorUnknown
* @param {string} errorMsg Error message shown to the user
* Fired for any other type of save error
*/
/**
* @event saveErrorPageDeleted
* Fired when user tries to save page that was deleted after opening VE
*/
/**
* @event saveErrorTitleBlacklist
* Fired when the user tries to save page in violation of the TitleBlacklist
*/
/**
* @event saveErrorHookAborted
* Fired when the user tries to save page in violation of an extension
*/
/**
* @event saveErrorReadOnly
* Fired when the user tries to save page but the database is locked
* @event saveError
* @param {string} code Error code
*/
/**
@ -656,8 +596,9 @@ ve.init.mw.ArticleTarget.prototype.saveComplete = function ( data ) {
* @param {Object|null} data Full API response data, or XHR error details
*/
ve.init.mw.ArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry, code, data ) {
var name, handler, i, error,
var name, handler, i, error, errorCodes,
saveErrorHandlerFactory = ve.init.mw.saveErrorHandlerFactory,
handled = false,
target = this;
this.pageDeletedWarning = false;
@ -665,49 +606,66 @@ ve.init.mw.ArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry
// Handle empty response
if ( !data ) {
this.saveErrorEmpty();
return;
handled = true;
}
if ( data.errors ) {
if ( !handled && data.errors ) {
for ( i = 0; i < data.errors.length; i++ ) {
error = data.errors[ i ];
if ( error.code === 'badtoken' ) {
this.saveErrorBadToken();
handled = true;
} else if ( error.code === 'assertanonfailed' || error.code === 'assertuserfailed' || error.code === 'assertnameduserfailed' ) {
this.refreshUser().then( function ( username ) {
target.saveErrorNewUser( username );
}, function () {
target.saveErrorUnknown( data );
} );
return;
handled = true;
} else if ( error.code === 'editconflict' ) {
this.editConflict();
return;
handled = true;
} else if ( error.code === 'pagedeleted' ) {
this.saveErrorPageDeleted();
return;
handled = true;
} else if ( error.code === 'hookaborted' ) {
this.saveErrorHookAborted( data );
return;
handled = true;
} else if ( error.code === 'readonly' ) {
this.saveErrorReadOnly( data );
return;
handled = true;
}
}
}
for ( name in saveErrorHandlerFactory.registry ) {
handler = saveErrorHandlerFactory.lookup( name );
if ( handler.static.matchFunction( data ) ) {
handler.static.process( data, this );
// Error was handled
return;
if ( !handled ) {
for ( name in saveErrorHandlerFactory.registry ) {
handler = saveErrorHandlerFactory.lookup( name );
if ( handler.static.matchFunction( data ) ) {
handler.static.process( data, this );
handled = true;
}
}
}
// Handle (other) unknown and/or unrecoverable errors
this.saveErrorUnknown( data );
if ( !handled ) {
this.saveErrorUnknown( data );
handled = true;
}
if ( data.errors ) {
errorCodes = data.errors.map( function ( err ) {
return err.code;
} ).join( ',' );
} else if ( ve.getProp( data, 'visualeditoredit', 'edit', 'captcha' ) ) {
// Eww
errorCodes = 'captcha';
} else {
errorCodes = 'http-' + ( ( data.xhr && data.xhr.status ) || 0 );
}
this.emit( 'saveError', errorCodes );
};
/**
@ -739,39 +697,31 @@ ve.init.mw.ArticleTarget.prototype.extractErrorMessages = function ( data ) {
/**
* Handle general save error
*
* @fires saveErrorEmpty
*/
ve.init.mw.ArticleTarget.prototype.saveErrorEmpty = function () {
this.showSaveError(
ve.msg( 'visualeditor-saveerror', ve.msg( 'visualeditor-error-invalidresponse' ) ),
false /* prevents reapply */
);
this.emit( 'saveErrorEmpty' );
};
/**
* Handle hook abort save error
*
* @param {Object} data API response data
* @fires saveErrorHookAborted
*/
ve.init.mw.ArticleTarget.prototype.saveErrorHookAborted = function ( data ) {
this.showSaveError( this.extractErrorMessages( data ) );
this.emit( 'saveErrorHookAborted' );
};
/**
* Handle assert error indicating another user is logged in.
*
* @param {string|null} username Name of newly logged-in user, or null if anonymous
* @fires saveErrorNewUser
*/
ve.init.mw.ArticleTarget.prototype.saveErrorNewUser = function ( username ) {
var $msg;
this.emit( 'saveErrorNewUser' );
// TODO: Improve this message, concatenating it this way is a bad practice.
// This should read more like 'session_fail_preview' in MediaWiki core
// (but with the caveat that we know already whether you're logged in or not).
@ -789,12 +739,8 @@ ve.init.mw.ArticleTarget.prototype.saveErrorNewUser = function ( username ) {
/**
* Handle token fetch errors.
*
* @fires saveErrorBadToken
*/
ve.init.mw.ArticleTarget.prototype.saveErrorBadToken = function () {
this.emit( 'saveErrorBadToken', false );
// TODO: Improve this message, concatenating it this way is a bad practice.
// Also, it's not always true that you're "no longer logged in".
// This should read more like 'session_fail_preview' in MediaWiki core.
@ -808,53 +754,33 @@ ve.init.mw.ArticleTarget.prototype.saveErrorBadToken = function () {
* Handle unknown save error
*
* @param {Object|null} data API response data
* @fires saveErrorUnknown
*/
ve.init.mw.ArticleTarget.prototype.saveErrorUnknown = function ( data ) {
var errorCodes;
this.showSaveError( this.extractErrorMessages( data ), false );
if ( data.errors ) {
errorCodes = data.errors.map( function ( err ) {
return err.code;
} ).join( ',' );
} else {
errorCodes = 'http-' + ( ( data.xhr && data.xhr.status ) || 0 );
}
this.emit( 'saveErrorUnknown', errorCodes );
};
/**
* Handle page deleted error
*
* @fires saveErrorPageDeleted
*/
ve.init.mw.ArticleTarget.prototype.saveErrorPageDeleted = function () {
this.pageDeletedWarning = true;
// The API error message 'apierror-pagedeleted' is poor, make our own
this.showSaveError( mw.msg( 'visualeditor-recreate', mw.msg( 'ooui-dialog-process-continue' ) ), true, true );
this.emit( 'saveErrorPageDeleted' );
};
/**
* Handle read only error
*
* @param {Object} data API response data
* @fires saveErrorReadOnly
*/
ve.init.mw.ArticleTarget.prototype.saveErrorReadOnly = function ( data ) {
this.showSaveError( this.extractErrorMessages( data ), true, true );
this.emit( 'saveErrorReadOnly' );
};
/**
* Handle an edit conflict
*
* @fires editConflict
*/
ve.init.mw.ArticleTarget.prototype.editConflict = function () {
this.emit( 'editConflict' );
this.saveDialog.popPending();
this.saveDialog.swapPanel( 'conflict' );
};

47
modules/ve-mw/init/ve.init.mw.ArticleTargetEvents.js
View file

@ -23,18 +23,7 @@ ve.init.mw.ArticleTargetEvents = function VeInitMwArticleTargetEvents( target )
saveInitiated: 'onSaveInitiated',
save: 'onSaveComplete',
saveReview: 'onSaveReview',
saveErrorEmpty: [ 'trackSaveError', 'empty' ],
saveErrorSpamBlacklist: [ 'trackSaveError', 'spamblacklist' ],
saveErrorAbuseFilter: [ 'trackSaveError', 'abusefilter' ],
saveErrorBadToken: [ 'trackSaveError', 'badtoken' ],
saveErrorNewUser: [ 'trackSaveError', 'newuser' ],
saveErrorPageDeleted: [ 'trackSaveError', 'pagedeleted' ],
saveErrorHookAborted: [ 'trackSaveError', 'responseUnknown' ], // FIXME: Make a specific one.
saveErrorTitleBlacklist: [ 'trackSaveError', 'titleblacklist' ],
saveErrorCaptcha: [ 'trackSaveError', 'captcha' ],
saveErrorReadOnly: [ 'trackSaveError', 'unknown', 'readonly' ],
saveErrorUnknown: [ 'trackSaveError', 'unknown' ],
editConflict: [ 'trackSaveError', 'editconflict' ],
saveError: 'trackSaveError',
surfaceReady: 'onSurfaceReady',
showChanges: 'onShowChanges',
showChangesError: 'onShowChangesError',
@ -133,49 +122,43 @@ ve.init.mw.ArticleTargetEvents.prototype.onSaveComplete = function ( data ) {
/**
* Track a save error by type
*
* @param {string} type Text for error type
* @param {string} code Error code
*/
ve.init.mw.ArticleTargetEvents.prototype.trackSaveError = function ( type ) {
ve.init.mw.ArticleTargetEvents.prototype.trackSaveError = function ( code ) {
var key, data,
failureArguments = [],
// Maps mwtiming types to mwedit types
typeMap = {
badtoken: 'userBadToken',
newuser: 'userNewUser',
abusefilter: 'extensionAbuseFilter',
assertanonfailed: 'userNewUser',
assertuserfailed: 'userNewUser',
assertnameduserfailed: 'userNewUser',
'abusefilter-disallowed': 'extensionAbuseFilter',
'abusefilter-warning': 'extensionAbuseFilter',
captcha: 'extensionCaptcha',
spamblacklist: 'extensionSpamBlacklist',
empty: 'responseEmpty',
unknown: 'responseUnknown',
'titleblacklist-forbidden': 'extensionTitleBlacklist',
pagedeleted: 'editPageDeleted',
titleblacklist: 'extensionTitleBlacklist',
editconflict: 'editConflict'
},
// Types that are logged as performance.user.saveError.{type}
// Types that are logged as performance.user.saveError.{code}
// (for historical reasons; this sucks)
specialTypes = [ 'editconflict' ];
if ( arguments ) {
failureArguments = Array.prototype.slice.call( arguments, 1 );
}
key = 'performance.user.saveError';
if ( specialTypes.indexOf( type ) !== -1 ) {
key += '.' + type;
if ( specialTypes.indexOf( code ) !== -1 ) {
key += '.' + code;
}
this.trackTiming( key, {
duration: ve.now() - this.timings.saveInitiated,
retries: this.timings.saveRetries,
type: type
type: code
} );
data = {
type: typeMap[ type ] || 'responseUnknown',
message: code,
type: typeMap[ code ] || 'responseUnknown',
timing: ve.now() - this.timings.saveInitiated + ( this.timings.serializeForCache || 0 )
};
if ( type === 'unknown' && failureArguments[ 0 ] ) {
data.message = failureArguments[ 0 ];
}
this.track( 'mwedit.saveFailure', data );
};