Skip to content

Commit

Permalink
fix(request): This reverts commit 2d5496d, promise based implementati…
Browse files Browse the repository at this point in the history 
…on doesnt return header only body.
  • Loading branch information
@sonicoder86
sonicoder86 committed Nov 12, 2017
1 parent 2d5496d commit f512da7
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 95 deletions.
121 changes: 64 additions & 57 deletions lib/request.spec.js
View file
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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);
Expand Down
39 changes: 22 additions & 17 deletions lib/wrapper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 11 additions & 7 deletions lib/wrapper.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const request = require('request-promise-native');
const request = require('request');
const Wrapper = require('./wrapper');
const SuiteRequestError = require('./requestError');

Expand Down Expand Up @@ -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:');
});

Expand Down Expand Up @@ -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);
});
});
});
17 changes: 5 additions & 12 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit f512da7

Please sign in to comment.