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

feat: better error message on ipfs cat #1965

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/core/components/files-regular/cat-pull-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,40 @@ module.exports = function (self) {
options = options || {}

ipfsPath = normalizePath(ipfsPath)

const pathComponents = ipfsPath.split('/')
const fileNameOrHash = pathComponents[pathComponents.length - 1]
const cid = pathComponents[0]

if (options.preload !== false) {
self._preload(pathComponents[0])
}

const d = deferred.source()

let closestMatchedLink

pull(
exporter(ipfsPath, self._ipld, options),
pull.filter(file => file.path === fileNameOrHash),
exporter(cid, self._ipld, options),
Copy link
Member

Choose a reason for hiding this comment

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

If you export the CID not CID+path you'll have to traverse through the entire graph until you get to the path you were looking for. This could make cat VERY slow for big directories if the content is not in your repo already (and be slow anyway, even if it is).

cc @achingbrain

Copy link
Member

Choose a reason for hiding this comment

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

+1 that - if you pass the path as well as the CID (e.g. /ipfs/Qmfoo/path/to/file.txt) the the unixfs exporter is smart enough to not load whole DAGs for sharded directories, etc which saves a load of time & bandwidth. Maybe this change is better made there?

pull.filter(link => {
return link.path === ipfsPath.substring(0, link.path.length)
}),
pull.filter(link => {
// save the closest matched path so we can show in error if no file was found
if (!closestMatchedLink || link.depth > closestMatchedLink.depth) {
closestMatchedLink = link
}

return link.path === ipfsPath
}),
pull.take(1),
pull.collect((err, files) => {
if (err) {
return d.abort(err)
}

if (!files.length) {
return d.abort(new Error('No such file'))
const linkNotFound = ipfsPath.substring(closestMatchedLink.path.length + 1)
return d.abort(new Error(`no file named "${linkNotFound}" under ${closestMatchedLink.path}`))
}

const file = files[0]
Expand Down
10 changes: 10 additions & 0 deletions test/cli/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,16 @@ describe('files', () => runOnAndOff((thing) => {
.then(() => expect.fail(0, 1, 'Should have thrown an error'))
.catch((err) => {
expect(err).to.exist()
expect(err.stdout).to.contain('no file named "dummy" under QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB')
})
})

it('cat non-existent file (nested)', () => {
return ipfs('cat Qmaj2NmcyAXT8dFmZRRytE12wpcaHADzbChKToMEjBsj5Z/init-docs/wrong-dir/dummy')
.then(() => expect.fail(0, 1, 'Should have thrown an error'))
.catch((err) => {
expect(err).to.exist()
expect(err.stdout).to.contain('no file named "wrong-dir/dummy" under Qmaj2NmcyAXT8dFmZRRytE12wpcaHADzbChKToMEjBsj5Z/init-docs')
})
})

Expand Down