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

fix(adapter-fetch): Correctly handle Request instance passed into fetch #259

Merged
merged 3 commits into from
Sep 30, 2019
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
74 changes: 65 additions & 9 deletions packages/@pollyjs/adapter-fetch/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import serializeHeaders from './utils/serializer-headers';

const { defineProperty } = Object;
const IS_STUBBED = Symbol();
const REQUEST_ARGUMENTS = Symbol();

export default class FetchAdapter extends Adapter {
static get name() {
Expand All @@ -26,23 +27,72 @@ export default class FetchAdapter extends Adapter {
);
}

['fetch', 'Response', 'Headers'].forEach(key =>
['fetch', 'Request', 'Response', 'Headers'].forEach(key =>
this.assert(`${key} global not found.`, !!(context && context[key]))
);
this.assert(
'Running concurrent fetch adapters is unsupported, stop any running Polly instances.',
!context.fetch[IS_STUBBED]
!context.fetch[IS_STUBBED] && !context.Request[IS_STUBBED]
);

this.native = context.fetch;
this.nativeFetch = context.fetch;
this.NativeRequest = context.Request;

/*
Patch the Request class so we can store all the passed in options. This
allows us the access the `body` directly instead of having to do
`await req.blob()` as well as not having to hard code each option we want
to extract from the Request instance.
*/
class ExtendedRequest extends context.Request {
constructor(url, options) {
super(url, options);

let args;

options = options || {};

/*
The Request constructor can receive another Request instance as
the first argument so we use its arguments and merge it with the
new options.
*/
if (url instanceof ExtendedRequest) {
const reqArgs = url[REQUEST_ARGUMENTS];

args = { ...reqArgs, options: { ...reqArgs.options, ...options } };
} else {
args = { url, options };
}

defineProperty(this, REQUEST_ARGUMENTS, { value: args });
}

clone() {
return new ExtendedRequest(this);
}
}

context.Request = ExtendedRequest;
defineProperty(context.Request, IS_STUBBED, { value: true });

context.fetch = (url, options = {}) => {
let respond;

// Support Request object
if (typeof url === 'object' && 'url' in url) {
url = url.url;
if (url instanceof ExtendedRequest) {
const req = url;
const reqArgs = req[REQUEST_ARGUMENTS];

url = reqArgs.url;
options = { ...reqArgs.options, ...options };

// If a body exists in the Request instance, mimic reading the body
if ('body' in reqArgs.options) {
defineProperty(req, 'bodyUsed', { value: true });
}
}

let respond;
const promise = new Promise((resolve, reject) => {
respond = ({ response, error }) => {
if (error) {
Expand All @@ -68,14 +118,20 @@ export default class FetchAdapter extends Adapter {
}

onDisconnect() {
this.options.context.fetch = this.native;
this.native = null;
const { context } = this.options;

context.fetch = this.nativeFetch;
context.Request = this.NativeRequest;

this.nativeFetch = null;
this.NativeRequest = null;
}

async passthroughRequest(pollyRequest) {
const { context } = this.options;
const { options } = pollyRequest.requestArguments;

const response = await this.native.apply(global, [
const response = await this.nativeFetch.apply(context, [
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch

pollyRequest.url,
{
...options,
Expand Down
153 changes: 133 additions & 20 deletions packages/@pollyjs/adapter-fetch/tests/integration/adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import adapterBrowserTests from '@pollyjs-tests/integration/adapter-browser-test
import FetchAdapter from '../../src';
import pollyConfig from '../utils/polly-config';

class MockRequest {}
class MockResponse {}
class MockHeaders {}

Expand All @@ -29,16 +30,6 @@ describe('Integration | Fetch Adapter', function() {
expect(res.status).to.equal(200);
});

it('should support Request objects', async function() {
const { server } = this.polly;

server.any(this.recordUrl()).intercept((_, res) => res.sendStatus(200));

const res = await this.fetch(new Request(this.recordUrl()));

expect(res.status).to.equal(200);
});

it('should support array of key/value pair headers', async function() {
const { server } = this.polly;
let headers;
Expand All @@ -57,29 +48,129 @@ describe('Integration | Fetch Adapter', function() {
expect(res.status).to.equal(200);
expect(headers).to.deep.equal({ 'content-type': 'application/json' });
});

describe('Request', function() {
it('should support Request objects', async function() {
const { server } = this.polly;

server.any(this.recordUrl()).intercept((_, res) => res.sendStatus(200));

const res = await this.fetch(new Request(this.recordUrl()));

expect(res.status).to.equal(200);
});

it('should set bodyUsed to true if a body is present', async function() {
const { server } = this.polly;
const request = new Request('/', { method: 'POST', body: '{}' });

server.any().intercept((_, res) => res.sendStatus(200));

expect(request.bodyUsed).to.equal(false);
await this.fetch(request);
expect(request.bodyUsed).to.equal(true);
});

it('should not set bodyUsed to true if a body is not present', async function() {
const { server } = this.polly;
const request = new Request('/');

server.any().intercept((_, res) => res.sendStatus(200));

expect(request.bodyUsed).to.equal(false);
await this.fetch(request);
expect(request.bodyUsed).to.equal(false);
});

function testRequestOptions(createRequest, options) {
return async function() {
const { server } = this.polly;
let receivedOptions;

server.any().intercept((req, res) => {
receivedOptions = req.requestArguments.options;
res.sendStatus(200);
});

const res = await this.fetch(createRequest());

expect(res.status).to.equal(200);
expect(options).to.deep.equal(receivedOptions);
};
}

it(
'should handle no options',
testRequestOptions(() => new Request('/'), {})
);

it(
'should handle simple options',
testRequestOptions(
() =>
new Request('/', { method: 'POST', body: '{}', cache: 'no-cache' }),
{ method: 'POST', body: '{}', cache: 'no-cache' }
)
);

it(
'should handle a cloned request',
testRequestOptions(
() => new Request('/', { method: 'POST', body: '{}' }).clone(),
{ method: 'POST', body: '{}' }
)
);

it(
'should handle a request instance',
testRequestOptions(
() => new Request(new Request('/', { method: 'POST', body: '{}' })),
{ method: 'POST', body: '{}' }
)
);

it(
'should handle a request instance with overrides',
testRequestOptions(
() =>
new Request(new Request('/', { method: 'POST', body: '{}' }), {
method: 'PATCH',
headers: { foo: 'bar' }
}),
{ method: 'PATCH', headers: { foo: 'bar' }, body: '{}' }
)
);
});
});

describe('Integration | Fetch Adapter | Init', function() {
describe('Context', function() {
it(`should assign context's fetch as the native fetch`, async function() {
it(`should assign context's fetch as the native fetch and Request as the native Request`, async function() {
const polly = new Polly('context', { adapters: [] });
const fetch = () => {};
const adapterOptions = {
fetch: {
context: { fetch, Response: MockResponse, Headers: MockHeaders }
context: {
fetch,
Request: MockRequest,
Response: MockResponse,
Headers: MockHeaders
}
}
};

polly.configure({
adapters: [FetchAdapter],
adapterOptions
});
polly.configure({ adapters: [FetchAdapter], adapterOptions });

expect(polly.adapters.get('fetch').native).to.equal(fetch);
expect(polly.adapters.get('fetch').native).to.not.equal(
expect(polly.adapters.get('fetch').nativeFetch).to.equal(fetch);
offirgolan marked this conversation as resolved.
Show resolved Hide resolved
expect(polly.adapters.get('fetch').nativeFetch).to.not.equal(
adapterOptions.fetch.context.fetch
);

expect(polly.adapters.get('fetch').NativeRequest).to.equal(MockRequest);
expect(polly.adapters.get('fetch').NativeRequest).to.not.equal(
adapterOptions.fetch.context.Request
);

expect(function() {
polly.configure({
adapterOptions: { fetch: { context: {} } }
Expand All @@ -89,7 +180,7 @@ describe('Integration | Fetch Adapter | Init', function() {
await polly.stop();
});

it('should throw when context, fetch, Response, and Headers are undefined', async function() {
it('should throw when context, fetch, Request, Response, and Headers are undefined', async function() {
const polly = new Polly('context', { adapters: [] });

polly.configure({
Expand All @@ -108,6 +199,7 @@ describe('Integration | Fetch Adapter | Init', function() {
fetch: {
context: {
fetch: undefined,
Request: MockRequest,
Response: MockResponse,
Headers: MockHeaders
}
Expand All @@ -120,7 +212,27 @@ describe('Integration | Fetch Adapter | Init', function() {
polly.configure({
adapterOptions: {
fetch: {
context: { fetch() {}, Response: undefined, Headers: MockHeaders }
context: {
fetch() {},
Request: undefined,
Response: MockResponse,
Headers: MockHeaders
}
}
}
});
}).to.throw(/Request global not found/);

expect(function() {
polly.configure({
adapterOptions: {
fetch: {
context: {
fetch() {},
Request: MockRequest,
Response: undefined,
Headers: MockHeaders
}
}
}
});
Expand All @@ -132,6 +244,7 @@ describe('Integration | Fetch Adapter | Init', function() {
fetch: {
context: {
fetch() {},
Request: MockRequest,
Response: MockResponse,
Headers: undefined
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/adapter-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ export default function adapterTests() {
expect(res.ok).to.be.true;
});

describe('expired tests', () => {
describe('Expiration', () => {
async function testExpiration() {
const { persister, recordingId } = this.polly;
const url = '/api';
Expand Down