From 621973c99916b4829e59aaf0fb67101284facfae Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 14 Nov 2019 13:48:17 +0000 Subject: [PATCH] refactor: convert config API to async await (#1155) BREAKING CHANGE: Errors returned from request failures are now all [`HTTPError`](https://github.com/sindresorhus/ky/blob/c0d9d2bb07e4c122a08f019b39e9c55a4c9324f3/index.js#L117-L123)s which carry a `response` property. This is a [`Response`](https://developer.mozilla.org/en-US/docs/Web/API/Response) that can be used to inspect _all_ information relating to the HTTP response. This means that the `err.status` or `err.statusCode` property should now be accessed via `err.response.status`. License: MIT Signed-off-by: Alan Shaw --- src/config/get.js | 54 +++++++++++++--------------------- src/config/index.js | 19 +++++------- 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/lib/error-handler.js | 12 ++++++-- src/utils/load-commands.js | 2 +- test/lib.error-handler.spec.js | 3 +- test/request-api.spec.js | 7 ++--- 11 files changed, 106 insertions(+), 116 deletions(-) create mode 100644 src/config/profiles/index.js 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..36621fd39 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -1,13 +1,10 @@ 'use strict' -module.exports = (send, config) => { - return { - get: require('./get')(send), - set: require('./set')(send), - replace: require('./replace')(send), - profiles: { - apply: require('./profiles/apply')(config), - list: require('./profiles/list')(config) - } - } -} +const callbackify = require('callbackify') + +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..f9e3309f2 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 type') } - 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/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/src/utils/load-commands.js b/src/utils/load-commands.js index 9e9db2cf5..34a7591e5 100644 --- a/src/utils/load-commands.js +++ b/src/utils/load-commands.js @@ -92,6 +92,7 @@ function requireCommands (send, config) { bitswap: require('../bitswap')(config), block: require('../block')(config), bootstrap: require('../bootstrap')(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'), 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..c213193bb 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('Unexpected token M in JSON at position 2') server.close(done) }) })