Skip to content

Commit

Permalink
fix(adapter-node-http): Improve binary response body handling (#329)
Browse files Browse the repository at this point in the history
  • Loading branch information
offirgolan authored Apr 30, 2020
1 parent 63c23ca commit 9466989
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 17 deletions.
24 changes: 15 additions & 9 deletions packages/@pollyjs/adapter-node-http/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,14 @@ export default class HttpAdapter extends Adapter {
return {
headers: response.headers,
statusCode: response.statusCode,
body: responseBody
body: responseBody.body,
isBinary: responseBody.isBinary
};
}

async respondToRequest(pollyRequest, error) {
const { req, respond } = pollyRequest.requestArguments;
const { statusCode, body, headers } = pollyRequest.response;
const { statusCode, body, headers, isBinary } = pollyRequest.response;

if (pollyRequest[ABORT_HANDLER]) {
req.off('abort', pollyRequest[ABORT_HANDLER]);
Expand All @@ -229,7 +230,7 @@ export default class HttpAdapter extends Adapter {
return;
}

const chunks = this.getChunksFromBody(body, headers);
const chunks = this.getChunksFromBody(body, headers, isBinary);
const stream = new ReadableStream();

// Expose the response data as a stream of chunks since
Expand Down Expand Up @@ -274,18 +275,25 @@ export default class HttpAdapter extends Adapter {
return chunk.toString('hex');
});

return JSON.stringify(hexChunks);
return {
isBinary: true,
body: JSON.stringify(hexChunks)
};
}

const buffer = mergeChunks(chunks);
const isBinaryBuffer = !isUtf8Representable(buffer);

// The merged buffer can be one of two things:
// 1. A binary buffer which then has to be recorded as a hex string.
// 2. A string buffer.
return buffer.toString(isUtf8Representable(buffer) ? 'utf8' : 'hex');
return {
isBinary: isBinaryBuffer,
body: buffer.toString(isBinaryBuffer ? 'hex' : 'utf8')
};
}

getChunksFromBody(body, headers) {
getChunksFromBody(body, headers, isBinary = false) {
if (!body) {
return [];
}
Expand All @@ -302,11 +310,9 @@ export default class HttpAdapter extends Adapter {
return hexChunks.map(chunk => Buffer.from(chunk, 'hex'));
}

const buffer = Buffer.from(body);

// The body can be one of two things:
// 1. A hex string which then means its binary data.
// 2. A utf8 string which means a regular string.
return [Buffer.from(buffer, isUtf8Representable(buffer) ? 'utf8' : 'hex')];
return [Buffer.from(body, isBinary ? 'hex' : 'utf8')];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,26 +125,38 @@ function commonTests(transport) {

it('should be able to download binary content', async function() {
const url = `${protocol}//via.placeholder.com/150/92c952`;
const { server } = this.polly;

server.get(url).passthrough(true);
this.polly.disconnectFrom(NodeHTTPAdapter);

const nativeResponseStream = await getResponseFromRequest(
transport.request(url)
);

server.get(url).passthrough(false);
this.polly.connectTo(NodeHTTPAdapter);

const recordedResponseStream = await getResponseFromRequest(
transport.request(url)
);

const [nativeHash, recordedHash] = await Promise.all([
const { recordingName, config } = this.polly;

await this.polly.stop();
this.polly = new Polly(recordingName, config);
this.polly.replay();

const replayedResponseStream = await getResponseFromRequest(
transport.request(url)
);

const [nativeHash, recordedHash, replayedHash] = await Promise.all([
calculateHashFromStream(nativeResponseStream),
calculateHashFromStream(recordedResponseStream)
calculateHashFromStream(recordedResponseStream),
calculateHashFromStream(replayedResponseStream)
]);

expect(nativeHash).to.equal(recordedHash);
expect(recordedHash).to.equal(replayedHash);
expect(nativeHash).to.equal(replayedHash);
});

it('should handle aborting a request', async function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ export default function normalizeRecordedResponse(response) {
statusText,
statusCode: status,
headers: normalizeHeaders(headers),
body: content && content.text
body: content && content.text,
isBinary: Boolean(content && content._isBinary)
};
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@pollyjs/core/src/-private/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export default class PollyRequest extends HTTPBase {
}

async respond(response) {
const { statusCode, headers, body } = response || {};
const { statusCode, headers, body, isBinary = false } = response || {};

assert(
'Cannot respond to a request that already has a response.',
Expand All @@ -195,6 +195,8 @@ export default class PollyRequest extends HTTPBase {
// Set the body without modifying any headers (instead of using .send())
this.response.body = body;

this.response.isBinary = isBinary;

// Trigger the `beforeResponse` event
await this._emit('beforeResponse', this.response);

Expand Down
3 changes: 2 additions & 1 deletion packages/@pollyjs/core/src/-private/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import HTTPBase from './http-base';
const DEFAULT_STATUS_CODE = 200;

export default class PollyResponse extends HTTPBase {
constructor(statusCode, headers, body) {
constructor(statusCode, headers, body, isBinary = false) {
super();
this.status(statusCode || DEFAULT_STATUS_CODE);
this.setHeaders(headers);
this.body = body;
this.isBinary = isBinary;
}

get ok() {
Expand Down
4 changes: 4 additions & 0 deletions packages/@pollyjs/core/tests/unit/-private/response-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ describe('Unit | Response', function() {
expect(new PollyResponse().statusCode).to.equal(200);
});

it('should default isBinary to false', function() {
expect(new PollyResponse().isBinary).to.be.false;
});

describe('API', function() {
beforeEach(function() {
response = new PollyResponse();
Expand Down
4 changes: 4 additions & 0 deletions packages/@pollyjs/persister/src/har/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export default class Response {

if (response.body && typeof response.body === 'string') {
this.content.text = response.body;

if (response.isBinary) {
this.content._isBinary = true;
}
}

const contentLength = getFirstHeader(response, 'Content-Length');
Expand Down
51 changes: 51 additions & 0 deletions tests/integration/persister-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,4 +346,55 @@ export default function persisterTests() {
expect(har.log.entries[2].request.url).to.include(orderedRecordUrl(1));
expect(har.log.entries[3].request.url).to.include(orderedRecordUrl(2));
});

it('should correctly handle binary responses', async function() {
const { recordingId, server, persister } = this.polly;
let har, content;

this.polly.record();

// Non binary content
server.get(this.recordUrl()).once('beforeResponse', (req, res) => {
res.body = 'Some content';
});

await this.fetchRecord();
await persister.persist();

har = await persister.find(recordingId);
content = har.log.entries[0].response.content;

expect(await validate.har(har)).to.be.true;
expect(content._isBinary).to.be.undefined;

// Binary content
server.get(this.recordUrl()).once('beforeResponse', (req, res) => {
res.isBinary = true;
res.body = 'Some binary content';
});

await this.fetchRecord();
await persister.persist();

har = await persister.find(recordingId);
content = har.log.entries[1].response.content;

expect(await validate.har(har)).to.be.true;
expect(content._isBinary).to.be.true;

// Binary content with no body
server.get(this.recordUrl()).once('beforeResponse', (req, res) => {
res.isBinary = true;
res.body = '';
});

await this.fetchRecord();
await persister.persist();

har = await persister.find(recordingId);
content = har.log.entries[2].response.content;

expect(await validate.har(har)).to.be.true;
expect(content._isBinary).to.be.undefined;
});
}

0 comments on commit 9466989

Please sign in to comment.