Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Support node's ReadStream in ipfs.add instead of AsyncIterable<Uint8Array> / Iterable<Uint8Array> #3188

Closed
Gozala opened this issue Jul 20, 2020 · 5 comments

Comments

@Gozala
Copy link
Contributor

Gozala commented Jul 20, 2020

This is a followup to #3167. I have proposed to simplify ipfs.add even further by removing Iterable<Uint8Array> and AsyncIterable<Uint8Array> from the list of possible input types.

As per #3167 (comment) (quoted bolew for convenience)

We support AsyncIterable in order to support adding node streams:

 const fs = require('rs')
 ipfs.add(fs.createReadStream('/path/to/file'))

We could get rid of Iterable<Uint8Array>|AsyncIterable<Uint8Array> and still support above use case, by adding node ReadStream (which is what fs.createReadStream returns) to the list of accepted options.

That would significantly reduce complexity of the normaliseInput and wire protocol encoder used in the #3022, but more importantly it remove mental overhead of the API and all the false positives that can occur when probing inputs.

@Gozala Gozala added the need/triage Needs initial labeling and prioritization label Jul 20, 2020
@Gozala
Copy link
Contributor Author

Gozala commented Jul 20, 2020

While we're at it I would also like to propose to remove Iterable<Uint8Array> from the FileContent because:

It introduces this ambiguity when it comes to choosing right strategy. Which is something I have faced both in #3022 and #3151

It becomes unclear whether chunks are already in memory (where better strategy would be to collect them and pass them together down the pipe) or not e.g. they are generated on consumption (in which case it would be better to avoid buffering). It is also not a purely an optimization thing, it adds complexity to the implementation as well.

Which is why I would like to propose dropping Iterable<Uint8Array> and limit FileContent to: Uint8Array | Blob | String | AsyncIterable<Uint8Array> because:

  • Based on input type it is clear which strategy to use.
  • If user has multiple chunks they want to use they are more informed to choose right strategy:
    • Concat chunks: ipfs.add({ content: new Blob(chunks) }) / ipfs.add({ concat: Buffer.concat(chunks) })
    • Stream chunks: ipfs.add({ async * concat() { yield * chunks } })

@achingbrain achingbrain added exploration and removed need/triage Needs initial labeling and prioritization labels Jul 22, 2020
@achingbrain
Copy link
Member

achingbrain commented Jul 22, 2020

The whole idea here is that node streams are async iterators, and we use async iterators internally extensively, so let's just support async iterators and have everything else convert to that. Non-async iterators still work with await for..of so it's cheap to support them too.

We support Blobs so people can drag and drop files into a UI, but they'll get turned into an async iterator internally when running a node in-browser. If we're just running the HTTP client in the browser, then we can keep Blobs as Blobs and pass them to FormData to send to the remote node. When it arrives at the remote node we're back to async iterators.

If user has multiple chunks they want to use they are more informed to choose right strategy

If the user has chunks, they should be able to just pass them in without making any choices. No wrapping with a Blob or a file object.

@markg85
Copy link

markg85 commented May 19, 2022

I'm curious about this one too.
I did try exactly as @Gozala suggested with a multi-gigabyte-file but always seem to be getting a code: 'UND_ERR_TRAILER_MISMATCH' error.

The full stack trace in my case:

(node:3886) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
    at emitExperimentalWarning (node:internal/util:224:11)
    at fetch (node:internal/bootstrap/pre_execution:198:5)
    at fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http/fetch.node.js:20:3)
    at Client.fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http.js:129:7)
    at Client.fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/lib/core.js:141:20)
    at Client.post (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http.js:171:17)
    at addAll (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/add-all.js:21:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Object.last [as default] (/home/mark/GitProjects/wefm/node_modules/it-last/index.js:13:20)
    at async Object.add (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/add.js:18:14)
    at async Object.handler (/home/mark/GitProjects/wefm/index.js:66:29)
TypeError: terminated
    at Fetch.onAborted (node:internal/deps/undici/undici:6266:53)
    at Fetch.emit (node:events:527:28)
    at Fetch.terminate (node:internal/deps/undici/undici:5522:14)
    at Object.onError (node:internal/deps/undici/undici:6357:36)
    at Request.onError (node:internal/deps/undici/undici:2023:31)
    at errorRequest (node:internal/deps/undici/undici:3949:17)
    at Socket.onSocketClose (node:internal/deps/undici/undici:3411:9)
    at Socket.emit (node:events:527:28)
    at TCP.<anonymous> (node:net:715:12) {
  [cause]: TrailerMismatchError: Trailers does not match trailer header
      at Parser.onMessageComplete (node:internal/deps/undici/undici:3317:34)
      at wasm_on_message_complete (node:internal/deps/undici/undici:2953:34)
      at wasm://wasm/0002afd2:wasm-function[45]:0x8dc
      at wasm://wasm/0002afd2:wasm-function[56]:0x1ad3
      at wasm://wasm/0002afd2:wasm-function[55]:0xcd7
      at wasm://wasm/0002afd2:wasm-function[21]:0x4e4
      at Parser.execute (node:internal/deps/undici/undici:3055:26)
      at Parser.readMore (node:internal/deps/undici/undici:3034:16)
      at Socket.onSocketReadable (node:internal/deps/undici/undici:3364:14)
      at Socket.emit (node:events:527:28)
      at emitReadable_ (node:internal/streams/readable:590:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:81:21) {
    code: 'UND_ERR_TRAILER_MISMATCH'

In terms of version, i am using the latest at the moment of writing. For ipfs-http-client that is 56.0.3 and for node it's 18.1.0.

@aminhdev
Copy link

I'm curious about this one too. I did try exactly as @Gozala suggested with a multi-gigabyte-file but always seem to be getting a code: 'UND_ERR_TRAILER_MISMATCH' error.

The full stack trace in my case:

(node:3886) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
    at emitExperimentalWarning (node:internal/util:224:11)
    at fetch (node:internal/bootstrap/pre_execution:198:5)
    at fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http/fetch.node.js:20:3)
    at Client.fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http.js:129:7)
    at Client.fetch (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/lib/core.js:141:20)
    at Client.post (/home/mark/GitProjects/wefm/node_modules/ipfs-utils/src/http.js:171:17)
    at addAll (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/add-all.js:21:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Object.last [as default] (/home/mark/GitProjects/wefm/node_modules/it-last/index.js:13:20)
    at async Object.add (/home/mark/GitProjects/wefm/node_modules/ipfs-http-client/cjs/src/add.js:18:14)
    at async Object.handler (/home/mark/GitProjects/wefm/index.js:66:29)
TypeError: terminated
    at Fetch.onAborted (node:internal/deps/undici/undici:6266:53)
    at Fetch.emit (node:events:527:28)
    at Fetch.terminate (node:internal/deps/undici/undici:5522:14)
    at Object.onError (node:internal/deps/undici/undici:6357:36)
    at Request.onError (node:internal/deps/undici/undici:2023:31)
    at errorRequest (node:internal/deps/undici/undici:3949:17)
    at Socket.onSocketClose (node:internal/deps/undici/undici:3411:9)
    at Socket.emit (node:events:527:28)
    at TCP.<anonymous> (node:net:715:12) {
  [cause]: TrailerMismatchError: Trailers does not match trailer header
      at Parser.onMessageComplete (node:internal/deps/undici/undici:3317:34)
      at wasm_on_message_complete (node:internal/deps/undici/undici:2953:34)
      at wasm://wasm/0002afd2:wasm-function[45]:0x8dc
      at wasm://wasm/0002afd2:wasm-function[56]:0x1ad3
      at wasm://wasm/0002afd2:wasm-function[55]:0xcd7
      at wasm://wasm/0002afd2:wasm-function[21]:0x4e4
      at Parser.execute (node:internal/deps/undici/undici:3055:26)
      at Parser.readMore (node:internal/deps/undici/undici:3034:16)
      at Socket.onSocketReadable (node:internal/deps/undici/undici:3364:14)
      at Socket.emit (node:events:527:28)
      at emitReadable_ (node:internal/streams/readable:590:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:81:21) {
    code: 'UND_ERR_TRAILER_MISMATCH'

In terms of version, i am using the latest at the moment of writing. For ipfs-http-client that is 56.0.3 and for node it's 18.1.0.

I have the same issue

@SgtPooki SgtPooki self-assigned this May 17, 2023
@SgtPooki SgtPooki moved this to 🥞 Todo in js-ipfs deprecation May 17, 2023
@SgtPooki SgtPooki moved this from 🥞 Todo to 🏃‍♀️ In Progress in js-ipfs deprecation May 25, 2023
@SgtPooki
Copy link
Member

js-ipfs is being deprecated in favor of Helia. You can learn more about this deprecation and read the migration guide.

Please feel to reopen with any comments by 2023-06-02. We will do a final pass on reopened issues afterwards (see #4336).

Please attempt to use Helia for the same and open a bug there if you're still running into issues!

@github-project-automation github-project-automation bot moved this from 🏃‍♀️ In Progress to ✅ Done in js-ipfs deprecation May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants