-
Notifications
You must be signed in to change notification settings - Fork 296
Handle case where ky responses have no body with a getter for a ReadableStream #1224
base: master
Are you sure you want to change the base?
Changes from 4 commits
f0d4016
37b7d27
3ee0016
74c286e
528a0b9
4ea0b03
02b6269
4819f0e
7a0d504
5fffc22
2508489
6585971
843a97e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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') | ||
|
||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just opened the issue #1225 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've disabled the |
||
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, | ||
|
@@ -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) { | ||
|
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @achingbrain @hugomrdias Do we want to |
||
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') | ||
} |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.