From 25baade9ef7f78935dff44dd65d68be3270fef4e Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 28 Aug 2020 12:03:39 +0100 Subject: [PATCH 01/10] fix: use native fetch if available If you are in the Electron Renderer process, native fetch is available but it's ignored because the renderer does not respect the browser field of package.json, so we end up with node-fetch that uses a browser polyfill of the node http api. Instead use native fetch if it's available or node fetch if not. --- package.json | 4 +++- src/http.js | 11 ++++++----- test/http.spec.js | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 0ef447f..e17a473 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "license": "MIT", "dependencies": { "abort-controller": "^3.0.0", - "any-signal": "^1.1.0", + "any-signal": "achingbrain/any-signal#fix/use-native-abort-controller-if-available", "buffer": "^5.6.0", "err-code": "^2.0.0", "fs-extra": "^9.0.1", @@ -43,6 +43,8 @@ "it-glob": "0.0.8", "merge-options": "^2.0.0", "nanoid": "^3.1.3", + "native-abort-controller": "0.0.3", + "native-browser-fetch": "0.0.1", "node-fetch": "^2.6.0", "stream-to-it": "^0.2.0" }, diff --git a/src/http.js b/src/http.js index 6655e06..42718af 100644 --- a/src/http.js +++ b/src/http.js @@ -1,16 +1,17 @@ /* eslint-disable no-undef */ 'use strict' -const fetch = require('node-fetch') +const { + default: fetch, + Request, + Headers +} = require('native-browser-fetch') const merge = require('merge-options').bind({ ignoreUndefined: true }) const { URL, URLSearchParams } = require('iso-url') const TextDecoder = require('./text-decoder') -const AbortController = require('abort-controller') +const AbortController = require('native-abort-controller') const anySignal = require('any-signal') -const Request = fetch.Request -const Headers = fetch.Headers - class TimeoutError extends Error { constructor () { super('Request timed out') diff --git a/test/http.spec.js b/test/http.spec.js index b79ea62..3c02951 100644 --- a/test/http.spec.js +++ b/test/http.spec.js @@ -5,7 +5,7 @@ const { expect } = require('aegir/utils/chai') const HTTP = require('../src/http') const toStream = require('it-to-stream') const delay = require('delay') -const AbortController = require('abort-controller') +const AbortController = require('native-abort-controller') const drain = require('it-drain') const all = require('it-all') const { isBrowser, isWebWorker } = require('../src/env') From b047b22877d94fd0c75c063c725486e55f52f5b8 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 3 Sep 2020 08:02:19 +0100 Subject: [PATCH 02/10] chore: swap out native-browser-fetch for native-fetch --- package.json | 2 +- src/http.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index e17a473..b0f9709 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "merge-options": "^2.0.0", "nanoid": "^3.1.3", "native-abort-controller": "0.0.3", - "native-browser-fetch": "0.0.1", + "native-fetch": "^2.0.0", "node-fetch": "^2.6.0", "stream-to-it": "^0.2.0" }, diff --git a/src/http.js b/src/http.js index 42718af..c7ff902 100644 --- a/src/http.js +++ b/src/http.js @@ -5,7 +5,7 @@ const { default: fetch, Request, Headers -} = require('native-browser-fetch') +} = require('native-fetch') const merge = require('merge-options').bind({ ignoreUndefined: true }) const { URL, URLSearchParams } = require('iso-url') const TextDecoder = require('./text-decoder') From 8bbfc10d3b138c08d531404c94b5815d90ee93f5 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Tue, 6 Oct 2020 15:59:42 +0100 Subject: [PATCH 03/10] chore: remove gh dep --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b0f9709..21079ce 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "license": "MIT", "dependencies": { "abort-controller": "^3.0.0", - "any-signal": "achingbrain/any-signal#fix/use-native-abort-controller-if-available", + "any-signal": "^2.1.0", "buffer": "^5.6.0", "err-code": "^2.0.0", "fs-extra": "^9.0.1", From 5ee52857d847900c83719568ca7de568fb0e1344 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Wed, 7 Oct 2020 14:21:16 -0700 Subject: [PATCH 04/10] chore: reset on top of master --- package.json | 9 ++- src/abort-controller.browser.js | 4 + src/abort-controller.js | 9 +++ src/http.js | 25 ++---- src/http/error.js | 26 ++++++ src/http/fetch.browser.js | 135 ++++++++++++++++++++++++++++++++ src/http/fetch.js | 9 +++ src/http/fetch.node.js | 101 ++++++++++++++++++++++++ src/http/fetch.polyfill.js | 22 ++++++ test/http.spec.js | 30 ++++++- 10 files changed, 346 insertions(+), 24 deletions(-) create mode 100644 src/abort-controller.browser.js create mode 100644 src/abort-controller.js create mode 100644 src/http/error.js create mode 100644 src/http/fetch.browser.js create mode 100644 src/http/fetch.js create mode 100644 src/http/fetch.node.js create mode 100644 src/http/fetch.polyfill.js diff --git a/package.json b/package.json index 0ef447f..2dc35a6 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,8 @@ "dist" ], "browser": { + "./src/http/fetch.js": "./src/http/fetch.browser.js", + "./src/abort-controller.js": "./src/abort-controller.browser.js", "./src/text-encoder.js": "./src/text-encoder.browser.js", "./src/text-decoder.js": "./src/text-decoder.browser.js", "./src/temp-dir.js": "./src/temp-dir.browser.js", @@ -34,7 +36,7 @@ "license": "MIT", "dependencies": { "abort-controller": "^3.0.0", - "any-signal": "^1.1.0", + "any-signal": "^1.2.0", "buffer": "^5.6.0", "err-code": "^2.0.0", "fs-extra": "^9.0.1", @@ -44,7 +46,8 @@ "merge-options": "^2.0.0", "nanoid": "^3.1.3", "node-fetch": "^2.6.0", - "stream-to-it": "^0.2.0" + "stream-to-it": "^0.2.0", + "it-to-stream": "^0.1.2" }, "devDependencies": { "aegir": "^25.0.0", @@ -52,7 +55,7 @@ "it-all": "^1.0.2", "it-drain": "^1.0.1", "it-last": "^1.0.2", - "it-to-stream": "^0.1.2" + "uint8arrays": "^1.1.0" }, "contributors": [ "Hugo Dias ", diff --git a/src/abort-controller.browser.js b/src/abort-controller.browser.js new file mode 100644 index 0000000..1c36f73 --- /dev/null +++ b/src/abort-controller.browser.js @@ -0,0 +1,4 @@ +'use strict' +/* eslint-env browser */ + +module.exports = AbortController diff --git a/src/abort-controller.js b/src/abort-controller.js new file mode 100644 index 0000000..76ad7df --- /dev/null +++ b/src/abort-controller.js @@ -0,0 +1,9 @@ +'use strict' + +// Electron has `AbortController` and should use that instead of custom. +if (typeof AbortController === 'function') { + /* eslint-env browser */ + module.exports = require('./abort-controller.browser') +} else { + module.exports = require('abort-controller') +} diff --git a/src/http.js b/src/http.js index 6655e06..bba2ef5 100644 --- a/src/http.js +++ b/src/http.js @@ -1,31 +1,14 @@ /* eslint-disable no-undef */ 'use strict' -const fetch = require('node-fetch') +const { fetch, Request, Headers } = require('./http/fetch') +const { TimeoutError, HTTPError } = require('./http/error') const merge = require('merge-options').bind({ ignoreUndefined: true }) const { URL, URLSearchParams } = require('iso-url') const TextDecoder = require('./text-decoder') -const AbortController = require('abort-controller') +const AbortController = require('./abort-controller') const anySignal = require('any-signal') -const Request = fetch.Request -const Headers = fetch.Headers - -class TimeoutError extends Error { - constructor () { - super('Request timed out') - this.name = 'TimeoutError' - } -} - -class HTTPError extends Error { - constructor (response) { - super(response.statusText) - this.name = 'HTTPError' - this.response = response - } -} - const timeout = (promise, ms, abortController) => { if (ms === undefined) { return promise @@ -87,6 +70,8 @@ const defaults = { * @prop {function(URLSearchParams): URLSearchParams } [transformSearchParams] * @prop {function(any): any} [transform] - When iterating the response body, transform each chunk with this function. * @prop {function(Response): Promise} [handleError] - Handle errors + * @prop {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress] - Can be passed to track upload progress. + * Note that if this option in passed undelying request will be performed using `XMLHttpRequest` and response will not be streamed. */ class HTTP { diff --git a/src/http/error.js b/src/http/error.js new file mode 100644 index 0000000..057d2c5 --- /dev/null +++ b/src/http/error.js @@ -0,0 +1,26 @@ +'use strict' + +class TimeoutError extends Error { + constructor (message = 'Request timed out') { + super(message) + this.name = 'TimeoutError' + } +} +exports.TimeoutError = TimeoutError + +class AbortError extends Error { + constructor (message = 'The operation was aborted.') { + super(message) + this.name = 'AbortError' + } +} +exports.AbortError = AbortError + +class HTTPError extends Error { + constructor (response) { + super(response.statusText) + this.name = 'HTTPError' + this.response = response + } +} +exports.HTTPError = HTTPError diff --git a/src/http/fetch.browser.js b/src/http/fetch.browser.js new file mode 100644 index 0000000..691523a --- /dev/null +++ b/src/http/fetch.browser.js @@ -0,0 +1,135 @@ +'use strict' +/* eslint-env browser */ + +const { TimeoutError, AbortError } = require('./error') +const { Request, Response, Headers, fetch } = require('./fetch.polyfill') + +/** + * @typedef {RequestInit & ExtraFetchOptions} FetchOptions + * @typedef {Object} ExtraFetchOptions + * @property {number} [timeout] + * @property {URLSearchParams} [searchParams] + * @property {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress] + * @property {string} [overrideMimeType] + * @returns {Promise} + */ + +/** + * @param {string|URL} url + * @param {FetchOptions} [options] + * @returns {Promise} + */ +const fetchWithProgress = (url, options = {}) => { + const request = new XMLHttpRequest() + request.open(options.method || 'GET', url.toString(), true) + + const { timeout } = options + if (timeout > 0 && timeout < Infinity) { + request.timeout = options.timeout + } + + if (options.overrideMimeType != null) { + request.overrideMimeType(options.overrideMimeType) + } + + if (options.headers) { + for (const [name, value] of options.headers.entries()) { + request.setRequestHeader(name, value) + } + } + + if (options.signal) { + options.signal.onabort = () => request.abort() + } + + if (options.onUploadProgress) { + request.upload.onprogress = options.onUploadProgress + } + + // Note: Need to use `arraybuffer` here instead of `blob` because `Blob` + // instances coming from JSDOM are not compatible with `Response` from + // node-fetch (which is the setup we get when testing with jest because + // it uses JSDOM which does not provide a global fetch + // https://github.com/jsdom/jsdom/issues/1724) + request.responseType = 'arraybuffer' + + return new Promise((resolve, reject) => { + /** + * @param {Event} event + */ + const handleEvent = (event) => { + switch (event.type) { + case 'error': { + resolve(Response.error()) + break + } + case 'load': { + resolve( + new ResponseWithURL(request.responseURL, request.response, { + status: request.status, + statusText: request.statusText, + headers: parseHeaders(request.getAllResponseHeaders()) + }) + ) + break + } + case 'timeout': { + reject(new TimeoutError()) + break + } + case 'abort': { + reject(new AbortError()) + break + } + default: { + break + } + } + } + request.onerror = handleEvent + request.onload = handleEvent + request.ontimeout = handleEvent + request.onabort = handleEvent + + request.send(options.body) + }) +} + +const fetchWithStreaming = fetch + +const fetchWith = (url, options = {}) => + (options.onUploadProgress != null) + ? fetchWithProgress(url, options) + : fetchWithStreaming(url, options) + +exports.fetch = fetchWith +exports.Request = Request +exports.Headers = Headers + +/** + * @param {string} input + * @returns {Headers} + */ +const parseHeaders = (input) => { + const headers = new Headers() + for (const line of input.trim().split(/[\r\n]+/)) { + const index = line.indexOf(': ') + if (index > 0) { + headers.set(line.slice(0, index), line.slice(index + 1)) + } + } + + return headers +} + +class ResponseWithURL extends Response { + /** + * @param {string} url + * @param {string|Blob|ArrayBufferView|ArrayBuffer|FormData|ReadableStream} body + * @param {ResponseInit} options + */ + constructor (url, body, options) { + super(body, options) + Object.defineProperty(this, 'url', { value: url }) + } +} diff --git a/src/http/fetch.js b/src/http/fetch.js new file mode 100644 index 0000000..c968b1e --- /dev/null +++ b/src/http/fetch.js @@ -0,0 +1,9 @@ +'use strict' + +// Electron has `XMLHttpRequest` and should get the browser implementation +// instead of node. +if (typeof XMLHttpRequest !== 'undefined') { + module.exports = require('./fetch.browser') +} else { + module.exports = require('./fetch.node') +} diff --git a/src/http/fetch.node.js b/src/http/fetch.node.js new file mode 100644 index 0000000..399213d --- /dev/null +++ b/src/http/fetch.node.js @@ -0,0 +1,101 @@ +// @ts-check +'use strict' + +/** @type {import('node-fetch') & typeof fetch} */ +// @ts-ignore +const nodeFetch = require('node-fetch') +const toStream = require('it-to-stream') +const { Buffer } = require('buffer') +const { Request, Response, Headers } = nodeFetch +/** + * @typedef {RequestInit & ExtraFetchOptions} FetchOptions + * + * @typedef {import('stream').Readable} Readable + * @typedef {Object} LoadProgress + * @property {number} total + * @property {number} loaded + * @property {boolean} lengthComputable + * @typedef {Object} ExtraFetchOptions + * @property {number} [timeout] + * @property {URLSearchParams} [searchParams] + * @property {function(LoadProgress):void} [onUploadProgress] + * @property {function(LoadProgress):void} [onDownloadProgress] + * @property {string} [overrideMimeType] + * @returns {Promise} + */ + +/** + * @param {string|URL} url + * @param {FetchOptions} [options] + * @returns {Promise} + */ +const fetch = (url, options = {}) => + nodeFetch(url, withUploadProgress(options)) + +exports.fetch = fetch +exports.Request = Request +exports.Headers = Headers + +/** + * Takes fetch options and wraps request body to track uploda progress if + * `onUploadProgress` is supplied. Otherwise returns options as is. + * @param {FetchOptions} options + * @returns {FetchOptions} + */ +const withUploadProgress = (options) => { + const { onUploadProgress } = options + if (onUploadProgress) { + return { + ...options, + // @ts-ignore + body: bodyWithUploadProgress(options, onUploadProgress) + } + } else { + return options + } +} + +/** + * Takes request `body` and `onUploadProgress` handler and returns wrapped body + * that as consumed will report progress to suppled `onUploadProgress` handler. + * @param {FetchOptions} init + * @param {function(LoadProgress):void} onUploadProgress + * @returns {Readable} + */ +const bodyWithUploadProgress = (init, onUploadProgress) => { + // @ts-ignore - node-fetch is typed poorly + const { body } = new Response(init.body, init) + // @ts-ignore - Unlike standard Response, node-fetch `body` has a differnt + // type see: see https://github.com/node-fetch/node-fetch/blob/master/src/body.js + const source = iterateBodyWithProgress(body, onUploadProgress) + return toStream.readable(source) +} + +/** + * Takes body from node-fetch response as body and `onUploadProgress` handler + * and returns async iterable that emits body chunks and emits + * `onUploadProgress`. + * @param {Buffer|null|Readable} body + * @param {function(LoadProgress):void} onUploadProgress + * @returns {AsyncIterable} + */ +const iterateBodyWithProgress = async function * (body, onUploadProgress) { + /** @type {Buffer|null|Readable} */ + if (body == null) { + onUploadProgress({ total: 0, loaded: 0, lengthComputable: true }) + } else if (Buffer.isBuffer(body)) { + const total = body.byteLength + const lengthComputable = true + yield body + onUploadProgress({ total, loaded: total, lengthComputable }) + } else { + const total = 0 + const lengthComputable = false + let loaded = 0 + for await (const chunk of body) { + loaded += chunk.byteLength + yield chunk + onUploadProgress({ total, loaded, lengthComputable }) + } + } +} diff --git a/src/http/fetch.polyfill.js b/src/http/fetch.polyfill.js new file mode 100644 index 0000000..bcb3a5d --- /dev/null +++ b/src/http/fetch.polyfill.js @@ -0,0 +1,22 @@ +'use strict' + +/* eslint-env browser */ + +// JSDOM has `XMLHttpRequest` but it does not have a `fetch` or `Response` so +// we workaround by pulling in node-fetch. +// See: https://github.com/jsdom/jsdom/issues/1724 +exports.fetch = typeof fetch === 'function' + ? fetch + : require('node-fetch') + +exports.Response = typeof Response === 'function' + ? Response + : require('node-fetch').Response + +exports.Request = typeof Request === 'function' + ? Request + : require('node-fetch').Response + +exports.Headers = typeof Headers === 'function' + ? Headers + : require('node-fetch').Headers diff --git a/test/http.spec.js b/test/http.spec.js index b79ea62..ab98d75 100644 --- a/test/http.spec.js +++ b/test/http.spec.js @@ -5,11 +5,13 @@ const { expect } = require('aegir/utils/chai') const HTTP = require('../src/http') const toStream = require('it-to-stream') const delay = require('delay') -const AbortController = require('abort-controller') +const AbortController = require('../src/abort-controller') const drain = require('it-drain') const all = require('it-all') const { isBrowser, isWebWorker } = require('../src/env') const { Buffer } = require('buffer') +const uint8ArrayFromString = require('uint8arrays/from-string') +const uint8ArrayEquals = require('uint8arrays/equals') describe('http', function () { it('makes a GET request', async function () { @@ -150,4 +152,30 @@ describe('http', function () { await expect(drain(HTTP.ndjson(res.body))).to.eventually.be.rejectedWith(/aborted/) }) + + it('progress events', async () => { + let upload = 0 + const body = new Uint8Array(1000000 / 2) + const request = await HTTP.post(`${process.env.ECHO_SERVER}/echo`, { + body, + onUploadProgress: (progress) => { + expect(progress).to.have.property('lengthComputable').to.be.a('boolean') + expect(progress).to.have.property('total', body.byteLength) + expect(progress).to.have.property('loaded').to.be.greaterThan(0) + upload += 1 + } + }) + await all(request.iterator()) + + expect(upload).to.be.greaterThan(0) + }) + + it('makes a GET request with unprintable characters', async function () { + const buf = uint8ArrayFromString('a163666f6f6c6461672d63626f722d626172', 'base16') + const params = Array.from(buf).map(val => `data=${val.toString()}`).join('&') + + const req = await HTTP.get(`${process.env.ECHO_SERVER}/download?${params}`) + const rsp = await req.arrayBuffer() + expect(uint8ArrayEquals(new Uint8Array(rsp), buf)).to.be.true() + }) }) From afb78819067ffa53547704a2dee26ac1d74fc33a Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Mon, 12 Oct 2020 16:34:51 -0700 Subject: [PATCH 05/10] chore: remove redundunt abort-controller --- src/abort-controller.browser.js | 4 ---- src/abort-controller.js | 9 --------- 2 files changed, 13 deletions(-) delete mode 100644 src/abort-controller.browser.js delete mode 100644 src/abort-controller.js diff --git a/src/abort-controller.browser.js b/src/abort-controller.browser.js deleted file mode 100644 index 1c36f73..0000000 --- a/src/abort-controller.browser.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict' -/* eslint-env browser */ - -module.exports = AbortController diff --git a/src/abort-controller.js b/src/abort-controller.js deleted file mode 100644 index 76ad7df..0000000 --- a/src/abort-controller.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict' - -// Electron has `AbortController` and should use that instead of custom. -if (typeof AbortController === 'function') { - /* eslint-env browser */ - module.exports = require('./abort-controller.browser') -} else { - module.exports = require('abort-controller') -} From 8111a80b470108b7602d034f440129a224c9125e Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Mon, 12 Oct 2020 17:05:48 -0700 Subject: [PATCH 06/10] chore: update resolve API change incompabilities --- src/http/fetch.browser.js | 3 +-- src/http/fetch.node.js | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/http/fetch.browser.js b/src/http/fetch.browser.js index dc0bc98..275b912 100644 --- a/src/http/fetch.browser.js +++ b/src/http/fetch.browser.js @@ -2,8 +2,7 @@ /* eslint-env browser */ const { TimeoutError, AbortError } = require('./error') -const fetch = require('../fetch') -const { Request, Response, Headers } = fetch +const { Request, Response, Headers } = require('../fetch') /** * @typedef {RequestInit & ExtraFetchOptions} FetchOptions diff --git a/src/http/fetch.node.js b/src/http/fetch.node.js index 3952214..b78f2ff 100644 --- a/src/http/fetch.node.js +++ b/src/http/fetch.node.js @@ -1,12 +1,10 @@ // @ts-check 'use strict' -/** @type {import('node-fetch') & typeof fetch} */ -// @ts-ignore -const nodeFetch = require('../fetch') +const { Request, Response, Headers, default: nodeFetch } = require('../fetch') const toStream = require('it-to-stream') const { Buffer } = require('buffer') -const { Request, Response, Headers } = nodeFetch + /** * @typedef {RequestInit & ExtraFetchOptions} FetchOptions * @@ -30,6 +28,7 @@ const { Request, Response, Headers } = nodeFetch * @returns {Promise} */ const fetch = (url, options = {}) => + // @ts-ignore nodeFetch(url, withUploadProgress(options)) exports.fetch = fetch From 1a56843a63425e957be1df3ccf3f8a23db80b0ad Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Mon, 12 Oct 2020 17:44:06 -0700 Subject: [PATCH 07/10] fix: workaround electron-fetch issues --- src/http/fetch.node.js | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/http/fetch.node.js b/src/http/fetch.node.js index b78f2ff..17dc775 100644 --- a/src/http/fetch.node.js +++ b/src/http/fetch.node.js @@ -62,14 +62,37 @@ const withUploadProgress = (options) => { * @returns {Readable} */ const bodyWithUploadProgress = (init, onUploadProgress) => { - // @ts-ignore - node-fetch is typed poorly - const { body } = new Response(init.body, init) + // This works around the fact that electron-fetch serializes `Uint8Array`s + // and `ArrayBuffer`'s to strings. + const content = normalizeBody(init.body) + + // @ts-ignore - Response does not accept node `Readable` streams. + const { body } = new Response(content, init) // @ts-ignore - Unlike standard Response, node-fetch `body` has a differnt // type see: see https://github.com/node-fetch/node-fetch/blob/master/src/body.js const source = iterateBodyWithProgress(body, onUploadProgress) return toStream.readable(source) } +/** + * @param {BodyInit} [input] + * @returns {Buffer|Readable|Blob|null} + */ +const normalizeBody = (input = null) => { + if (input instanceof ArrayBuffer) { + return Buffer.from(input) + } else if (ArrayBuffer.isView(input)) { + return Buffer.from(input.buffer, input.byteOffset, input.byteLength) + } else if (typeof input === 'string') { + return Buffer.from(input) + } else { + // @ts-ignore - Could be FormData|URLSearchParams|ReadableStream + // however electron-fetch does not support either of those types and + // node-fetch normalizes those to node streams. + return input + } +} + /** * Takes body from native-fetch response as body and `onUploadProgress` handler * and returns async iterable that emits body chunks and emits From 6947fcd8eb713e05283b0017be8d9342238e2385 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Thu, 12 Nov 2020 23:20:52 -0800 Subject: [PATCH 08/10] chore: apply suggestions from code review --- src/http.js | 2 +- src/http/fetch.node.js | 6 +++--- test/http.spec.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/http.js b/src/http.js index 37f4ca6..00e1002 100644 --- a/src/http.js +++ b/src/http.js @@ -71,7 +71,7 @@ const defaults = { * @prop {function(any): any} [transform] - When iterating the response body, transform each chunk with this function. * @prop {function(Response): Promise} [handleError] - Handle errors * @prop {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress] - Can be passed to track upload progress. - * Note that if this option in passed undelying request will be performed using `XMLHttpRequest` and response will not be streamed. + * Note that if this option in passed underlying request will be performed using `XMLHttpRequest` and response will not be streamed. */ class HTTP { diff --git a/src/http/fetch.node.js b/src/http/fetch.node.js index 17dc775..a37295b 100644 --- a/src/http/fetch.node.js +++ b/src/http/fetch.node.js @@ -36,7 +36,7 @@ exports.Request = Request exports.Headers = Headers /** - * Takes fetch options and wraps request body to track uploda progress if + * Takes fetch options and wraps request body to track upload progress if * `onUploadProgress` is supplied. Otherwise returns options as is. * @param {FetchOptions} options * @returns {FetchOptions} @@ -56,14 +56,14 @@ const withUploadProgress = (options) => { /** * Takes request `body` and `onUploadProgress` handler and returns wrapped body - * that as consumed will report progress to suppled `onUploadProgress` handler. + * that as consumed will report progress to supplied `onUploadProgress` handler. * @param {FetchOptions} init * @param {function(LoadProgress):void} onUploadProgress * @returns {Readable} */ const bodyWithUploadProgress = (init, onUploadProgress) => { // This works around the fact that electron-fetch serializes `Uint8Array`s - // and `ArrayBuffer`'s to strings. + // and `ArrayBuffer`s to strings. const content = normalizeBody(init.body) // @ts-ignore - Response does not accept node `Readable` streams. diff --git a/test/http.spec.js b/test/http.spec.js index a32dfaf..ee98527 100644 --- a/test/http.spec.js +++ b/test/http.spec.js @@ -161,7 +161,7 @@ describe('http', function () { onUploadProgress: (progress) => { expect(progress).to.have.property('lengthComputable').to.be.a('boolean') expect(progress).to.have.property('total', body.byteLength) - expect(progress).to.have.property('loaded').to.be.greaterThan(0) + expect(progress).to.have.property('loaded').that.is.greaterThan(0) upload += 1 } }) From e0a521944e367ea1b1149a40c7258b03a9e22384 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Thu, 12 Nov 2020 23:31:38 -0800 Subject: [PATCH 09/10] chore: address review comment --- test/http.spec.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/http.spec.js b/test/http.spec.js index ee98527..e5dc3f5 100644 --- a/test/http.spec.js +++ b/test/http.spec.js @@ -12,6 +12,7 @@ const { isBrowser, isWebWorker } = require('../src/env') const { Buffer } = require('buffer') const uint8ArrayFromString = require('uint8arrays/from-string') const uint8ArrayEquals = require('uint8arrays/equals') +const uint8ArrayConcat = require('uint8arrays/concat') describe('http', function () { it('makes a GET request', async function () { @@ -165,7 +166,9 @@ describe('http', function () { upload += 1 } }) - await all(request.iterator()) + + const out = uint8ArrayConcat(await all(request.iterator())) + expect(uint8ArrayEquals(out, body)) expect(upload).to.be.greaterThan(0) }) From e9d3b05ffd11ebcf95c18e9e97647131d8556d09 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Thu, 12 Nov 2020 23:37:23 -0800 Subject: [PATCH 10/10] fix: lint issues --- src/http/fetch.node.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/http/fetch.node.js b/src/http/fetch.node.js index a37295b..bf3cf97 100644 --- a/src/http/fetch.node.js +++ b/src/http/fetch.node.js @@ -38,6 +38,7 @@ exports.Headers = Headers /** * Takes fetch options and wraps request body to track upload progress if * `onUploadProgress` is supplied. Otherwise returns options as is. + * * @param {FetchOptions} options * @returns {FetchOptions} */ @@ -57,6 +58,7 @@ const withUploadProgress = (options) => { /** * Takes request `body` and `onUploadProgress` handler and returns wrapped body * that as consumed will report progress to supplied `onUploadProgress` handler. + * * @param {FetchOptions} init * @param {function(LoadProgress):void} onUploadProgress * @returns {Readable} @@ -97,6 +99,7 @@ const normalizeBody = (input = null) => { * Takes body from native-fetch response as body and `onUploadProgress` handler * and returns async iterable that emits body chunks and emits * `onUploadProgress`. + * * @param {Buffer|null|Readable} body * @param {function(LoadProgress):void} onUploadProgress * @returns {AsyncIterable}