Add rejection logging to providers

Make sure it is easy to debug when one of the promises rejects (and
causes the whole promise chain to fail).

I'm not really happy with this, but still seemed better than adding
the same boilerplate error logging code to each provider one-by-one.

Change-Id: Idd2b638f012ef2ff250e350e2f6a60bb8b81899b
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/268
This commit is contained in:
Gergő Tisza 2014-03-03 19:04:01 +00:00
parent 4dad39c117
commit ead038c34f
8 changed files with 56 additions and 42 deletions

View file

@ -46,6 +46,33 @@
this.cache = {}; this.cache = {};
} }
/**
* Wraps a caching layer around a function returning a promise; if getCachedPromise has been
* called with the same key already, it will return the previous result.
*
* Since it is the promise and not the API response that gets cached, this method can ensure
* that there are no race conditions and multiple calls to the same resource: even if the
* request is still in progress, separate calls (with the same key) to getCachedPromise will
* share on the same promise object.
* The promise is cached even if it is rejected, so if the API request fails, all later calls
* to getCachedPromise will fail immediately without retrying the request.
*
* @param {string} key cache key
* @param {function(): jQuery.Promise} getPromise a function to get the promise on cache miss
*/
Api.prototype.getCachedPromise = function( key, getPromise ) {
var provider = this;
if ( !this.cache[key] ) {
this.cache[key] = getPromise();
this.cache[key].fail( function ( error ) {
// constructor.name is usually not reliable in inherited classes, but OOjs fixes that
mw.log( provider.constructor.name + ' provider failed to load: ', error );
} );
}
return this.cache[key];
};
/** /**
* Pulls an error message out of an API response. * Pulls an error message out of an API response.
* @param {Object} data * @param {Object} data

View file

@ -37,8 +37,8 @@
FileRepoInfo.prototype.get = function() { FileRepoInfo.prototype.get = function() {
var provider = this; var provider = this;
if ( !this.cache['*'] ) { return this.getCachedPromise( '*', function () {
this.cache['*'] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
meta: 'filerepoinfo', meta: 'filerepoinfo',
format: 'json' format: 'json'
@ -51,9 +51,7 @@
} ); } );
return reposHash; return reposHash;
} ); } );
} } );
return this.cache['*'];
}; };
mw.mmv.provider.FileRepoInfo = FileRepoInfo; mw.mmv.provider.FileRepoInfo = FileRepoInfo;

View file

@ -53,7 +53,6 @@
*/ */
GlobalUsage.prototype.get = function( file ) { GlobalUsage.prototype.get = function( file ) {
var provider = this, var provider = this,
cacheKey = file.getPrefixedDb(),
fileUsage; fileUsage;
if ( this.options.doNotUseApi ) { if ( this.options.doNotUseApi ) {
@ -63,14 +62,14 @@
return $.Deferred().resolve( fileUsage ); return $.Deferred().resolve( fileUsage );
} }
if ( !this.cache[cacheKey] ) { return this.getCachedPromise( file.getPrefixedDb(), function () {
this.cache[cacheKey] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
prop: 'globalusage', prop: 'globalusage',
titles: file.getPrefixedDb(), titles: file.getPrefixedDb(),
guprop: ['url', 'namespace'], guprop: ['url', 'namespace'],
gufilterlocal: 1, gufilterlocal: 1,
gulimit: this.options.apiLimit, gulimit: provider.options.apiLimit,
format: 'json' format: 'json'
} ).then( function( data ) { } ).then( function( data ) {
return provider.getQueryPage( file, data ); return provider.getQueryPage( file, data );
@ -90,9 +89,7 @@
!!( data['query-continue'] && data['query-continue'].globalusage ) !!( data['query-continue'] && data['query-continue'].globalusage )
); );
} ); } );
} } );
return this.cache[cacheKey];
}; };
mw.mmv.provider.GlobalUsage = GlobalUsage; mw.mmv.provider.GlobalUsage = GlobalUsage;

View file

@ -61,7 +61,9 @@
} }
} }
return this.cache[cacheKey]; return this.cache[cacheKey].fail( function ( error ) {
mw.log( provider.constructor + ' provider failed to load: ', error );
} );
}; };
/** /**

View file

@ -73,17 +73,16 @@
* @return {jQuery.Promise} a promise which resolves to an mw.mmv.model.Image object. * @return {jQuery.Promise} a promise which resolves to an mw.mmv.model.Image object.
*/ */
ImageInfo.prototype.get = function( file ) { ImageInfo.prototype.get = function( file ) {
var provider = this, var provider = this;
cacheKey = file.getPrefixedDb();
if ( !this.cache[cacheKey] ) { return this.getCachedPromise( file.getPrefixedDb(), function () {
this.cache[cacheKey] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
prop: 'imageinfo', prop: 'imageinfo',
titles: file.getPrefixedDb(), titles: file.getPrefixedDb(),
iiprop: this.iiprop, iiprop: provider.iiprop,
iiextmetadatafilter: this.iiextmetadatafilter, iiextmetadatafilter: provider.iiextmetadatafilter,
iiextmetadatalanguage: this.options.language, iiextmetadatalanguage: provider.options.language,
format: 'json' format: 'json'
} ).then( function( data ) { } ).then( function( data ) {
return provider.getQueryPage( file, data ); return provider.getQueryPage( file, data );
@ -96,9 +95,7 @@
return $.Deferred().reject( 'unknown error' ); return $.Deferred().reject( 'unknown error' );
} }
} ); } );
} } );
return this.cache[cacheKey];
}; };
mw.mmv.provider.ImageInfo = ImageInfo; mw.mmv.provider.ImageInfo = ImageInfo;

View file

@ -47,16 +47,15 @@
* @return {jQuery.Promise} * @return {jQuery.Promise}
*/ */
ImageUsage.prototype.get = function( file ) { ImageUsage.prototype.get = function( file ) {
var provider = this, var provider = this;
cacheKey = file.getPrefixedDb();
if ( !this.cache[cacheKey] ) { return this.getCachedPromise( file.getPrefixedDb(), function () {
this.cache[cacheKey] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
list: 'imageusage', list: 'imageusage',
iutitle: file.getPrefixedDb(), iutitle: file.getPrefixedDb(),
iunamespace: this.options.namespaces, iunamespace: provider.options.namespaces,
iulimit: this.options.apiLimit, iulimit: provider.options.apiLimit,
format: 'json' format: 'json'
} ).then( function( data ) { } ).then( function( data ) {
return provider.getQueryField( 'imageusage', data ); return provider.getQueryField( 'imageusage', data );
@ -76,9 +75,7 @@
!!( data['query-continue'] && data['query-continue'].imageusage ) !!( data['query-continue'] && data['query-continue'].imageusage )
); );
} ); } );
} } );
return this.cache[cacheKey];
}; };
mw.mmv.provider.ImageUsage = ImageUsage; mw.mmv.provider.ImageUsage = ImageUsage;

View file

@ -45,8 +45,8 @@
var provider = this, var provider = this,
cacheKey = file.getPrefixedDb() + '|' + ( width || '' ) + '|' + ( height || '' ); cacheKey = file.getPrefixedDb() + '|' + ( width || '' ) + '|' + ( height || '' );
if ( !this.cache[cacheKey] ) { return this.getCachedPromise( cacheKey, function () {
this.cache[cacheKey] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
prop: 'imageinfo', prop: 'imageinfo',
titles: file.getPrefixedDb(), titles: file.getPrefixedDb(),
@ -76,9 +76,7 @@
return $.Deferred().reject( 'unknown error' ); return $.Deferred().reject( 'unknown error' );
} }
} ); } );
} } );
return this.cache[cacheKey];
}; };
mw.mmv.provider.ThumbnailInfo = ThumbnailInfo; mw.mmv.provider.ThumbnailInfo = ThumbnailInfo;

View file

@ -53,8 +53,8 @@
cacheKey = cacheKey + '|' + repoInfo.apiUrl; // local and remote user names could conflict cacheKey = cacheKey + '|' + repoInfo.apiUrl; // local and remote user names could conflict
} }
if ( !this.cache[cacheKey] ) { return this.getCachedPromise( cacheKey, function () {
this.cache[cacheKey] = this.api.get( { return provider.api.get( {
action: 'query', action: 'query',
list: 'users', list: 'users',
ususers: username, ususers: username,
@ -69,9 +69,7 @@
return new mw.mmv.model.User( username, mw.mmv.model.User.Gender.UNKNOWN ); return new mw.mmv.model.User( username, mw.mmv.model.User.Gender.UNKNOWN );
} }
} ); } );
} } );
return this.cache[cacheKey];
}; };
mw.mmv.provider.UserInfo = UserInfo; mw.mmv.provider.UserInfo = UserInfo;