Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(adapter-fetch): Add support for handling binary data #332

Merged
merged 4 commits into from
May 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ node_js:
- '10'
- '12'

dist: trusty
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dist is EOL and is no longer supported by Chrome. That was the reason for the failing builds.


addons:
chrome: stable

Expand Down
3 changes: 2 additions & 1 deletion packages/@pollyjs/adapter-fetch/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
"dependencies": {
"@pollyjs/adapter": "^4.2.1",
"@pollyjs/utils": "^4.1.0",
"detect-node": "^2.0.4"
"detect-node": "^2.0.4",
"to-arraybuffer": "^1.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to-arraybuffer uses require('buffer').Buffer so I'm confused how this all is built in our current build pipeline - is it some how "browserifying" the node implementation and also bundling the npm "buffer" package?

Copy link
Collaborator Author

@offirgolan offirgolan May 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so looked more into it to see what was actually being built. We current have rollup-plugin-node-builtins setup which has support for using Buffer via buffer-es6.

If I imported the buffer via import { Buffer } from 'buffer' it only inlines the polyfill for the UMD build but when I used from 'buffer/', it inlined it for all 3 builds (CJS, ES, and UMD). Since this is only meant for the browser, it would make sense for it to be be polyfilled in all 3.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explains a lot, was very confused - thanks for looking into it

},
"devDependencies": {
"@pollyjs/core": "^4.2.1",
Expand Down
24 changes: 18 additions & 6 deletions packages/@pollyjs/adapter-fetch/src/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import Adapter from '@pollyjs/adapter';
import isNode from 'detect-node';
import { Buffer } from 'buffer/';
import bufferToArrayBuffer from 'to-arraybuffer';

import serializeHeaders from './utils/serializer-headers';
import isBufferUtf8Representable from './utils/is-buffer-utf8-representable';

const { defineProperty } = Object;
const IS_STUBBED = Symbol();
Expand Down Expand Up @@ -164,10 +167,14 @@ export default class FetchAdapter extends Adapter {
}
]);

const buffer = Buffer.from(await response.arrayBuffer());
const isBinaryBuffer = !isBufferUtf8Representable(buffer);

return {
statusCode: response.status,
headers: serializeHeaders(response.headers),
body: await response.text()
body: buffer.toString(isBinaryBuffer ? 'hex' : 'utf8'),
isBinary: isBinaryBuffer
};
}

Expand Down Expand Up @@ -199,11 +206,16 @@ export default class FetchAdapter extends Adapter {
}

const { absoluteUrl, response: pollyResponse } = pollyRequest;
const { statusCode } = pollyResponse;
const responseBody =
statusCode === 204 && pollyResponse.body === ''
? null
: pollyResponse.body;
const { statusCode, body, isBinary } = pollyResponse;

let responseBody = body;

if (statusCode === 204 && responseBody === '') {
responseBody = null;
} else if (isBinary) {
responseBody = bufferToArrayBuffer(Buffer.from(body, 'hex'));
}

const response = new Response(responseBody, {
status: statusCode,
headers: pollyResponse.headers
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Buffer } from 'buffer/';

/**
* Determine if the given buffer is utf8.
* @param {Buffer} buffer
*/
export default function isBufferUtf8Representable(buffer) {
const utfEncodedBuffer = buffer.toString('utf8');
const reconstructedBuffer = Buffer.from(utfEncodedBuffer, 'utf8');

return reconstructedBuffer.equals(buffer);
}
32 changes: 32 additions & 0 deletions packages/@pollyjs/adapter-fetch/tests/integration/adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import adapterTests from '@pollyjs-tests/integration/adapter-tests';
import adapterPollyTests from '@pollyjs-tests/integration/adapter-polly-tests';
import adapterBrowserTests from '@pollyjs-tests/integration/adapter-browser-tests';
import adapterIdentifierTests from '@pollyjs-tests/integration/adapter-identifier-tests';
import { Buffer } from 'buffer/';

import FetchAdapter from '../../src';
import pollyConfig from '../utils/polly-config';
Expand Down Expand Up @@ -103,6 +104,37 @@ describe('Integration | Fetch Adapter', function() {
expect(error.message).to.contain('The user aborted a request.');
});

it('should be able to download binary content', async function() {
this.timeout(10000);

const fetch = async () =>
Buffer.from(
await this.fetch('/assets/32x32.png').then(res => res.arrayBuffer())
);

this.polly.disconnectFrom(FetchAdapter);

const nativeResponseBuffer = await fetch();

this.polly.connectTo(FetchAdapter);

const recordedResponseBuffer = await fetch();

const { recordingName, config } = this.polly;

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

const replayedResponseBuffer = await fetch();

expect(nativeResponseBuffer.equals(recordedResponseBuffer)).to.equal(true);
expect(recordedResponseBuffer.equals(replayedResponseBuffer)).to.equal(
true
);
expect(nativeResponseBuffer.equals(replayedResponseBuffer)).to.equal(true);
});

describe('Request', function() {
it('should support Request objects', async function() {
const { server } = this.polly;
Expand Down
Binary file added tests/assets/32x32.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion tests/integration/persister-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ export default function persisterTests() {
// Binary content
server.get(this.recordUrl()).once('beforeResponse', (req, res) => {
res.isBinary = true;
res.body = 'Some binary content';
res.body = '536f6d6520636f6e74656e74';
});

await this.fetchRecord();
Expand Down
4 changes: 4 additions & 0 deletions tests/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ module.exports = function attachMiddleware(app) {
recordingsDir: path.join(__dirname, 'recordings')
});

app.get('/assets/:name', (req, res) => {
res.sendFile(path.join(__dirname, 'assets', req.params.name));
});

app.use(bodyParser.json());

app.get('/echo', (req, res) => {
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13542,7 +13542,7 @@ to-array@0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/to-array/-/to-array-0.1.4.tgz#17e6c11f73dd4f3d74cda7a4ff3238e9ad9bf890"

to-arraybuffer@^1.0.0:
to-arraybuffer@^1.0.0, to-arraybuffer@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/to-arraybuffer/-/to-arraybuffer-1.0.1.tgz#7d229b1fcc637e466ca081180836a7aabff83f43"

Expand Down