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

chore: move mfs and multipart files into core #2811

Merged
merged 23 commits into from
Mar 19, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Mar 3, 2020

I've converted the mfs tests to interface tests and they are a lot more thorough than the previous ones.

Turns out a whole bunch of stuff didn't work over http. Also, some of our interface tests now make go-ipfs fail, mostly around mfs operations on hamt shards - removing files from shards in particular. I've skipped all of these tests when we run against go-ipfs but we'll need some follow up issues.

BREAKING CHANGES:

ipfs.files.stat(path) run on a hamt sharded directory used to report a type of hamt-sharded-directory, now it reports directory to bring js-ipfs in line with go-ipfs.

@achingbrain achingbrain mentioned this pull request Mar 10, 2020
33 tasks
@achingbrain achingbrain marked this pull request as ready for review March 13, 2020 20:27
feat: streaming http requests

No more 4GB file limits.
BREAKING CHANGE:

When the path passed to `ipfs.files.stat(path)` was a hamt sharded dir, the resovled
value returned by js-ipfs previously had a `type` property of with a value of
`'hamt-sharded-directory'`.  To bring it in line with go-ipfs this value is now
`'directory'`.
@achingbrain achingbrain changed the title chore: move mfs files into core chore: move mfs and multipart files into core Mar 17, 2020
packages/interface-ipfs-core/src/files/cp.js Outdated Show resolved Hide resolved
packages/interface-ipfs-core/src/files/ls.js Outdated Show resolved Hide resolved
packages/interface-ipfs-core/src/files/mv.js Outdated Show resolved Hide resolved
packages/interface-ipfs-core/src/files/read.js Outdated Show resolved Hide resolved
packages/ipfs-http-client/package.json Outdated Show resolved Hide resolved
packages/ipfs-http-client/src/files/cp.js Show resolved Hide resolved
Comment on lines 126 to 137
let bodyErr

if (opts.body && (opts.body.on || opts.body.addEventListener)) {
const on = (opts.body.on || opts.body.addEventListener).bind(opts.body)

// TODO: not sure this will work after the high water mark is hit and the request
// starts streaming
on('error', (err) => {
bodyErr = err
})
}

Copy link
Member

Choose a reason for hiding this comment

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

can you explain why we need this ?

Copy link
Member Author

@achingbrain achingbrain Mar 18, 2020

Choose a reason for hiding this comment

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

If the body is a stream, and that stream errors the error is not surfaced to the calling code.

This will throw any error thrown until the response headers arrive. I'm not sure if we should keep it TBH as it stops working once the response headers have been received, after that any error is smothered, same as without the bodyErr lines in the PR.

It makes this test pass (http://localhost:3000 is an echo server):

it('should handle errors in streaming bodies', async function () {
  const err = new Error('Should be caught')
  const body = async function * () {
    yield Buffer.from([0, 1, 2])

    // await delay(1000) <-- uncomment this line and the test fails as the 
    // response headers arrive before err is thrown

    throw err
  }()

  await expect(HTTP.post('http://localhost:3000', {
    body: toStream.readable(body)
  })).to.eventually.be.rejectedWith(err)
})

packages/ipfs-utils/src/http.js Outdated Show resolved Hide resolved
packages/ipfs/src/core/components/files/index.js Outdated Show resolved Hide resolved
@achingbrain achingbrain merged commit 82b9e08 into master Mar 19, 2020
@achingbrain achingbrain deleted the refactor/merge-mfs-into-core branch March 19, 2020 18:08
mistakia pushed a commit to mistakia/js-ipfs that referenced this pull request Mar 22, 2020
BREAKING CHANGE:

When the path passed to `ipfs.files.stat(path)` was a hamt sharded dir, the resovled
value returned by js-ipfs previously had a `type` property of with a value of
`'hamt-sharded-directory'`.  To bring it in line with go-ipfs this value is now
`'directory'`.
SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
BREAKING CHANGE:

When the path passed to `ipfs.files.stat(path)` was a hamt sharded dir, the resovled
value returned by js-ipfs previously had a `type` property of with a value of
`'hamt-sharded-directory'`.  To bring it in line with go-ipfs this value is now
`'directory'`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants