Use request lib in xmlrpc

refs TryGhost#9178, refs TryGhost#8980
kirrg001 · Dec 14, 2017 · 897fe4a · 897fe4a
1 parent a3091a3
commit 897fe4a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 43 deletions.
diff --git a/core/server/services/xmlrpc.js b/core/server/services/xmlrpc.js
@@ -1,9 +1,9 @@
 var _ = require('lodash'),
-    http = require('http'),
     xml = require('xml'),
     config = require('../config'),
     urlService = require('../services/url'),
     common = require('../lib/common'),
+    request = require('../lib/request'),
     settingsCache = require('./settings/cache'),
 
     defaultPostSlugs = [
@@ -16,13 +16,14 @@ var _ = require('lodash'),
         'themes'
     ],
     // ToDo: Make this configurable
-    pingList = [{
-        host: 'blogsearch.google.com',
-        path: '/ping/RPC2'
-    }, {
-        host: 'rpc.pingomatic.com',
-        path: '/'
-    }];
+    pingList = [
+        {
+            url: 'blogsearch.google.com/ping/RPC2'
+        },
+        {
+            url: 'rpc.pingomatic.com'
+        }
+    ];
 
 function ping(post) {
     var pingXML,
@@ -65,25 +66,19 @@ function ping(post) {
     // Ping each of the defined services.
     _.each(pingList, function (pingHost) {
         var options = {
-                hostname: pingHost.host,
-                path: pingHost.path,
-                method: 'POST'
-            },
-            req;
+            body: pingXML,
+            timeout: 2 * 1000
+        };
 
-        req = http.request(options);
-        req.write(pingXML);
-
-        req.on('error', function handleError(err) {
-            common.logging.error(new common.errors.GhostError({
-                err: err,
-                message: err.message,
-                context: common.i18n.t('errors.services.ping.requestFailed.error', {service: 'slack'}),
-                help: common.i18n.t('errors.services.ping.requestFailed.help', {url: 'http://docs.ghost.org'})
-            }));
-        });
-
-        req.end();
+        request(pingHost.url, options)
+            .catch(function (err) {
+                common.logging.error(new common.errors.GhostError({
+                    err: err,
+                    message: err.message,
+                    context: common.i18n.t('errors.services.ping.requestFailed.error', {service: 'slack'}),
+                    help: common.i18n.t('errors.services.ping.requestFailed.help', {url: 'http://docs.ghost.org'})
+                }));
+            });
     });
 }
 

diff --git a/core/test/unit/services/xmlrpc_spec.js b/core/test/unit/services/xmlrpc_spec.js
@@ -73,15 +73,20 @@ describe('XMLRPC', function () {
     describe('ping()', function () {
         var ping = xmlrpc.__get__('ping');
 
-        it('with a post should execute two pings', function () {
+        it('with a post should execute two pings', function (done) {
             var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
                 ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
                 testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
 
             ping(testPost);
 
-            ping1.isDone().should.be.true();
-            ping2.isDone().should.be.true();
+            (function retry() {
+                if (ping1.isDone() && ping2.isDone()) {
+                    return done();
+                }
+
+                setTimeout(retry, 100);
+            }());
         });
 
         it('with default post should not execute pings', function () {
@@ -121,25 +126,23 @@ describe('XMLRPC', function () {
             ping2.isDone().should.be.false();
         });
 
-        it('captures && logs errors from requests', function () {
+        it('captures && logs errors from requests', function (done) {
             var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]),
-                httpMock = sandbox.stub(http, 'request').returns({
-                    write: function () {
-                    },
-                    end: function () {
-                    },
-                    on: function (eventName, eventDone) {
-                        eventDone(new Error('ping site is down'));
-                    }
-                }),
+                ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(500),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').reply(400),
                 loggingStub = sandbox.stub(common.logging, 'error');
 
             ping(testPost);
 
-            should.exist(httpMock);
-            // pinglist contains 2 endpoints, both return ping site is down
-            loggingStub.calledTwice.should.eql(true);
-            loggingStub.args[0][0].message.should.eql('ping site is down');
+            (function retry() {
+                if (ping1.isDone() && ping2.isDone()) {
+                    loggingStub.calledTwice.should.eql(true);
+                    loggingStub.args[0][0].message.should.containEql('Response code 500');
+                    return done();
+                }
+
+                setTimeout(retry, 100);
+            }());
         });
     });
 });