Drop custom Promises and refactor to async functions (node-fetch#845)

* refactor to async

* no custsom promises anymore

* restore server premature handler

* simplify

* fixing break

* lint

* remove promise dependency

* fix docs

Author: tinovyatkin

README.md

@@ -85,9 +85,9 @@ See Jason Miller's [isomorphic-unfetch](https://www.npmjs.com/package/isomorphic
 
 - Stay consistent with `window.fetch` API.
 - Make conscious trade-off when following [WHATWG fetch spec][whatwg-fetch] and [stream spec](https://streams.spec.whatwg.org/) implementation details, document known differences.
-- Use native promise, but allow substituting it with [insert your favorite promise library].
+- Use native promise and async functions.
 - Use native Node streams for body, on both request and response.
-- Decode content encoding (gzip/deflate) properly, and convert string output (such as `res.text()` and `res.json()`) to UTF-8 automatically.
+- Decode content encoding (gzip/deflate/brotli) properly, and convert string output (such as `res.text()` and `res.json()`) to UTF-8 automatically.
 - Useful extensions such as redirect limit, response size limit, [explicit errors][error-handling.md] for troubleshooting.
 
 ## Difference from client-side fetch
@@ -116,15 +116,6 @@ const fetch = require('node-fetch');
 import fetch from 'node-fetch';
 ```
 
-If you are using a Promise library other than native, set it through `fetch.Promise`:
-
-```js
-const fetch = require('node-fetch');
-const Bluebird = require('bluebird');
-
-fetch.Promise = Bluebird;
-```
-
 If you want to patch the global object in node:
 
 ```js

package.json

@@ -60,7 +60,6 @@
 		"mocha": "^7.1.2",
 		"p-timeout": "^3.2.0",
 		"parted": "^0.1.1",
-		"promise": "^8.1.0",
 		"resumer": "0.0.0",
 		"rollup": "^2.10.8",
 		"string-to-arraybuffer": "^1.0.2",

src/body.js

@@ -5,7 +5,7 @@
  * Body interface provides common methods for Request and Response
  */
 
-import Stream, {finished, PassThrough} from 'stream';
+import Stream, {PassThrough} from 'stream';
 import {types} from 'util';
 
 import Blob from 'fetch-blob';
@@ -148,22 +148,22 @@ Object.defineProperties(Body.prototype, {
  *
  * @return  Promise
  */
-const consumeBody = data => {
+async function consumeBody(data) {
 	if (data[INTERNALS].disturbed) {
-		return Body.Promise.reject(new TypeError(`body used already for: ${data.url}`));
+		throw new TypeError(`body used already for: ${data.url}`);
 	}
 
 	data[INTERNALS].disturbed = true;
 
 	if (data[INTERNALS].error) {
-		return Body.Promise.reject(data[INTERNALS].error);
+		throw data[INTERNALS].error;
 	}
 
 	let {body} = data;
 
 	// Body is null
 	if (body === null) {
-		return Body.Promise.resolve(Buffer.alloc(0));
+		return Buffer.alloc(0);
 	}
 
 	// Body is blob
@@ -173,61 +173,49 @@ const consumeBody = data => {
 
 	// Body is buffer
 	if (Buffer.isBuffer(body)) {
-		return Body.Promise.resolve(body);
+		return body;
 	}
 
 	/* c8 ignore next 3 */
 	if (!(body instanceof Stream)) {
-		return Body.Promise.resolve(Buffer.alloc(0));
+		return Buffer.alloc(0);
 	}
 
 	// Body is stream
 	// get ready to actually consume the body
 	const accum = [];
 	let accumBytes = 0;
-	let abort = false;
 
-	return new Body.Promise((resolve, reject) => {
-		body.on('data', chunk => {
-			if (abort || chunk === null) {
-				return;
-			}
-
-			if (data.size && accumBytes + chunk.length > data.size) {
-				abort = true;
-				reject(new FetchError(`content size at ${data.url} over limit: ${data.size}`, 'max-size'));
-				return;
+	try {
+		for await (const chunk of body) {
+			if (data.size > 0 && accumBytes + chunk.length > data.size) {
+				const err = new FetchError(`content size at ${data.url} over limit: ${data.size}`, 'max-size');
+				body.destroy(err);
+				throw err;
 			}
 
 			accumBytes += chunk.length;
 			accum.push(chunk);
-		});
+		}
+	} catch (error) {
+		if (isAbortError(error) || error instanceof FetchError) {
+			throw error;
+		} else {
+			// Other errors, such as incorrect content-encoding
+			throw new FetchError(`Invalid response body while trying to fetch ${data.url}: ${error.message}`, 'system', error);
+		}
+	}
 
-		finished(body, {writable: false}, err => {
-			if (err) {
-				if (isAbortError(err)) {
-					// If the request was aborted, reject with this Error
-					abort = true;
-					reject(err);
-				} else {
-					// Other errors, such as incorrect content-encoding
-					reject(new FetchError(`Invalid response body while trying to fetch ${data.url}: ${err.message}`, 'system', err));
-				}
-			} else {
-				if (abort) {
-					return;
-				}
-
-				try {
-					resolve(Buffer.concat(accum, accumBytes));
-				} catch (error) {
-					// Handle streams that have accumulated too much data (issue #414)
-					reject(new FetchError(`Could not create Buffer from response body for ${data.url}: ${error.message}`, 'system', error));
-				}
-			}
-		});
-	});
-};
+	if (body.readableEnded === true || body._readableState.ended === true) {
+		try {
+			return Buffer.concat(accum, accumBytes);
+		} catch (error) {
+			throw new FetchError(`Could not create Buffer from response body for ${data.url}: ${error.message}`, 'system', error);
+		}
+	} else {
+		throw new FetchError(`Premature close of server response while trying to fetch ${data.url}`);
+	}
+}
 
 /**
  * Clone body given Res/Req instance
@@ -370,5 +358,3 @@ export const writeToStream = (dest, {body}) => {
 	}
 };
 
-// Expose Promise
-Body.Promise = global.Promise;

src/index.js

@@ -12,7 +12,7 @@ import zlib from 'zlib';
 import Stream, {PassThrough, pipeline as pump} from 'stream';
 import dataURIToBuffer from 'data-uri-to-buffer';
 
-import Body, {writeToStream, getTotalBytes} from './body.js';
+import {writeToStream, getTotalBytes} from './body.js';
 import Response from './response.js';
 import Headers, {fromRawHeaders} from './headers.js';
 import Request, {getNodeRequestOptions} from './request.js';
@@ -29,32 +29,25 @@ export {Headers, Request, Response, FetchError, AbortError, isRedirect};
  * @param   Object   opts  Fetch options
  * @return  Promise
  */
-export default function fetch(url, options_) {
-	// Allow custom promise
-	if (!fetch.Promise) {
-		throw new Error('native promise missing, set fetch.Promise to your favorite alternative');
-	}
-
+export default async function fetch(url, options_) {
 	// Regex for data uri
 	const dataUriRegex = /^\s*data:([a-z]+\/[a-z]+(;[a-z-]+=[a-z-]+)?)?(;base64)?,[\w!$&',()*+;=\-.~:@/?%\s]*\s*$/i;
 
 	// If valid data uri
 	if (dataUriRegex.test(url)) {
 		const data = dataURIToBuffer(url);
 		const response = new Response(data, {headers: {'Content-Type': data.type}});
-		return fetch.Promise.resolve(response);
+		return response;
 	}
 
 	// If invalid data uri
 	if (url.toString().startsWith('data:')) {
 		const request = new Request(url, options_);
-		return fetch.Promise.reject(new FetchError(`[${request.method}] ${request.url} invalid URL`, 'system'));
+		throw new FetchError(`[${request.method}] ${request.url} invalid URL`, 'system');
 	}
 
-	Body.Promise = fetch.Promise;
-
 	// Wrap http.request into fetch
-	return new fetch.Promise((resolve, reject) => {
+	return new Promise((resolve, reject) => {
 		// Build request object
 		const request = new Request(url, options_);
 		const options = getNodeRequestOptions(request);
@@ -290,5 +283,3 @@ export default function fetch(url, options_) {
 	});
 }
 
-// Expose Promise
-fetch.Promise = global.Promise;

test/main.js

@@ -10,7 +10,6 @@ import chai from 'chai';
 import chaiPromised from 'chai-as-promised';
 import chaiIterator from 'chai-iterator';
 import chaiString from 'chai-string';
-import then from 'promise';
 import resumer from 'resumer';
 import FormData from 'form-data';
 import stringToArrayBuffer from 'string-to-arraybuffer';
@@ -77,29 +76,10 @@ describe('node-fetch', () => {
 	it('should return a promise', () => {
 		const url = `${base}hello`;
 		const p = fetch(url);
-		expect(p).to.be.an.instanceof(fetch.Promise);
+		expect(p).to.be.an.instanceof(Promise);
 		expect(p).to.have.property('then');
 	});
 
-	it('should allow custom promise', () => {
-		const url = `${base}hello`;
-		const old = fetch.Promise;
-		fetch.Promise = then;
-		expect(fetch(url)).to.be.an.instanceof(then);
-		expect(fetch(url)).to.not.be.an.instanceof(old);
-		fetch.Promise = old;
-	});
-
-	it('should throw error when no promise implementation are found', () => {
-		const url = `${base}hello`;
-		const old = fetch.Promise;
-		fetch.Promise = undefined;
-		expect(() => {
-			fetch(url);
-		}).to.throw(Error);
-		fetch.Promise = old;
-	});
-
 	it('should expose Headers, Response and Request constructors', () => {
 		expect(FetchError).to.equal(FetchErrorOrig);
 		expect(Headers).to.equal(HeadersOrig);