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

Drop custom Promises and refactor to async functions #845

Merged
merged 8 commits into from May 28, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 2 additions & 11 deletions README.md
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion package.json
Expand Up @@ -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",
Expand Down
78 changes: 32 additions & 46 deletions src/body.js
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -370,5 +358,3 @@ export const writeToStream = (dest, {body}) => {
}
};

// Expose Promise
Body.Promise = global.Promise;
19 changes: 5 additions & 14 deletions src/index.js
Expand Up @@ -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';
Expand All @@ -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);
Expand Down Expand Up @@ -290,5 +283,3 @@ export default function fetch(url, options_) {
});
}

// Expose Promise
fetch.Promise = global.Promise;
22 changes: 1 addition & 21 deletions test/main.js
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down