From 19a9577984e4490d35eaa8104093dd4f7c132daa Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Sun, 16 Dec 2018 23:33:21 +0100 Subject: [PATCH] refactor(window.ipfs): backward-compatible errors To avoid breaking changes we should deprecate old error codes first. This commit: - restores `permission` attribute and adds a deprecation warning when it is accessed. - adds error `code` to simplify error handling and alight convention with js-ipfs - updates docs --- add-on/src/lib/ipfs-proxy/pre-acl.js | 16 +++++-- add-on/src/lib/ipfs-proxy/pre-command.js | 16 +++++-- docs/window.ipfs.md | 46 ++++++++----------- .../lib/ipfs-proxy/enable-command.test.js | 8 ++-- .../functional/lib/ipfs-proxy/pre-acl.test.js | 7 +-- ...-whitelist.test.js => pre-command.test.js} | 7 +-- 6 files changed, 57 insertions(+), 43 deletions(-) rename test/functional/lib/ipfs-proxy/{pre-api-whitelist.test.js => pre-command.test.js} (87%) diff --git a/add-on/src/lib/ipfs-proxy/pre-acl.js b/add-on/src/lib/ipfs-proxy/pre-acl.js index 9433ef0d5..febb53aaf 100644 --- a/add-on/src/lib/ipfs-proxy/pre-acl.js +++ b/add-on/src/lib/ipfs-proxy/pre-acl.js @@ -44,10 +44,20 @@ function createProxyAclError (scope, permission) { const permissions = Array.isArray(permission) ? permission : [permission] err.output = { payload: { - isIpfsProxyError: true, - isIpfsProxyAclError: true, + // Error follows convention from https://github.com/ipfs/js-ipfs/pull/1746/files + code: 'ERR_IPFS_PROXY_ACCESS_DENIED', permissions, - scope + scope, + // TODO: remove below after deprecation period ends with Q1 + get isIpfsProxyAclError () { + console.warn("[ipfs-companion] reading .isIpfsProxyAclError from Ipfs Proxy errors is deprecated, use '.code' instead") + return true + }, + get permission () { + if (!this.permissions || !this.permissions.length) return undefined + console.warn("[ipfs-companion] reading .permission from Ipfs Proxy errors is deprecated, use '.permissions' instead") + return this.permissions[0] + } } } return err diff --git a/add-on/src/lib/ipfs-proxy/pre-command.js b/add-on/src/lib/ipfs-proxy/pre-command.js index 2dc79df69..564e4f33f 100644 --- a/add-on/src/lib/ipfs-proxy/pre-command.js +++ b/add-on/src/lib/ipfs-proxy/pre-command.js @@ -29,9 +29,19 @@ function createCommandWhitelistError (permission) { const err = new Error(`Access to '${permission}' commands over IPFS Proxy is globally blocked`) err.output = { payload: { - isIpfsProxyError: true, - isIpfsProxyWhitelistError: true, - permissions + // Error follows convention from https://github.com/ipfs/js-ipfs/pull/1746/files + code: 'ERR_IPFS_PROXY_ACCESS_DENIED', + permissions, + // TODO: remove below after deprecation period ends with Q1 + get isIpfsProxyWhitelistError () { + console.warn("[ipfs-companion] reading .isIpfsProxyWhitelistError from Ipfs Proxy errors is deprecated, use '.code' instead") + return true + }, + get permission () { + if (!this.permissions || !this.permissions.length) return undefined + console.warn("[ipfs-companion] reading .permission from Ipfs Proxy errors is deprecated, use '.permissions' instead") + return this.permissions[0] + } } } return err diff --git a/docs/window.ipfs.md b/docs/window.ipfs.md index 0536593d3..64a5dd9e4 100644 --- a/docs/window.ipfs.md +++ b/docs/window.ipfs.md @@ -7,8 +7,8 @@ > Want to help in shaping it? See [#589](https://github.com/ipfs-shipyard/ipfs-companion/issues/589) and [issues with `window.ipfs` label](https://github.com/ipfs-shipyard/ipfs-companion/labels/window.ipfs). - [Background](#background) -- [Creating applications using window.ipfs](#creating-applications-using-windowipfs) - - [Error messages](#error-messages) +- [Creating applications using `window.ipfs`](#creating-applications-using-windowipfs) + - [Error Codes](#error-codes) - [Q&A](#qa) - [What is a `window.ipfs`?](#what-is-a-windowipfs) - [How do I fallback if `window.ipfs` is not available?](#how-do-i-fallback-if-windowipfs-is-not-available) @@ -42,7 +42,7 @@ if (window.ipfs && window.ipfs.enable) { } ``` -To add and get content, you could do something like this: +To add and get content, you could update above example to do something like this: ```js if (window.ipfs && window.ipfs.enable) { @@ -52,15 +52,17 @@ if (window.ipfs && window.ipfs.enable) { const data = await ipfs.cat(hash) console.log(data.toString()) // =^.^= } catch (err) { - if (err.isIpfsProxyAclError) { - // Fallback - console.log('Unable to get ACL decision from user :(', err) + if (err.code === 'ERR_IPFS_PROXY_ACCESS_DENIED') { + // Proxy is present but user denied access. + // (fallback to js-ipfs or js-ipfs-http-client goes here) } else { + // Something else went wrong (error handling) throw err } } } else { - // Fallback + // No IPFS Proxy + // (fallback to js-ipfs or js-ipfs-http-client goes here) } ``` @@ -68,39 +70,27 @@ Note that IPFS Companion also adds `window.Buffer` if it doesn't already exist. See also: [How do I fallback if `window.ipfs` is not available?](#how-do-i-fallback-if-windowipfs-is-not-available) -### Error messages - -If access was denied: - -``` -User denied access to ${permission} -``` - -If the user closes the dialog without making a decision: +### Error Codes -``` -Failed to obtain access response for ${permission} at ${scope} -``` - -If access to IPFS was disabled entirely: - -``` -User disabled access to IPFS -``` +Errors returned by IPFS Proxy can be identified by the value of `code` attribute: -Note these might have been re-worded already. Please send a PR. +#### `ERR_IPFS_PROXY_ACCESS_DENIED` +Thrown when current scope has no access rights to requested commands. +Optional `scope` and `permissions` attributes provide detailed information: + - IF access was denied for a specific command THEN the `permissions` list is present and includes names of blocked commands + - IF entire IPFS Proxy was disabled by the user THEN the `permissions` list is missing entirely # Q&A ## What _is_ a `window.ipfs`? -It is an endpoint that enables you to obtain IPFS API instance. +It is an IPFS Proxy endpoint that enables you to obtain IPFS API instance. Depending how IPFS companion is configured, you may be talking directly to a `js-ipfs` node running in the companion, a `go-ipfs` daemon over `js-ipfs-http-client` or a `js-ipfs` daemon over `js-ipfs-http-client` and potentially others in the future. -Note that object returned by `window.ipfs.enable` is _not_ an instance of `js-ipfs` or `js-ipfs-http-client` but is a proxy to one of them, so don't expect to be able to detect either of them or be able to use any undocumented or instance specific functions. +Note that object returned by `window.ipfs.enable` is _not_ an instance of `js-ipfs` or `js-ipfs-http-client` but is a Proxy to one of them, so don't expect to be able to detect either of them or be able to use any undocumented or instance specific functions. See the [js-ipfs](https://github.com/ipfs/js-ipfs#api)/[js-ipfs-http-client](https://github.com/ipfs/js-ipfs-http-client#api) docs for available functions. If you find that some new functions are missing, the proxy might be out of date. Please check the [current status](https://github.com/tableflip/ipfs-postmsg-proxy#current-status) and submit a PR. diff --git a/test/functional/lib/ipfs-proxy/enable-command.test.js b/test/functional/lib/ipfs-proxy/enable-command.test.js index 611e2f3c0..be2aa7857 100644 --- a/test/functional/lib/ipfs-proxy/enable-command.test.js +++ b/test/functional/lib/ipfs-proxy/enable-command.test.js @@ -230,10 +230,12 @@ describe('lib/ipfs-proxy/enable-command', () => { } expect(error.output.payload).to.deep.eql({ - isIpfsProxyError: true, - isIpfsProxyAclError: true, + code: 'ERR_IPFS_PROXY_ACCESS_DENIED', permissions: permissions.commands, - scope: getScope() + scope: getScope(), + isIpfsProxyAclError: true, // deprecated + permission: permissions.commands[0] // deprecated + }) }) diff --git a/test/functional/lib/ipfs-proxy/pre-acl.test.js b/test/functional/lib/ipfs-proxy/pre-acl.test.js index 73784d161..c2151a8f6 100644 --- a/test/functional/lib/ipfs-proxy/pre-acl.test.js +++ b/test/functional/lib/ipfs-proxy/pre-acl.test.js @@ -133,10 +133,11 @@ describe('lib/ipfs-proxy/pre-acl', () => { } expect(error.output.payload).to.deep.eql({ - isIpfsProxyError: true, - isIpfsProxyAclError: true, + code: 'ERR_IPFS_PROXY_ACCESS_DENIED', permissions: [permission], - scope: getScope() + scope: getScope(), + isIpfsProxyAclError: true, // deprecated + permission // deprecated }) }) diff --git a/test/functional/lib/ipfs-proxy/pre-api-whitelist.test.js b/test/functional/lib/ipfs-proxy/pre-command.test.js similarity index 87% rename from test/functional/lib/ipfs-proxy/pre-api-whitelist.test.js rename to test/functional/lib/ipfs-proxy/pre-command.test.js index 0ff2d786d..4d6dda769 100644 --- a/test/functional/lib/ipfs-proxy/pre-api-whitelist.test.js +++ b/test/functional/lib/ipfs-proxy/pre-command.test.js @@ -37,9 +37,10 @@ describe('lib/ipfs-proxy/pre-command', () => { } expect(error.output.payload).to.deep.eql({ - isIpfsProxyError: true, - isIpfsProxyWhitelistError: true, - permissions: [permission] + code: 'ERR_IPFS_PROXY_ACCESS_DENIED', + permissions: [permission], + isIpfsProxyWhitelistError: true, // deprecated + permission // deprecated }) })