From f512da7b2fdcde4062509b3e002ffed8922ca794 Mon Sep 17 00:00:00 2001 From: gsoos Date: Sun, 12 Nov 2017 17:57:13 +0100 Subject: [PATCH] fix(request): This reverts commit 2d5496dfc40c23829c16ddc587946dff0071060b, promise based implementation doesnt return header only body. --- lib/request.spec.js | 121 +++++++++++++++++++++++--------------------- lib/wrapper.js | 39 +++++++------- lib/wrapper.spec.js | 18 ++++--- package-lock.json | 17 ++----- package.json | 3 +- 5 files changed, 103 insertions(+), 95 deletions(-) diff --git a/lib/request.spec.js b/lib/request.spec.js index ac0f2e3..f00c434 100644 --- a/lib/request.spec.js +++ b/lib/request.spec.js @@ -1,7 +1,7 @@ 'use strict'; const SuiteRequest = require('./request'); -const request = require('request-promise-native'); +const request = require('request'); const Escher = require('escher-auth'); describe('SuiteRequest', function() { @@ -30,114 +30,121 @@ describe('SuiteRequest', function() { it('should sign headers of GET request', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - this.sandbox.stub(request, 'get').resolves(createDummyResponse()); + this.sandbox.stub(request, 'get').callsFake(function(options, callback) { + expect(options.headers['x-ems-auth']) + .to.have.string('SignedHeaders=content-type;host;x-ems-date,'); + callback(null, createDummyResponse()); + }); suiteRequest.get('/path'); - - const requestArgument = request.get.getCall(0).args[0]; - expect(requestArgument.headers['x-ems-auth']).to.have.string('SignedHeaders=content-type;host;x-ems-date,'); }); it('should sign headers of POST request', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + expect(options.headers['x-ems-auth']) + .to.have.string('SignedHeaders=content-type;host;x-ems-date,'); + callback(null, createDummyResponse()); + }); suiteRequest.post('/path', { name: 'Almanach' }); - - const requestArgument = request.post.getCall(0).args[0]; - expect(requestArgument.headers['x-ems-auth']).to.have.string('SignedHeaders=content-type;host;x-ems-date,'); - }); - - it.only('should return response body', function*() { - const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); - - const result = yield suiteRequest.post('/path', { name: 'Almanach' }); - - expect(result.body).to.eql('response body dummy'); }); it('should sign headers of DELETE request', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - this.sandbox.stub(request, 'delete').resolves(createDummyResponse()); + this.sandbox.stub(request, 'delete').callsFake(function(options, callback) { + expect(options.headers['x-ems-auth']) + .to.have.string('SignedHeaders=content-type;host;x-ems-date,'); + callback(null, createDummyResponse()); + }); suiteRequest.delete('/path'); - - const requestArgument = request.delete.getCall(0).args[0]; - expect(requestArgument.headers['x-ems-auth']).to.have.string('SignedHeaders=content-type;host;x-ems-date,'); }); it('should sign headers with non string values', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); requestOptions.setHeader(['x-customer-id', 15]); - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + expect(options.headers['x-ems-auth']) + .to.have.string('content-type;host;x-customer-id;x-ems-date,'); + callback(null, createDummyResponse()); + }); suiteRequest.post('/path', { name: 'Almanach' }); - - const requestArgument = request.post.getCall(0).args[0]; - expect(requestArgument.headers['x-ems-auth']).to.have.string('content-type;host;x-customer-id;x-ems-date,'); }); - it('should encode payload when content type is json', function*() { + it('should encode payload when content type is json', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); - - yield suiteRequest.post('/path', { name: 'Almanach' }); - - const requestArgument = request.post.getCall(0).args[0]; - expect(requestArgument.body).to.eql('{"name":"Almanach"}'); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + try { + expect(options.body).to.eql('{"name":"Almanach"}'); + callback(null, createDummyResponse()); + } catch (e) { + callback(e, createDummyResponse()); + } + }); + + return suiteRequest.post('/path', { name: 'Almanach' }); }); - it('should encode payload when content type is utf8 json', function*() { + it('should encode payload when content type is utf8 json', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); requestOptions.setHeader(['content-type', 'application/json;charset=utf-8']); - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + try { + expect(options.body).to.eql('{"name":"Almanach"}'); + callback(null, createDummyResponse()); + } catch (e) { + callback(e, createDummyResponse()); + } + }); - yield suiteRequest.post('/path', { name: 'Almanach' }); - - const requestArgument = request.post.getCall(0).args[0]; - expect(requestArgument.body).to.eql('{"name":"Almanach"}'); + return suiteRequest.post('/path', { name: 'Almanach' }); }); - it('should skip encoding of payload when content type is not json', function*() { + it('should skip encoding of payload when content type is not json', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); requestOptions.setHeader(['content-type', 'text/csv']); - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); - - yield suiteRequest.post('/path', 'header1;header2'); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + try { + expect(options.body).to.eql('header1;header2'); + callback(null, createDummyResponse()); + } catch (e) { + callback(e, createDummyResponse()); + } + }); - const requestArgument = request.post.getCall(0).args[0]; - expect(requestArgument.body).to.eql('header1;header2'); + return suiteRequest.post('/path', 'header1;header2'); }); - it('signs extra headers too', function*() { + it('signs extra headers too', function() { requestOptions.setHeader(['extra-header', 'header-value']); const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); - this.sandbox.stub(request, 'get').resolves(createDummyResponse()); + this.sandbox.stub(request, 'get').callsFake(function(options, callback) { + expect(options.headers['x-ems-auth']) + .to.have.string('SignedHeaders=content-type;extra-header;host;x-ems-date,'); + callback(null, createDummyResponse()); + }); - yield suiteRequest.get('/path'); - - const requestArgument = request.get.getCall(0).args[0]; - expect(requestArgument.headers['x-ems-auth']) - .to.have.string('SignedHeaders=content-type;extra-header;host;x-ems-date,'); + suiteRequest.get('/path'); }); - it('should sign the payload of POST request', function*() { + it('should sign the payload of POST request', function() { const suiteRequest = SuiteRequest.create('key-id', 'secret', requestOptions); const payload = { name: 'Test' }; this.sandbox.spy(Escher.prototype, 'signRequest'); - this.sandbox.stub(request, 'post').resolves(createDummyResponse()); + this.sandbox.stub(request, 'post').callsFake(function(options, callback) { + callback(null, createDummyResponse()); + }); - yield suiteRequest.post('/path', payload); + suiteRequest.post('/path', payload); expect(Escher.prototype.signRequest.callCount).to.eql(1); const firstCall = Escher.prototype.signRequest.getCall(0); diff --git a/lib/wrapper.js b/lib/wrapper.js index f9224a6..d9f56c8 100644 --- a/lib/wrapper.js +++ b/lib/wrapper.js @@ -3,7 +3,7 @@ const SuiteRequestError = require('./requestError'); const logger = require('logentries-logformat')('suiterequest'); const debugLogger = require('logentries-logformat')('suiterequest-debug'); -const request = require('request-promise-native'); +const request = require('request'); class RequestWrapper { @@ -17,24 +17,29 @@ class RequestWrapper { } send() { + return new Promise((resolve, reject) => { + this._sendRequest(resolve, reject); + }); + } + + _sendRequest(resolve, reject) { const startTime = this._getTime(); const method = this.requestOptions.method.toLowerCase(); const reqOptions = this._getRequestOptions(); - return request[method](reqOptions).then( - (response) => { - this._handleResponse(response); + request[method](reqOptions, (err, response) => { + try { + this._handleResponse(err, response); + } catch (e) { + return reject(e); + } - const endTime = this._getTime(); - logger.success('send', this._getLogParameters({ time: startTime - endTime })); + const endTime = this._getTime(); + logger.success('send', this._getLogParameters({ time: startTime - endTime })); - return response; - }, - (error) => { - this._handleError(error); - } - ); + return resolve(response); + }); } _isJsonResponse(response) { @@ -78,12 +83,12 @@ class RequestWrapper { return reqOptions; } - _handleError(error) { - logger.error('fatal_error', error.message, this._getLogParameters()); - throw new SuiteRequestError(error.message, 500); - } + _handleResponse(err, response) { + if (err) { + logger.error('fatal_error', err.message, this._getLogParameters()); + throw new SuiteRequestError(err.message, 500); + } - _handleResponse(response) { if (response.statusCode >= 400) { logger.error('server_error', response.body.replyText, this._getLogParameters({ code: response.statusCode diff --git a/lib/wrapper.spec.js b/lib/wrapper.spec.js index f16944d..a3c7a32 100644 --- a/lib/wrapper.spec.js +++ b/lib/wrapper.spec.js @@ -1,6 +1,6 @@ 'use strict'; -const request = require('request-promise-native'); +const request = require('request'); const Wrapper = require('./wrapper'); const SuiteRequestError = require('./requestError'); @@ -47,7 +47,9 @@ describe('Wrapper', function() { let requestGetStub; beforeEach(function() { - requestGetStub = this.sandbox.stub(request, 'get').resolves(apiResponse); + requestGetStub = this.sandbox.stub(request, 'get').callsFake(function(options, callback) { + callback(null, apiResponse); + }); wrapper = new Wrapper(escherRequestOptions, 'http:'); }); @@ -164,14 +166,16 @@ describe('Wrapper', function() { describe('timeout', function() { it('should send GET request with given timeout in options', function*() { - escherRequestOptions = Object.assign({}, escherRequestOptions, { timeout: 60000 }); - - this.sandbox.stub(request, 'get').resolves(apiResponse); + let actualRequestOption; + this.sandbox.stub(request, 'get').callsFake(function(options, callback) { + actualRequestOption = options; + callback(null, apiResponse); + }); + escherRequestOptions.timeout = 60000; yield (new Wrapper(escherRequestOptions, 'http:')).send(); - const requestArgument = request.get.getCall(0).args[0]; - expect(requestArgument.timeout).to.be.eql(60000); + expect(actualRequestOption.timeout).to.be.eql(60000); }); }); }); diff --git a/package-lock.json b/package-lock.json index 139f777..9df4173 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2217,7 +2217,8 @@ "lodash": { "version": "4.14.0", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.14.0.tgz", - "integrity": "sha1-t0LWqAte6H58GjXBQ8Wxm9lmwQs=" + "integrity": "sha1-t0LWqAte6H58GjXBQ8Wxm9lmwQs=", + "dev": true }, "lodash._baseassign": { "version": "3.2.0", @@ -3041,20 +3042,11 @@ "version": "1.1.1", "resolved": "https://registry.npmjs.org/request-promise-core/-/request-promise-core-1.1.1.tgz", "integrity": "sha1-Pu4AssWqgyOc+wTFcA2jb4HNCLY=", + "dev": true, "requires": { "lodash": "4.14.0" } }, - "request-promise-native": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/request-promise-native/-/request-promise-native-1.0.5.tgz", - "integrity": "sha1-UoF3D2jgyXGeUWP9P6tIIhX0/aU=", - "requires": { - "request-promise-core": "1.1.1", - "stealthy-require": "1.1.1", - "tough-cookie": "2.3.3" - } - }, "require-relative": { "version": "0.8.7", "resolved": "https://registry.npmjs.org/require-relative/-/require-relative-0.8.7.tgz", @@ -3384,7 +3376,8 @@ "stealthy-require": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/stealthy-require/-/stealthy-require-1.1.1.tgz", - "integrity": "sha1-NbCYdbT/SfJqd35QmzCQoyJr8ks=" + "integrity": "sha1-NbCYdbT/SfJqd35QmzCQoyJr8ks=", + "dev": true }, "stream-combiner": { "version": "0.0.4", diff --git a/package.json b/package.json index 2592e9a..b817f3d 100644 --- a/package.json +++ b/package.json @@ -28,8 +28,7 @@ "dependencies": { "escher-auth": "1.0.0", "logentries-logformat": "0.2.0", - "request": "2.83.0", - "request-promise-native": "^1.0.5" + "request": "2.83.0" }, "devDependencies": { "chai": "4.1.2",