Incoherent error message with the file API specs

[fsdiogo]

Hey guys, I stumbled upon an error when trying the ipfs-101 example.

• Version: 0.28.0
• Platform: Darwin
• Subsystem: interface-ipfs-core

Type: Bug

Severity: Low

Description:

I was following the tutorial along and got to the adding a file to IPFS part:

// Create the File to add, a file consists of a path + content. More details on
// https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/FILES.md
(cb) => node.files.add({
  path: 'hello.txt',
  content: Buffer.from('Hello World')
}, (err, filesAdded) => {
  if (err) { return cb(err) }

  // Once the file is added, we get back an object containing the path, the
  // multihash and the sie of the file
  console.log('\nAdded file:', filesAdded[0].path, filesAdded[0].hash)
  fileMultihash = filesAdded[0].hash
  cb()
})

After running the script nothing was being logged because an error occurred. When logging the error I got the following output:

Error: Invalid arguments, data must be an object, Buffer or readable stream

The files API specs states:

JavaScript - ipfs.files.add(data, [options], [callback])
Where data may be:

a Buffer instance
a Readable Stream
a Pull Stream
a Path (caveat: will only work in Node.js)
a URL
an array of objects, each of the form:

So the error message isn't coherent with the files API.

To check which was wrong I looked at the files.js source code and found:

const ok = Buffer.isBuffer(data) ||
        isStream.readable(data) ||
        Array.isArray(data) ||
        OtherBuffer.isBuffer(data)

if (!ok) {
    return callback(new Error('Invalid arguments, data must be an object, Buffer or readable stream'))
}

So the error message is wrong, it should be:

Error: Invalid arguments, data must be an array of objects, Buffer or readable stream

Also, in the example it's missing the array, it's just an object, that's why it doesn't work. Putting the object inside an array works as expected 👍

I can open a PR with the fixes if you want!

[JonKrone]

Hey there @fsdiogo! Thanks for raising this issue.

It just so happens that a couple hours ago another user @tiagochen submitted a pull request (#1255) that addresses this, albeit in a slightly different manner. Strange! May I ask what brought you to the project?

He suggests another approach to fix this problem which is to just pass a Buffer of the content. Both approaches are totally valid and give different hints about how IPFS works during the tutorial. However, given that this is a bare-bones introduction to IPFS, I think that using a Buffer is a simple and clear way to introduce adding content to users. Once people start using IPFS in more complex ways, they are likely to stumble upon the [{ path, content }] option.

I bet that error was confusing, though. We'd love a PR for that!

[fsdiogo]

Hey @JonKrone!

Oh cool, I didn't check the open PRs, that would have saved me some time debugging! 😛

I've heard about IPFS and saw some of the talks Juan Benet gave, thought that this was a really smart and ambitious project, so I decided to get my hands dirty!

I thought of wrapping the object inside an array just to keep the example almost the same, but maybe passing a buffer is simpler 👍

Yup, it was confusing enough, but that's what made me dig a little deeper. That being said, I can make a PR to change the error, I'll leave the example out as it's already being fixed 🙂

Sounds good?

[daviddias]

@fsdiogo this was a nasty regression on the latest release, fix incoming with #1257 thanks for the catch!! :)

[fsdiogo]

@diasdavid cool, that solves it then 😄

[daviddias]

Version with the fix published :)

A new version of the package ipfs (0.28.1) was published at 2018-03-09T11:19:03.971Z from
85.247.71.153. The shasum of this package was a13018087e90f98cf496d84ceed2f6095236e3dc.

Thank you and Welcome to IPFS community :)