Feat: IpfsApiAccessError

[JonKrone]

Use a common Error type, IpfsApiAccessError, when an app is denied access to a window.ipfs api.

Discussed in #455.

Remaining:

• Make this type available somewhere under window.ipfs. Perhaps the ipfs.types or ipfs.utils
  from Support and reuse ipfs.types and ipfs.util #444?

This also fixes some path.sep related test failures on Windows, lemme know what you think about that.

[JonKrone]

Make this type available somewhere under window.ipfs. Perhaps the ipfs.types or ipfs.utils
from #444?

Looking for thoughts here.

[lidel]

Browserify issue?

While it seems to work in tests, real life version injected as content script does not seem to produce Error with additional attributes. Try executing this against some website (and deny access to trigger error):

ipfs.files.add(Buffer.from('seed')).catch(e => console.log(`Error.permission=${e.permission}, scope=${e.scope}, message=${e.message}`))

I get:

Are you able to reproduce?

Namespace for proxy-specific stuff

The ipfs.types sounds like an obvious place, but I am worried about unexpected consequences of polluting original interface-ipfs-core API namespaces with proxy-specific stuff.

Perhaps a safer alternative is to use window.ipfs.proxy (idea suggested in #454 (comment)) and expose it at ipfs.proxy.types.IpfsApiAccessError.

@olizilla @alanshaw do we have any strong opinions or preferred conventions for handling this?

[alanshaw]

Yes, I posted my comments here #455 (comment)

[alanshaw]

On the posix stuff, I did check this originally because it needs to be posix.

The path module as included by browserify is https://github.com/substack/path-browserify/blob/master/index.js which seems to be using the posix versions and doesn't include a path.posix.

[lidel]

@JonKrone to limit the number of moving pieces, let's go with version proposed by @alanshaw in #455 (comment) (no new type, just a self-explanatory property):

if (err.isIpfsProxyAclError) {

That way we don't have to change anything on the backend and we don't need to think where to put the new type.

[JonKrone]

Hey guys, I'm sorry for the delay! I was totally swamped last week.

I pushed some changes a few days ago, thanks @alanshaw for the solution with ipfs-postmsg.

re: the posix path fix for tests on Windows: I explicitly imported path-browserify. We could alternatively manually normalize the paths, let me know what you think.

[lidel]

Thanks! The key properties that address needs raised in #455 look goood 👌

Small details remain to be fixed before we merge it, see below.

[lidel]

I think explicitly using path-browserify (which provides posix version by default) is bit safer than assuming browserify-ing process will swap the implementation.

Let's merge this, and if at some point path-browserify turns out to be not enough, we can swap it with own implementation / wrapper.

@alanshaw does it sound ok to you?

[lidel]

I would replace console.log(':(') with console.log('Unable to get ACL decision from user :(', err) to make it more obvious for first-timers :)

[lidel]

Sorry it took so long to review this :)

Doc with err.isIpfsProxyAclError looks good, let's merge this.