Merge pull request #580 from wheresrhys/nasty-clone-bug

fix the node-fetch cloned response hanging bug as best I can

docs/_api-inspection/lastResponse.md

@@ -4,11 +4,20 @@ navTitle: .lastResponse()
 position: 5.5
 versionAdded: 9.10.0
 description: |-
-  Returns the `Response` for the last call to `fetch` matching the given `filter` and `options`. 
+  Returns the `Response` for the last call to `fetch` matching the given `filter` and `options`. This is an experimental feature, very difficult to implement well given fetch's very private treatment of response bodies. 
 
   If `.lastResponse()` is called before fetch has been resolved then it will return `undefined`
   {: .warning} 
 
+    When doing all the following:
+    - using node-fetch
+    - responding with a real network response (using spy() or fallbackToNetwork)
+    - using \`fetchMock.LastResponse()\`
+    - awaiting the body content  
+    ... the response will hang unless your source code also awaits the response body.
+    This is an unavoidable consequence of the nodejs implementation of streams.
+  {: .warning} 
+
   To obtain json/text responses await the `.json()/.text()` methods of the response
   {: .info}
 ---

src/lib/fetch-handler.js

@@ -217,7 +217,7 @@ FetchMock.generateResponse = async function ({
 	// If the response is a pre-made Response, respond with it
 	if (this.config.Response.prototype.isPrototypeOf(response)) {
 		debug('response is already a Response instance - returning it');
-		callLog.response = response.clone();
+		callLog.response = response;
 		return response;
 	}
 
@@ -229,7 +229,7 @@ FetchMock.generateResponse = async function ({
 		route,
 	});
 
-	callLog.response = realResponse.clone();
+	callLog.response = realResponse;
 
 	return finalResponse;
 };

src/lib/inspecting.js

@@ -102,7 +102,24 @@ FetchMock.lastOptions = formatDebug(function (nameOrMatcher, options) {
 
 FetchMock.lastResponse = formatDebug(function (nameOrMatcher, options) {
 	debug('retrieving respose of last matching call');
-	return (this.lastCall(nameOrMatcher, options) || []).response;
+	console.warn(`When doing all the following:
+- using node-fetch
+- responding with a real network response (using spy() or fallbackToNetwork)
+- using \`fetchMock.LastResponse()\`
+- awaiting the body content
+... the response will hang unless your source code also awaits the response body.
+This is an unavoidable consequence of the nodejs implementation of streams.
+`);
+	const response = (this.lastCall(nameOrMatcher, options) || []).response;
+	try {
+		const clonedResponse = response.clone();
+		return clonedResponse;
+	} catch (err) {
+		Object.entries(response._fmResults).forEach(([name, result]) => {
+			response[name] = () => result;
+		});
+		return response;
+	}
 });
 
 FetchMock.called = formatDebug(function (nameOrMatcher, options) {

src/lib/response-builder.js

@@ -151,7 +151,7 @@ e.g. {"body": {"status: "registered"}}`);
 
 	buildObservableResponse(response) {
 		const fetchMock = this.fetchMock;
-
+		response._fmResults = {};
 		// Using a proxy means we can set properties that may not be writable on
 		// the original Response. It also means we can track the resolution of
 		// promises returned by res.json(), res.text() etc
@@ -181,6 +181,7 @@ e.g. {"body": {"status: "registered"}}`);
 							const result = func.apply(response, args);
 							if (result.then) {
 								fetchMock._holdingPromises.push(result.catch(() => null));
+								originalResponse._fmResults[name] = result;
 							}
 							return result;
 						},

test/server-specs/server-only.test.js

@@ -2,7 +2,6 @@ const chai = require('chai');
 chai.use(require('sinon-chai'));
 const expect = chai.expect;
 const sinon = require('sinon');
-
 const { fetchMock } = testGlobals;
 describe('nodejs only tests', () => {
 	describe('support for nodejs body types', () => {
@@ -40,5 +39,19 @@ describe('nodejs only tests', () => {
 				done();
 			});
 		});
+
+		// See https://github.com/wheresrhys/fetch-mock/issues/575
+		it('can respond with large bodies from the interweb', async () => {
+			const fm = fetchMock.sandbox();
+			fm.config.fetch = require('node-fetch');
+			fm.config.fallbackToNetwork = true;
+			fm.mock();
+			// this is an adequate test because the response hangs if the
+			// bug referenced above creeps back in
+			await fm
+				.fetchHandler('http://www.wheresrhys.co.uk/assets/img/chaffinch.jpg')
+				// res.blob() woudl make more sense, but not supported by node-fetch@1
+				.then((res) => res.text());
+		});
 	});
 });

test/specs/inspecting.test.js

@@ -518,7 +518,8 @@ describe('inspecting', () => {
 			expect(fm.lastResponse().status).to.equal(201);
 			fm.restore();
 		});
-		it('has readable response', async () => {
+
+		it('has readable response when response already read if using lastResponse', async () => {
 			const respBody = { foo: 'bar' };
 			fm.once('*', { status: 200, body: respBody }).once('*', 201, {
 				overwriteRoutes: false,
@@ -527,7 +528,7 @@ describe('inspecting', () => {
 			const resp = await fm.fetchHandler('http://a.com/');
 
 			await resp.json();
-			expect(await fm.calls()[0].response.json()).to.eql(respBody);
+			expect(await fm.lastResponse().json()).to.eql(respBody);
 		});
 	});
 });