From 3b2d9fe2b0659e5fad1c587a998d0fa94ada6ee9 Mon Sep 17 00:00:00 2001 From: Nick Young Date: Sat, 17 Mar 2018 23:17:59 -0400 Subject: [PATCH 1/5] Show which link does not exist on ipfs.cat --- src/core/components/files.js | 2 +- test/cli/files.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/components/files.js b/src/core/components/files.js index 4c66d954e0..cb5c8c8cc4 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -146,7 +146,7 @@ module.exports = function files (self) { files = files.filter(filterFile) } if (!files || !files.length) { - return d.abort(new Error('No such file')) + return d.abort(new Error(`no link named "${restPath}" under ${pathComponents[0]}`)) } const file = files[0] diff --git a/test/cli/files.js b/test/cli/files.js index 2e9fec39b5..0619f74a2f 100644 --- a/test/cli/files.js +++ b/test/cli/files.js @@ -322,7 +322,11 @@ describe('files', () => runOnAndOff((thing) => { return ipfs('cat QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB/dummy') .then(() => expect.fail(0, 1, 'Should have thrown an error')) .catch((err) => { + const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] expect(err).to.exist() + expect(message).to.eql( + 'no link named "dummy" under QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB' + ) }) }) From 87ab4bbf4c9967e613e3e398153464cb9ba9ebaf Mon Sep 17 00:00:00 2001 From: Nick Young Date: Sun, 18 Mar 2018 22:48:15 -0400 Subject: [PATCH 2/5] Determine which node in path is missing. --- src/core/components/files.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/core/components/files.js b/src/core/components/files.js index cb5c8c8cc4..1f6dc45873 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -131,22 +131,36 @@ module.exports = function files (self) { throw new Error('You must supply an ipfsPath') } + let bestMatch = 0 + ipfsPath = normalizePath(ipfsPath) const pathComponents = ipfsPath.split('/') const restPath = normalizePath(pathComponents.slice(1).join('/')) - const filterFile = (file) => (restPath && file.path === restPath) || (file.path === ipfsPath) + const filterFile = (file) => { + if (file.path === ipfsPath.substring(0, file.path.length)) { + const matchedNodes = file.path.split('/').length + + if (matchedNodes > bestMatch) { + bestMatch = matchedNodes + } + } + + return (restPath && file.path === restPath) || (file.path === ipfsPath) + } const d = deferred.source() pull( - exporter(ipfsPath, self._ipld), + exporter(pathComponents[0], self._ipld), pull.collect((err, files) => { if (err) { return d.abort(err) } if (files && files.length > 1) { files = files.filter(filterFile) } if (!files || !files.length) { - return d.abort(new Error(`no link named "${restPath}" under ${pathComponents[0]}`)) + const parent = pathComponents.slice(0, bestMatch).join('/') + const link = pathComponents.slice(bestMatch).join('/') + return d.abort(new Error(`no link named "${link}" under ${parent}`)) } const file = files[0] From aa2d6a65448b6d78baf71c4c30a188e3b3b95242 Mon Sep 17 00:00:00 2001 From: Nick Young Date: Tue, 20 Mar 2018 18:11:17 -0400 Subject: [PATCH 3/5] Add test for cat nested error. --- src/core/components/files.js | 5 +++-- test/cli/files.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/core/components/files.js b/src/core/components/files.js index 1f6dc45873..55c8cd55c9 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -135,6 +135,7 @@ module.exports = function files (self) { ipfsPath = normalizePath(ipfsPath) const pathComponents = ipfsPath.split('/') + const prePath = normalizePath(pathComponents.slice(0, 1).join('/')) const restPath = normalizePath(pathComponents.slice(1).join('/')) const filterFile = (file) => { if (file.path === ipfsPath.substring(0, file.path.length)) { @@ -151,10 +152,10 @@ module.exports = function files (self) { const d = deferred.source() pull( - exporter(pathComponents[0], self._ipld), + exporter(prePath, self._ipld), pull.collect((err, files) => { if (err) { return d.abort(err) } - if (files && files.length > 1) { + if (files) { files = files.filter(filterFile) } if (!files || !files.length) { diff --git a/test/cli/files.js b/test/cli/files.js index 0619f74a2f..24cb760d84 100644 --- a/test/cli/files.js +++ b/test/cli/files.js @@ -330,6 +330,20 @@ describe('files', () => runOnAndOff((thing) => { }) }) + it('cat specifies missing link in a nested directory', function () { + this.timeout(20 * 1000) + + return ipfs('cat QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU/docs/wrongdir/dummy') + .then(() => expect.fail(0, 1, 'Should have thrown an error')) + .catch((err) => { + const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] + expect(err).to.exist() + expect(message).to.eql( + 'no link named "wrongdir/dummy" under QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU/docs' + ) + }) + }) + it('get', function () { this.timeout(20 * 1000) From 4f3ec222cb4999306e02364f363b47e30f2b48ad Mon Sep 17 00:00:00 2001 From: Nick Young Date: Tue, 20 Mar 2018 22:31:17 -0400 Subject: [PATCH 4/5] Change cat error to match go-ipfs --- src/core/components/files.js | 14 ++++++-------- test/cli/files.js | 20 ++++++++++++++------ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/core/components/files.js b/src/core/components/files.js index 55c8cd55c9..f576cff7ee 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -131,7 +131,7 @@ module.exports = function files (self) { throw new Error('You must supply an ipfsPath') } - let bestMatch = 0 + let bestMatch = { depth: -1 } ipfsPath = normalizePath(ipfsPath) const pathComponents = ipfsPath.split('/') @@ -139,10 +139,8 @@ module.exports = function files (self) { const restPath = normalizePath(pathComponents.slice(1).join('/')) const filterFile = (file) => { if (file.path === ipfsPath.substring(0, file.path.length)) { - const matchedNodes = file.path.split('/').length - - if (matchedNodes > bestMatch) { - bestMatch = matchedNodes + if (file.depth > bestMatch.depth) { + bestMatch = file } } @@ -159,9 +157,9 @@ module.exports = function files (self) { files = files.filter(filterFile) } if (!files || !files.length) { - const parent = pathComponents.slice(0, bestMatch).join('/') - const link = pathComponents.slice(bestMatch).join('/') - return d.abort(new Error(`no link named "${link}" under ${parent}`)) + const hash = toB58String(bestMatch.hash) + const link = pathComponents[bestMatch.depth + 1] + return d.abort(new Error(`no link named "${link}" under ${hash}`)) } const file = files[0] diff --git a/test/cli/files.js b/test/cli/files.js index 24cb760d84..cb5d40d965 100644 --- a/test/cli/files.js +++ b/test/cli/files.js @@ -330,16 +330,24 @@ describe('files', () => runOnAndOff((thing) => { }) }) - it('cat specifies missing link in a nested directory', function () { - this.timeout(20 * 1000) - - return ipfs('cat QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU/docs/wrongdir/dummy') + it('cat specifies missing directory in a nested link', () => { + return ipfs('cat QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs/wrong-dir/dummy') + .then(() => expect.fail(0, 1, 'Should have thrown an error')) + .catch((err) => { + const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] + expect(message).to.eql( + 'no link named "wrong-dir" under QmegvLXxpVKiZ4b57Xs1syfBVRd8CbucVHAp7KpLQdGieC' + ) + }) + }) + + it('cat specifies missing file in a nested link', () => { + return ipfs('cat QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs/dummy') .then(() => expect.fail(0, 1, 'Should have thrown an error')) .catch((err) => { const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] - expect(err).to.exist() expect(message).to.eql( - 'no link named "wrongdir/dummy" under QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU/docs' + 'no link named "dummy" under QmegvLXxpVKiZ4b57Xs1syfBVRd8CbucVHAp7KpLQdGieC' ) }) }) From 8b6f79bcaadaa9d5b18fb9c85c33d32fd013b031 Mon Sep 17 00:00:00 2001 From: Nick Young Date: Wed, 21 Mar 2018 11:23:05 -0400 Subject: [PATCH 5/5] More specific cat errors. --- src/core/components/files.js | 22 ++++++++++++++++++++-- test/cli/files.js | 19 +++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/core/components/files.js b/src/core/components/files.js index f576cff7ee..57f4393d1b 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -138,6 +138,8 @@ module.exports = function files (self) { const prePath = normalizePath(pathComponents.slice(0, 1).join('/')) const restPath = normalizePath(pathComponents.slice(1).join('/')) const filterFile = (file) => { + // Save off best match to provide a better error message if file + // isn't found. if (file.path === ipfsPath.substring(0, file.path.length)) { if (file.depth > bestMatch.depth) { bestMatch = file @@ -157,9 +159,25 @@ module.exports = function files (self) { files = files.filter(filterFile) } if (!files || !files.length) { - const hash = toB58String(bestMatch.hash) + // File used as directory + if (bestMatch.type === 'file') { + const path = bestMatch.path.substring(0, bestMatch.path.length - bestMatch.name.length - 1) + return d.abort(new Error( + `"${bestMatch.name}" is a file not a directory under ${path}` + )) + } + const link = pathComponents[bestMatch.depth + 1] - return d.abort(new Error(`no link named "${link}" under ${hash}`)) + + // Missing directory + if (bestMatch.depth < pathComponents.length - 2) { + return d.abort(new Error( + `no directory named "${link}" under ${bestMatch.path}` + )) + } + + // Missing file + return d.abort(new Error(`no file named "${link}" under ${bestMatch.path}`)) } const file = files[0] diff --git a/test/cli/files.js b/test/cli/files.js index cb5d40d965..0c1e2a8af1 100644 --- a/test/cli/files.js +++ b/test/cli/files.js @@ -319,13 +319,13 @@ describe('files', () => runOnAndOff((thing) => { }) it('cat non-existent file', () => { - return ipfs('cat QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB/dummy') + return ipfs('cat QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU/dummy') .then(() => expect.fail(0, 1, 'Should have thrown an error')) .catch((err) => { const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] expect(err).to.exist() expect(message).to.eql( - 'no link named "dummy" under QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB' + 'no file named "dummy" under QmUhUuiTKkkK8J6JZ9zmj8iNHPuNfGYcszgRumzhHBxEEU' ) }) }) @@ -336,7 +336,7 @@ describe('files', () => runOnAndOff((thing) => { .catch((err) => { const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] expect(message).to.eql( - 'no link named "wrong-dir" under QmegvLXxpVKiZ4b57Xs1syfBVRd8CbucVHAp7KpLQdGieC' + 'no directory named "wrong-dir" under QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs' ) }) }) @@ -347,7 +347,18 @@ describe('files', () => runOnAndOff((thing) => { .catch((err) => { const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] expect(message).to.eql( - 'no link named "dummy" under QmegvLXxpVKiZ4b57Xs1syfBVRd8CbucVHAp7KpLQdGieC' + 'no file named "dummy" under QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs' + ) + }) + }) + + it('cat specifies a link is not a directory', () => { + return ipfs('cat QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs/index/dummy') + .then(() => expect.fail(0, 1, 'Should have thrown an error')) + .catch((err) => { + const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1] + expect(message).to.eql( + '"index" is a file not a directory under QmYmW4HiZhotsoSqnv2o1oUusvkRM8b9RweBoH7ao5nki2/init-docs/docs' ) }) })