From 1d9d263bd1beb86f11cc2f7d882cac7bd55f2458 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Tue, 12 Nov 2019 21:42:49 +0000 Subject: [PATCH 1/8] refactor: convert config.get to async await --- src/config/get.js | 54 ++++++++++++++++++--------------------------- src/config/index.js | 4 +++- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/config/get.js b/src/config/get.js index 882efe3df..b8a42a2e3 100644 --- a/src/config/get.js +++ b/src/config/get.js @@ -1,39 +1,27 @@ 'use strict' -const promisify = require('promisify-es6') +const configure = require('../lib/configure') -const toObject = function (res, callback) { - if (Buffer.isBuffer(res)) { - callback(null, JSON.parse(res.toString())) - } else { - callback(null, res) - } -} - -module.exports = (send) => { - return promisify((key, callback) => { - if (typeof key === 'function') { - callback = key - key = undefined +module.exports = configure(({ ky }) => { + return async (key, options) => { + if (key && typeof key === 'object') { + options = key + key = null } - if (!key) { - send.andTransform({ - path: 'config/show', - buffer: true - }, toObject, callback) - return - } + options = options || {} - send.andTransform({ - path: 'config', - args: key, - buffer: true - }, toObject, (err, response) => { - if (err) { - return callback(err) - } - callback(null, response.Value) - }) - }) -} + const searchParams = new URLSearchParams(options.searchParams) + if (key) searchParams.set('arg', key) + + const url = key ? 'config' : 'config/show' + const data = await ky.get(url, { + timeout: options.timeout, + signal: options.signal, + headers: options.headers, + searchParams + }).json() + + return key ? data.Value : data + } +}) diff --git a/src/config/index.js b/src/config/index.js index e88c195b5..3ece84ef9 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -1,8 +1,10 @@ 'use strict' +const callbackify = require('callbackify') + module.exports = (send, config) => { return { - get: require('./get')(send), + get: callbackify.variadic(require('./get')(config)), set: require('./set')(send), replace: require('./replace')(send), profiles: { From 359f13f7a8da4565b50a52712b14560d93655009 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 13:40:20 +0000 Subject: [PATCH 2/8] refactor: convert config.profiles.* and config.replace/set to async/await License: MIT Signed-off-by: Alan Shaw --- src/config/index.js | 17 +++++--------- src/config/profiles/apply.js | 21 ++++++++--------- src/config/profiles/index.js | 8 +++++++ src/config/profiles/list.js | 15 +++++------- src/config/replace.js | 36 +++++++++++++---------------- src/config/set.js | 45 ++++++++++++++++++------------------ src/utils/load-commands.js | 2 +- 7 files changed, 69 insertions(+), 75 deletions(-) create mode 100644 src/config/profiles/index.js diff --git a/src/config/index.js b/src/config/index.js index 3ece84ef9..36621fd39 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -2,14 +2,9 @@ const callbackify = require('callbackify') -module.exports = (send, config) => { - return { - get: callbackify.variadic(require('./get')(config)), - set: require('./set')(send), - replace: require('./replace')(send), - profiles: { - apply: require('./profiles/apply')(config), - list: require('./profiles/list')(config) - } - } -} +module.exports = config => ({ + get: callbackify.variadic(require('./get')(config)), + set: callbackify.variadic(require('./set')(config)), + replace: callbackify.variadic(require('./replace')(config)), + profiles: require('./profiles')(config) +}) diff --git a/src/config/profiles/apply.js b/src/config/profiles/apply.js index e4b05560b..deac884fe 100644 --- a/src/config/profiles/apply.js +++ b/src/config/profiles/apply.js @@ -1,27 +1,24 @@ 'use strict' -const callbackify = require('callbackify') const configure = require('../../lib/configure') module.exports = configure(({ ky }) => { - return callbackify.variadic(async (profile, options) => { + return async (profile, options) => { options = options || {} + const searchParams = new URLSearchParams(options.searchParams) + searchParams.set('arg', profile) + if (options.dryRun != null) searchParams.set('dry-run', options.dryRun) + const res = await ky.post('config/profile/apply', { timeout: options.timeout, signal: options.signal, headers: options.headers, - searchParams: { - arg: profile, - // can only pass strings or numbers as values https://github.com/sindresorhus/ky/issues/182 - 'dry-run': options.dryRun ? 'true' : 'false' - } - }) - - const parsed = await res.json() + searchParams + }).json() return { - original: parsed.OldCfg, updated: parsed.NewCfg + original: res.OldCfg, updated: res.NewCfg } - }) + } }) diff --git a/src/config/profiles/index.js b/src/config/profiles/index.js new file mode 100644 index 000000000..57cd1ad7f --- /dev/null +++ b/src/config/profiles/index.js @@ -0,0 +1,8 @@ +'use strict' + +const callbackify = require('callbackify') + +module.exports = config => ({ + apply: callbackify.variadic(require('./apply')(config)), + list: callbackify.variadic(require('./list')(config)) +}) diff --git a/src/config/profiles/list.js b/src/config/profiles/list.js index dbfa579cf..1e3f44308 100644 --- a/src/config/profiles/list.js +++ b/src/config/profiles/list.js @@ -1,22 +1,19 @@ 'use strict' -const callbackify = require('callbackify') const configure = require('../../lib/configure') const toCamel = require('../../lib/object-to-camel') module.exports = configure(({ ky }) => { - return callbackify.variadic(async (options) => { + return async (options) => { options = options || {} const res = await ky.get('config/profile/list', { timeout: options.timeout, signal: options.signal, - headers: options.headers - }) + headers: options.headers, + searchParams: options.searchParams + }).json() - const parsed = await res.json() - - return parsed - .map(profile => toCamel(profile)) - }) + return res.map(profile => toCamel(profile)) + } }) diff --git a/src/config/replace.js b/src/config/replace.js index 971c61ee0..ccca80beb 100644 --- a/src/config/replace.js +++ b/src/config/replace.js @@ -1,25 +1,21 @@ 'use strict' -const { Readable } = require('readable-stream') -const promisify = require('promisify-es6') -const SendOneFile = require('../utils/send-one-file') +const { Buffer } = require('buffer') +const configure = require('../lib/configure') +const toFormData = require('../lib/buffer-to-form-data') -function toStream (input) { - return new Readable({ - read () { - this.push(input) - this.push(null) - } - }) -} +module.exports = configure(({ ky }) => { + return async (config, options) => { + options = options || {} -module.exports = (send) => { - const sendOneFile = SendOneFile(send, 'config/replace') - return promisify((config, callback) => { - if (typeof config === 'object') { - config = toStream(Buffer.from(JSON.stringify(config))) - } + const res = await ky.post('config/replace', { + timeout: options.timeout, + signal: options.signal, + headers: options.headers, + searchParams: options.searchParams, + body: toFormData(Buffer.from(JSON.stringify(config))) + }).text() - sendOneFile(config, {}, callback) - }) -} + return res + } +}) diff --git a/src/config/set.js b/src/config/set.js index 0fa933565..933c8be89 100644 --- a/src/config/set.js +++ b/src/config/set.js @@ -1,35 +1,36 @@ 'use strict' -const promisify = require('promisify-es6') +const configure = require('../lib/configure') +const toCamel = require('../lib/object-to-camel') + +module.exports = configure(({ ky }) => { + return async (key, value, options) => { + options = options || {} -module.exports = (send) => { - return promisify((key, value, opts, callback) => { - if (typeof opts === 'function') { - callback = opts - opts = {} - } if (typeof key !== 'string') { - return callback(new Error('Invalid key type')) + throw new Error('invalid key') } - if (value === undefined || Buffer.isBuffer(value)) { - return callback(new Error('Invalid value type')) - } + const searchParams = new URLSearchParams(options.searchParams) if (typeof value === 'boolean') { + searchParams.set('bool', true) value = value.toString() - opts = { bool: true } } else if (typeof value !== 'string') { + searchParams.set('json', true) value = JSON.stringify(value) - opts = { json: true } } - send({ - path: 'config', - args: [key, value], - qs: opts, - files: undefined, - buffer: true - }, callback) - }) -} + searchParams.set('arg', key) + searchParams.append('arg', value) + + const res = await ky.post('config', { + timeout: options.timeout, + signal: options.signal, + headers: options.headers, + searchParams + }).json() + + return toCamel(res) + } +}) diff --git a/src/utils/load-commands.js b/src/utils/load-commands.js index 242b52463..c3fdf2edd 100644 --- a/src/utils/load-commands.js +++ b/src/utils/load-commands.js @@ -91,6 +91,7 @@ function requireCommands (send, config) { getEndpointConfig: require('../get-endpoint-config')(config), bitswap: require('../bitswap')(config), block: require('../block')(config), + config: require('../config')(config), dag: require('../dag')(config) } @@ -122,7 +123,6 @@ function requireCommands (send, config) { // Miscellaneous commands: require('../commands'), - config: require('../config'), diag: require('../diag'), id: require('../id'), key: require('../key'), From 113ffec75d908694b52bb96933f1d6ed6e3883d8 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 15:59:33 +0000 Subject: [PATCH 3/8] fix: error responses --- src/lib/error-handler.js | 12 +++++++++--- test/lib.error-handler.spec.js | 3 ++- test/request-api.spec.js | 7 +++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/lib/error-handler.js b/src/lib/error-handler.js index 032a6acc8..3aae8a41c 100644 --- a/src/lib/error-handler.js +++ b/src/lib/error-handler.js @@ -35,9 +35,15 @@ module.exports = async function errorHandler (input, options, response) { } catch (err) { log('Failed to parse error response', err) // Failed to extract/parse error message from response - throw new HTTPError(response) + msg = err.message } - if (!msg) throw new HTTPError(response) - throw Object.assign(new Error(msg), { status: response.status }) + const error = new HTTPError(response) + + // If we managed to extract a message from the response, use it + if (msg) { + error.message = msg + } + + throw error } diff --git a/test/lib.error-handler.spec.js b/test/lib.error-handler.spec.js index bdc0fc8a3..c39e3040d 100644 --- a/test/lib.error-handler.spec.js +++ b/test/lib.error-handler.spec.js @@ -21,8 +21,9 @@ describe('lib/error-handler', () => { const err = await throwsAsync(errorHandler(null, null, res)) + expect(err instanceof HTTPError).to.be.true() expect(err.message).to.eql('boom') - expect(err.status).to.eql(500) + expect(err.response.status).to.eql(500) }) it('should gracefully fail on parse json', async () => { diff --git a/test/request-api.spec.js b/test/request-api.spec.js index 1b0858786..fea54e1b2 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -81,7 +81,7 @@ describe('error handling', () => { server.listen(6001, () => { ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { expect(err).to.exist() - expect(err.statusCode).to.equal(403) + expect(err.response.status).to.equal(403) expect(err.message).to.equal('ipfs method not allowed') server.close(done) }) @@ -105,9 +105,8 @@ describe('error handling', () => { server.listen(6001, () => { ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { expect(err).to.exist() - expect(err.statusCode).to.equal(400) + expect(err.response.status).to.equal(400) expect(err.message).to.equal('client error') - expect(err.code).to.equal(1) server.close(done) }) }) @@ -130,7 +129,7 @@ describe('error handling', () => { server.listen(6001, () => { ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { expect(err).to.exist() - expect(err.message).to.include('Invalid JSON') + expect(err.message).to.include('invalid json') server.close(done) }) }) From 56a7b2d8a064e6f34f2e5bc498be860884074ef3 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 16:52:02 +0000 Subject: [PATCH 4/8] fix: yay asserting on error message --- test/request-api.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/request-api.spec.js b/test/request-api.spec.js index fea54e1b2..990f9d434 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -129,7 +129,7 @@ describe('error handling', () => { server.listen(6001, () => { ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { expect(err).to.exist() - expect(err.message).to.include('invalid json') + expect(err.message).to.match(/invalid json/i) server.close(done) }) }) From ea74f9d66cb64b92d72a55d3ab227acac20fa0b0 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 19:15:38 +0000 Subject: [PATCH 5/8] chore: add logging --- test/request-api.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/request-api.spec.js b/test/request-api.spec.js index 990f9d434..6b8391ad9 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -127,7 +127,9 @@ describe('error handling', () => { }) server.listen(6001, () => { + console.log('requesting...') ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { + console.log(err) expect(err).to.exist() expect(err.message).to.match(/invalid json/i) server.close(done) From cdc97add65ded99fd0cf6e163362f0d74c5254af Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 20:37:36 +0000 Subject: [PATCH 6/8] chore: more log --- test/request-api.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/request-api.spec.js b/test/request-api.spec.js index 6b8391ad9..2f4c1c747 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -112,7 +112,7 @@ describe('error handling', () => { }) }) - it('should handle JSON error response with invalid JSON', function (done) { + it.only('should handle JSON error response with invalid JSON', function (done) { if (!isNode) return this.skip() const server = require('http').createServer((req, res) => { @@ -130,8 +130,10 @@ describe('error handling', () => { console.log('requesting...') ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { console.log(err) + console.log('err.message=', err.message) expect(err).to.exist() expect(err.message).to.match(/invalid json/i) + console.log('closing...') server.close(done) }) }) From 67994bd0dc5f22cded8cedc7400d44519b9a1235 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 13 Nov 2019 21:14:10 +0000 Subject: [PATCH 7/8] fix: error message assertion --- test/request-api.spec.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/request-api.spec.js b/test/request-api.spec.js index 2f4c1c747..c213193bb 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -112,7 +112,7 @@ describe('error handling', () => { }) }) - it.only('should handle JSON error response with invalid JSON', function (done) { + it('should handle JSON error response with invalid JSON', function (done) { if (!isNode) return this.skip() const server = require('http').createServer((req, res) => { @@ -127,13 +127,9 @@ describe('error handling', () => { }) server.listen(6001, () => { - console.log('requesting...') ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => { - console.log(err) - console.log('err.message=', err.message) expect(err).to.exist() - expect(err.message).to.match(/invalid json/i) - console.log('closing...') + expect(err.message).to.include('Unexpected token M in JSON at position 2') server.close(done) }) }) From f4d5128beab4ec57e4cf5c1aee12aebefee203f0 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 14 Nov 2019 12:13:01 +0000 Subject: [PATCH 8/8] fix: revert error message change --- src/config/set.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/set.js b/src/config/set.js index 933c8be89..f9e3309f2 100644 --- a/src/config/set.js +++ b/src/config/set.js @@ -8,7 +8,7 @@ module.exports = configure(({ ky }) => { options = options || {} if (typeof key !== 'string') { - throw new Error('invalid key') + throw new Error('Invalid key type') } const searchParams = new URLSearchParams(options.searchParams)