Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

refactor: convert config API to async await #1155

Merged
merged 9 commits into from
Nov 14, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 21 additions & 33 deletions src/config/get.js
Original file line number Diff line number Diff line change
@@ -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
}
})
19 changes: 8 additions & 11 deletions src/config/index.js
Original file line number Diff line number Diff line change
@@ -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)
})
21 changes: 9 additions & 12 deletions src/config/profiles/apply.js
Original file line number Diff line number Diff line change
@@ -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
}
})
}
})
8 changes: 8 additions & 0 deletions src/config/profiles/index.js
Original file line number Diff line number Diff line change
@@ -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))
})
15 changes: 6 additions & 9 deletions src/config/profiles/list.js
Original file line number Diff line number Diff line change
@@ -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))
}
})
36 changes: 16 additions & 20 deletions src/config/replace.js
Original file line number Diff line number Diff line change
@@ -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
}
})
45 changes: 23 additions & 22 deletions src/config/set.js
Original file line number Diff line number Diff line change
@@ -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')
alanshaw marked this conversation as resolved.
Show resolved Hide resolved
}

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)
}
})
12 changes: 9 additions & 3 deletions src/lib/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion src/utils/load-commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -122,7 +123,6 @@ function requireCommands (send, config) {

// Miscellaneous
commands: require('../commands'),
config: require('../config'),
diag: require('../diag'),
id: require('../id'),
key: require('../key'),
Expand Down
3 changes: 2 additions & 1 deletion test/lib.error-handler.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking change? I guess errors aren't really part of the spec other than 'you will get an error'.

Copy link
Contributor Author

@alanshaw alanshaw Nov 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is breaking. I've updated the PR description with the info and will include it when merging.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. One small thing - the description says err.status has changed but it's err.statusCode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this test was looking for .status but others are looking for .statusCode...I've added both to the breaking change message.

})

it('should gracefully fail on parse json', async () => {
Expand Down
7 changes: 3 additions & 4 deletions test/request-api.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand All @@ -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)
})
})
Expand All @@ -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)
})
})
Expand Down