Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Handle case where ky responses have no body with a getter for a ReadableStream #1224

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ coverage
examples/sub-module/**/bundle.js
examples/sub-module/**/*-minified.js
examples/sub-module/*-bundle.js

.vscode/
8 changes: 4 additions & 4 deletions src/add-from-url.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
'use strict'

const kyDefault = require('ky-universal').default
const toIterable = require('./lib/stream-to-iterable')
const toAsyncIterable = require('./lib/stream-to-async-iterable')

module.exports = (config) => {
module.exports = config => {
pcowgill marked this conversation as resolved.
Show resolved Hide resolved
const add = require('./add')(config)

return async function * addFromURL (url, options) {
options = options || {}

const { body } = await kyDefault.get(url)
const res = await kyDefault.get(url)

const input = {
path: decodeURIComponent(new URL(url).pathname.split('/').pop() || ''),
content: toIterable(body)
content: toAsyncIterable(res)
}

yield * add(input, options)
Expand Down
33 changes: 24 additions & 9 deletions src/add/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')
const { toFormData } = require('./form-data')
const toCamel = require('../lib/object-to-camel')

Expand All @@ -16,21 +16,36 @@ module.exports = configure(({ ky }) => {
if (options.chunker) searchParams.set('chunker', options.chunker)
if (options.cidVersion) searchParams.set('cid-version', options.cidVersion)
if (options.cidBase) searchParams.set('cid-base', options.cidBase)
if (options.enableShardingExperiment != null) searchParams.set('enable-sharding-experiment', options.enableShardingExperiment)
if (options.enableShardingExperiment != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Were these changes necessary? I personally like curly brackets around if statement bodies, even one liners, but this has made everything inconsistent and is outside the scope of the intended change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, they weren't necessary. They happened due to the default code styling settings I have globally in VS Code. If this style is important and gates PR merging, though, we should modify the linter settings to throw a warning or an error with this style. Ideally, we would get aegir lint --fix to automatically resolve such warnings/errors too, which may happen out of the box when we enable the warning. I'll open an issue for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just opened the issue #1225

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've disabled the Prettier and JavaScript Standard Style extensions for this workspace locally in VS Code so that automatic linting changes won't make it into a PR against this repo next time, but I still think the linting settings change is worth making to make it simpler for others to make PRs in the future.

searchParams.set(
'enable-sharding-experiment',
options.enableShardingExperiment
)
}
if (options.hashAlg) searchParams.set('hash', options.hashAlg)
if (options.onlyHash != null) searchParams.set('only-hash', options.onlyHash)
if (options.onlyHash != null) { searchParams.set('only-hash', options.onlyHash) }
if (options.pin != null) searchParams.set('pin', options.pin)
if (options.progress) searchParams.set('progress', true)
if (options.quiet != null) searchParams.set('quiet', options.quiet)
if (options.quieter != null) searchParams.set('quieter', options.quieter)
if (options.rawLeaves != null) searchParams.set('raw-leaves', options.rawLeaves)
if (options.shardSplitThreshold) searchParams.set('shard-split-threshold', options.shardSplitThreshold)
if (options.rawLeaves != null) { searchParams.set('raw-leaves', options.rawLeaves) }
if (options.shardSplitThreshold) { searchParams.set('shard-split-threshold', options.shardSplitThreshold) }
if (options.silent) searchParams.set('silent', options.silent)
if (options.trickle != null) searchParams.set('trickle', options.trickle)
if (options.wrapWithDirectory != null) searchParams.set('wrap-with-directory', options.wrapWithDirectory)
if (options.wrapWithDirectory != null) { searchParams.set('wrap-with-directory', options.wrapWithDirectory) }
if (options.preload != null) searchParams.set('preload', options.preload)
if (options.fileImportConcurrency != null) searchParams.set('file-import-concurrency', options.fileImportConcurrency)
if (options.blockWriteConcurrency != null) searchParams.set('block-write-concurrency', options.blockWriteConcurrency)
if (options.fileImportConcurrency != null) {
searchParams.set(
'file-import-concurrency',
options.fileImportConcurrency
)
}
if (options.blockWriteConcurrency != null) {
searchParams.set(
'block-write-concurrency',
options.blockWriteConcurrency
)
}

const res = await ky.post('add', {
timeout: options.timeout,
Expand All @@ -40,7 +55,7 @@ module.exports = configure(({ ky }) => {
body: await toFormData(input)
})

for await (let file of ndjson(toIterable(res.body))) {
for await (let file of ndjson(toAsyncIterable(res))) {
file = toCamel(file)
// console.log(file)
if (options.progress && file.bytes) {
Expand Down
4 changes: 2 additions & 2 deletions src/block/rm-async-iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const CID = require('cids')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')
const toCamel = require('../lib/object-to-camel')

module.exports = configure(({ ky }) => {
Expand All @@ -30,7 +30,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const removed of ndjson(toIterable(res.body))) {
for await (const removed of ndjson(toAsyncIterable(res))) {
yield toCamel(removed)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cat.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const CID = require('cids')
const { Buffer } = require('buffer')
const configure = require('./lib/configure')
const toIterable = require('./lib/stream-to-iterable')
const toAsyncIterable = require('./lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * cat (path, options) {
Expand All @@ -27,7 +27,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const chunk of toIterable(res.body)) {
for await (const chunk of toAsyncIterable(res)) {
yield Buffer.from(chunk)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/dht/find-peer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const PeerInfo = require('peer-info')
const multiaddr = require('multiaddr')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * findPeer (peerId, options) {
Expand All @@ -22,7 +22,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const message of ndjson(toIterable(res.body))) {
for await (const message of ndjson(toAsyncIterable(res))) {
// 2 = FinalPeer
// https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L18
if (message.Type === 2 && message.Responses) {
Expand Down
6 changes: 3 additions & 3 deletions src/dht/find-provs.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ const PeerInfo = require('peer-info')
const multiaddr = require('multiaddr')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * findProvs (cid, options) {
options = options || {}

const searchParams = new URLSearchParams(options.searchParams)
searchParams.set('arg', `${cid}`)
if (options.numProviders) searchParams.set('num-providers', options.numProviders)
if (options.numProviders) { searchParams.set('num-providers', options.numProviders) }
if (options.verbose != null) searchParams.set('verbose', options.verbose)

const res = await ky.post('dht/findprovs', {
Expand All @@ -23,7 +23,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const message of ndjson(toIterable(res.body))) {
for await (const message of ndjson(toAsyncIterable(res))) {
// 4 = Provider
// https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L20
if (message.Type === 4 && message.Responses) {
Expand Down
4 changes: 2 additions & 2 deletions src/dht/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * get (key, options) {
Expand All @@ -19,7 +19,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const message of ndjson(toIterable(res.body))) {
for await (const message of ndjson(toAsyncIterable(res))) {
// 5 = Value
// https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L21
if (message.Type === 5) {
Expand Down
6 changes: 3 additions & 3 deletions src/dht/provide.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const PeerInfo = require('peer-info')
const multiaddr = require('multiaddr')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')
const toCamel = require('../lib/object-to-camel')

module.exports = configure(({ ky }) => {
Expand All @@ -15,7 +15,7 @@ module.exports = configure(({ ky }) => {

const searchParams = new URLSearchParams(options.searchParams)
cids.forEach(cid => searchParams.append('arg', `${cid}`))
if (options.recursive != null) searchParams.set('recursive', options.recursive)
if (options.recursive != null) { searchParams.set('recursive', options.recursive) }
if (options.verbose != null) searchParams.set('verbose', options.verbose)

const res = await ky.post('dht/provide', {
Expand All @@ -25,7 +25,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (let message of ndjson(toIterable(res.body))) {
for await (let message of ndjson(toAsyncIterable(res))) {
message = toCamel(message)
if (message.responses) {
message.responses = message.responses.map(({ ID, Addrs }) => {
Expand Down
12 changes: 8 additions & 4 deletions src/dht/put.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const PeerInfo = require('peer-info')
const multiaddr = require('multiaddr')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')
const encodeBufferURIComponent = require('../lib/encode-buffer-uri-component')
const toCamel = require('../lib/object-to-camel')

Expand All @@ -16,8 +16,12 @@ module.exports = configure(({ ky }) => {
const searchParams = new URLSearchParams(options.searchParams)
if (options.verbose != null) searchParams.set('verbose', options.verbose)

key = Buffer.isBuffer(key) ? encodeBufferURIComponent(key) : encodeURIComponent(key)
value = Buffer.isBuffer(value) ? encodeBufferURIComponent(value) : encodeURIComponent(value)
key = Buffer.isBuffer(key)
pcowgill marked this conversation as resolved.
Show resolved Hide resolved
? encodeBufferURIComponent(key)
: encodeURIComponent(key)
value = Buffer.isBuffer(value)
? encodeBufferURIComponent(value)
: encodeURIComponent(value)

const url = `dht/put?arg=${key}&arg=${value}&${searchParams}`
const res = await ky.post(url, {
Expand All @@ -26,7 +30,7 @@ module.exports = configure(({ ky }) => {
headers: options.headers
})

for await (let message of ndjson(toIterable(res.body))) {
for await (let message of ndjson(toAsyncIterable(res))) {
message = toCamel(message)
if (message.responses) {
message.responses = message.responses.map(({ ID, Addrs }) => {
Expand Down
4 changes: 2 additions & 2 deletions src/dht/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const PeerId = require('peer-id')
const PeerInfo = require('peer-info')
const ndjson = require('iterable-ndjson')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * query (peerId, options) {
Expand All @@ -21,7 +21,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const message of ndjson(toIterable(res.body))) {
for await (const message of ndjson(toAsyncIterable(res))) {
yield new PeerInfo(PeerId.createFromB58String(message.ID))
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/files/ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const CID = require('cids')
const ndjson = require('iterable-ndjson')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')
const configure = require('../lib/configure')
const toCamelWithMetadata = require('../lib/object-to-camel-with-metadata')

Expand All @@ -28,7 +28,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const result of ndjson(toIterable(res.body))) {
for await (const result of ndjson(toAsyncIterable(res))) {
// go-ipfs does not yet support the "stream" option
if ('Entries' in result) {
for (const entry of result.Entries || []) {
Expand Down
4 changes: 2 additions & 2 deletions src/files/read.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const { Buffer } = require('buffer')
const configure = require('../lib/configure')
const toIterable = require('../lib/stream-to-iterable')
const toAsyncIterable = require('../lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * read (path, options) {
Expand All @@ -20,7 +20,7 @@ module.exports = configure(({ ky }) => {
searchParams
})

for await (const chunk of toIterable(res.body)) {
for await (const chunk of toAsyncIterable(res)) {
yield Buffer.from(chunk)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const configure = require('./lib/configure')
const Tar = require('it-tar')
const { Buffer } = require('buffer')
const CID = require('cids')
const toIterable = require('./lib/stream-to-iterable')
const toAsyncIterable = require('./lib/stream-to-async-iterable')

module.exports = configure(({ ky }) => {
return async function * get (path, options) {
Expand Down Expand Up @@ -38,7 +38,7 @@ module.exports = configure(({ ky }) => {

const extractor = Tar.extract()

for await (const { header, body } of extractor(toIterable(res.body))) {
for await (const { header, body } of extractor(toAsyncIterable(res))) {
if (header.type === 'directory') {
yield {
path: header.name
Expand Down
40 changes: 40 additions & 0 deletions src/lib/stream-to-async-iterable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict'

module.exports = function toAsyncIterable (res) {
const { body } = res

// An env where res.body getter for ReadableStream with getReader
// is not supported, for example in React Native
if (!body) {
if (res.arrayBuffer) {
Copy link
Contributor Author

@pcowgill pcowgill Jan 22, 2020

Choose a reason for hiding this comment

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

@achingbrain @hugomrdias Do we want to console.log anything in this case, like "falling back to a hackier approach since the fetch being used here doesn't implement the streams spec"?

return (async function * () {
const arrayBuffer = await res.arrayBuffer()
yield arrayBuffer
})()
} else {
throw new Error('Neither Response.body nor Response.arrayBuffer is defined')
}
pcowgill marked this conversation as resolved.
Show resolved Hide resolved
}

// Node.js stream
if (body[Symbol.asyncIterator]) return body

// Browser ReadableStream
if (body.getReader) {
return (async function * () {
const reader = body.getReader()

try {
while (true) {
const { done, value } = await reader.read()
if (done) return
yield value
}
} finally {
reader.releaseLock()
}
})()
}

throw new Error('unknown stream')
}
25 changes: 0 additions & 25 deletions src/lib/stream-to-iterable.js

This file was deleted.

Loading