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 <alan.shaw@protocol.ai>

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
+  }
+})

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)
+})

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
     }
-  })
+  }
 })

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))
+})

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))
+  }
 })

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
+  }
+})

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)
+  }
+})

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
 }

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'),

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 () => {

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)
       })
     })