Mobrovac has submitted this change and it was merged.
Change subject: Fix lack of response when export fails
......................................................................
Fix lack of response when export fails
During failed export, no error was being
returned because fillBody function was
only filling the body with errors that
occured before export. This fills the body
with errors that occur after export.
Unfortunately, for some reason this
response is being returned as a Buffer
and therefore acts up in the browser,
although it demonstrates the correct
behaviour in curl.
Bug: T115271
Change-Id: I09e40682521f4d99775c4669238117b813985521
---
M lib/CitoidRequest.js
M lib/Exporter.js
A test/features/scraping/mockZotero.js
M test/features/scraping/noZotero.js
A test/utils/mockZoteroServer.js
5 files changed, 127 insertions(+), 50 deletions(-)
Approvals:
Mobrovac: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/CitoidRequest.js b/lib/CitoidRequest.js
index 2b2a490..624e6af 100644
--- a/lib/CitoidRequest.js
+++ b/lib/CitoidRequest.js
@@ -59,7 +59,6 @@
*/
CitoidRequest.prototype.fillBody = BBPromise.method(function(){
var self = this;
-
// Prevent body from accidentally being overwritten
if (this.response.body){
return BBPromise.reject('Body already filled');
@@ -77,7 +76,13 @@
if (!this.exporter){
return BBPromise.reject('No exporter registered with citoid service.');
}
- return this.exporter.export(this);
+ return this.exporter.export(self).then(function(cr){
+ if (cr.response.error){
+ // Fill body with error gotten during export, if present, and
return
+ cr.response.body = cr.response.error;
+ }
+ return cr;
+ });
});
CitoidRequest.prototype.getResponse = function(){
diff --git a/lib/Exporter.js b/lib/Exporter.js
index f7696a3..9ade946 100644
--- a/lib/Exporter.js
+++ b/lib/Exporter.js
@@ -33,6 +33,7 @@
this.stats = app.metrics;
this.zoteroService = null;
+
this.types = new CachedTypes();
defaultLogger = this.logger;
diff --git a/test/features/scraping/mockZotero.js
b/test/features/scraping/mockZotero.js
new file mode 100644
index 0000000..b50b5df
--- /dev/null
+++ b/test/features/scraping/mockZotero.js
@@ -0,0 +1,42 @@
+'use strict';
+
+/**
+ * Tests for when Zotero can translate but not export
+ */
+
+var preq = require('preq');
+var assert = require('../../utils/assert.js');
+var server = require('../../utils/server.js');
+var zotero = require('../../utils/mockZoteroServer.js');
+
+
+describe('mock Zotero service that cannot export', function() {
+
+ this.timeout(40000);
+
+ // Give Zotero port which is it is not running from-
+ // Mimics Zotero being down.
+ before(function () {
+ zotero.start(1968); // Start mock zotero server
+ return server.start({zoteroPort:1968}); // Start citoid server using
mock Zotero location
+ });
+
+ it('Get error for bibtex export from mock Zotero server', function() {
+ return server.query('http://www.example.com', 'bibtex', 'en')
+ .then(function(res) {
+ assert.status(res, 404);
+ }, function(err) {
+ assert.deepEqual(JSON.parse(err.body.toString()).Error,'Unable to
serve bibtex format at this time');
+ assert.status(err, 404);
+ //assert.checkError(err, 404, 'Unable to serve bibtex at this
time');
+ });
+ });
+
+ it('Success with bibtex export from mock Zotero server', function() {
+ return server.query('http://www.example.com').then(function(res) {
+ assert.status(res, 200);
+ assert.checkCitation(res, 'Example Domain');
+ });
+ });
+
+});
\ No newline at end of file
diff --git a/test/features/scraping/noZotero.js
b/test/features/scraping/noZotero.js
index 0882271..5d718e7 100644
--- a/test/features/scraping/noZotero.js
+++ b/test/features/scraping/noZotero.js
@@ -11,59 +11,59 @@
describe('unreachable Zotero service', function() {
- this.timeout(40000);
+ this.timeout(40000);
- // Give Zotero port which is it is not running from-
- // Mimics Zotero being down.
- before(function () { return server.start({zoteroPort:1971}); });
+ // Give Zotero port which is it is not running from-
+ // Mimics Zotero being down.
+ before(function () { return server.start({zoteroPort:1971}); });
- // PMID on NIH website that is not found in the id converter api
- // This will fail when Zotero is disabled because we no longer directly
scrape pubMed central URLs,
- // as they have blocked our UA in the past.
- it('PMID not in doi id converter api', function() {
- var pmid = '14656957';
- return server.query(pmid, 'mediawiki', 'en')
- .then(function(res) {
- assert.status(res, 404);
- }, function(err) {
- assert.checkError(err, 404, 'Unable to locate resource
with pmid ' + pmid,
- 'Unexpected error message ' + err.body.Error);
- });
- });
+ // PMID on NIH website that is not found in the id converter api
+ // This will fail when Zotero is disabled because we no longer directly
scrape pubMed central URLs,
+ // as they have blocked our UA in the past.
+ it('PMID not in doi id converter api', function() {
+ var pmid = '14656957';
+ return server.query(pmid, 'mediawiki', 'en')
+ .then(function(res) {
+ assert.status(res, 404);
+ }, function(err) {
+ assert.checkError(err, 404, 'Unable to locate resource with pmid '
+ pmid,
+ 'Unexpected error message ' + err.body.Error);
+ });
+ });
- // PMID on NIH website that is found in the id converter api- should
convert to DOI
- it('PMCID present in doi id converter api', function() {
- return server.query('PMC3605911').then(function(res) {
- assert.status(res, 200);
- assert.checkCitation(res, 'Viral Phylodynamics');
- assert.deepEqual(!!res.body[0].PMCID, true, 'Missing
PMCID');
- assert.deepEqual(!!res.body[0].DOI, true, 'Missing
DOI');
- assert.deepEqual(!!res.body[0].ISSN, false, 'Should not
contain ISSN'); // This indicates Zotero is actually activated since ISSN is
not in crossRef, where we're obtaining the metadata
- assert.deepEqual(res.body[0].itemType,
'journalArticle', 'Wrong itemType; expected journalArticle, got' +
res.body[0].itemType);
- });
- });
+ // PMID on NIH website that is found in the id converter api- should
convert to DOI
+ it('PMCID present in doi id converter api', function() {
+ return server.query('PMC3605911').then(function(res) {
+ assert.status(res, 200);
+ assert.checkCitation(res, 'Viral Phylodynamics');
+ assert.deepEqual(!!res.body[0].PMCID, true, 'Missing PMCID');
+ assert.deepEqual(!!res.body[0].DOI, true, 'Missing DOI');
+ assert.deepEqual(!!res.body[0].ISSN, false, 'Should not contain
ISSN'); // This indicates Zotero is actually activated since ISSN is not in
crossRef, where we're obtaining the metadata
+ assert.deepEqual(res.body[0].itemType, 'journalArticle', 'Wrong
itemType; expected journalArticle, got' + res.body[0].itemType);
+ });
+ });
- // JSTOR page with tabs in natively scraped title
- it('JSTOR page with tabs in natively scraped title', function() {
- return
server.query('http://www.jstor.org/discover/10.2307/3677029').then(function(res)
{
- assert.status(res, 200);
- assert.checkCitation(res, 'Flight Feather Moult in the
Red-Necked Nightjar Caprimulgus ruficollis');
- assert.deepEqual(!!res.body[0].DOI, true, 'Missing
DOI');
- assert.deepEqual(!!res.body[0].ISSN, false, 'Should not
contain ISSN'); // This indicates Zotero is actually activated since ISSN is
not in crossRef, where we're obtaining the metadata
- assert.deepEqual(res.body[0].itemType,
'journalArticle', 'Wrong itemType; expected journalArticle, got' +
res.body[0].itemType);
- });
- });
+ // JSTOR page with tabs in natively scraped title
+ it('JSTOR page with tabs in natively scraped title', function() {
+ return
server.query('http://www.jstor.org/discover/10.2307/3677029').then(function(res)
{
+ assert.status(res, 200);
+ assert.checkCitation(res, 'Flight Feather Moult in the Red-Necked
Nightjar Caprimulgus ruficollis');
+ assert.deepEqual(!!res.body[0].DOI, true, 'Missing DOI');
+ assert.deepEqual(!!res.body[0].ISSN, false, 'Should not contain
ISSN'); // This indicates Zotero is actually activated since ISSN is not in
crossRef, where we're obtaining the metadata
+ assert.deepEqual(res.body[0].itemType, 'journalArticle', 'Wrong
itemType; expected journalArticle, got' + res.body[0].itemType);
+ });
+ });
- // Article with publisher field filled in with dublinCore metadata
(general has it too as fallback)
- it('Article with doi within DublinCore metadata', function() {
- return
server.query('http://www.sciencemag.org/content/303/5656/387.short').then(function(res)
{
- assert.status(res, 200);
- assert.checkCitation(res, 'Multiple Ebola Virus
Transmission Events and Rapid Decline of Central African Wildlife');
- assert.deepEqual(res.body[0].date, '2004-01-16');
- assert.deepEqual(res.body[0].DOI,
'10.1126/science.1092528');
- assert.deepEqual(res.body[0].itemType,
'journalArticle', 'Wrong itemType; expected journalArticle, got' +
res.body[0].itemType);
- });
- });
+ // Article with publisher field filled in with dublinCore metadata
(general has it too as fallback)
+ it('Article with doi within DublinCore metadata', function() {
+ return
server.query('http://www.sciencemag.org/content/303/5656/387.short').then(function(res)
{
+ assert.status(res, 200);
+ assert.checkCitation(res, 'Multiple Ebola Virus Transmission
Events and Rapid Decline of Central African Wildlife');
+ assert.deepEqual(res.body[0].date, '2004-01-16');
+ assert.deepEqual(res.body[0].DOI, '10.1126/science.1092528');
+ assert.deepEqual(res.body[0].itemType, 'journalArticle', 'Wrong
itemType; expected journalArticle, got' + res.body[0].itemType);
+ });
+ });
it('doi spage and epage fields in crossRef coins data', function() {
return
server.query('http://dx.doi.org/10.1002/jlac.18571010113').then(function(res) {
@@ -76,4 +76,15 @@
});
});
+ it('Get error for bibtex export', function() {
+ return server.query('http://www.example.com', 'bibtex', 'en')
+ .then(function(res) {
+ assert.status(res, 404);
+ }, function(err) {
+ assert.deepEqual(JSON.parse(err.body.toString()).Error,'Unable to
serve bibtex format at this time');
+ assert.status(err, 404);
+ //assert.checkError(err, 404, 'Unable to serve bibtex format at
this time');
+ });
+ });
+
});
\ No newline at end of file
diff --git a/test/utils/mockZoteroServer.js b/test/utils/mockZoteroServer.js
new file mode 100644
index 0000000..10b69fb
--- /dev/null
+++ b/test/utils/mockZoteroServer.js
@@ -0,0 +1,18 @@
+var express = require('express');
+var app = express();
+
+app.post('/export', function(req, res){
+ res.status(500);
+ res.send('Internal server error');
+});
+
+app.post('/web', function(req, res){
+ res.status(200);
+
res.send('[{"url":"http://example.com","itemType":"webpage","title":"Example
Domain","accessDate":"2015-10-16","websiteTitle":"example.com"}]');
+});
+
+module.exports = app;
+
+module.exports.start = function(port){
+ app.listen(port);
+};
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/246163
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I09e40682521f4d99775c4669238117b813985521
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/services/citoid
Gerrit-Branch: master
Gerrit-Owner: Mvolz <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: Mvolz <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
