Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Agents 'n' timeouts #13

Merged
merged 4 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion API.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@ The following params are accepted:

| Name | Type | Default | Description |
| ---- | ---- | ------- | ----------- |
| `agent` | [`Agent`](https://devdocs.io/node/http#http_class_http_agent) | | optional shared agent to [manage connection persistence](https://devdocs.io/node/http#http_class_http_agent) |
| `body` | `Any` | | data to serialize and send as the request body |
| `deserialize` | `Function` | [`JSON.stringify`](http://devdocs.io/javascript/global_objects/json/stringify) | function with which to deserialize the response body |
| `headers` | `Object` | `{}` | headers to include on the request |
| `json` | `Boolean` | `true` | if `true`, assumes [json-formatted](#json-by-default) request and response |
| `jwt` | `String` | | [json web token](https://jwt.io/) to include in the `Authorization` header |
| `method` | `String` | `GET` | must be a valid `http` request method |
| `query` | `Object` | | data to serialize and append to the url as a query string |
| `serialize` | `Function` | [`JSON.parse`](http://devdocs.io/javascript/global_objects/json/parse) | function with which to serialize the request body |
| `stream` | `Boolean` | `false` | if `true`, the response `body` will be a [`stream.Readable`](http://devdocs.io/node/stream#stream_class_stream_readable) |
| `timeout` | `Number` | `0` (never) | milliseconds before the [request times out](https://devdocs.io/node/http#http_request_settimeout_timeout_callback) |
| `url` | `String` | | **required:** the `url` of the request |
| `query` | `Object` | | data to serialize and append to the url as a query string |

#### `Response` object

Expand Down
4 changes: 3 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@ const { validate } = require('@articulate/funky')
const gimme = require('./lib/gimme')

const schema = Joi.object({
agent: Joi.any(),
body: Joi.any(),
data: Joi.any(), // deprecated
deserialize: Joi.func(),
headers: Joi.object(),
json: Joi.boolean(),
jwt: Joi.string(),
method: Joi.string().valid(http.METHODS),
query: Joi.object(),
serialize: Joi.func(),
stream: Joi.boolean(),
timeout: Joi.number().integer(),
url: Joi.string().required(),
query: Joi.object(),
})

module.exports = composeP(gimme, validate(schema))
10 changes: 8 additions & 2 deletions lib/gimme.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { Readable } = require('stream')

const {
assoc, assocPath, evolve, flip, identity, is,
merge, nthArg, pick, pipe, replace, tryCatch
merge, nthArg, once, pick, pipe, replace, tryCatch
} = require('ramda')

const { name } = require('../package')
Expand All @@ -24,21 +24,25 @@ const clean = pipe(

const parseJSON = tryCatch(JSON.parse, nthArg(1))

const warnOnce = once(console.warn)

const gimme = opts => {
const {
data,
json = true
} = opts

if (data) console.warn(chalk.yellow(`[${name}] The 'data' option is deprecated in favor of 'body'.`))
if (data) warnOnce(chalk.yellow(`[${name}] The 'data' option is deprecated in favor of 'body'.`))

const {
agent,
body = data,
deserialize = json ? parseJSON : identity,
jwt,
method = 'GET',
serialize = json ? JSON.stringify : identity,
stream = false,
timeout = 0,
} = opts

const headers = merge({}, opts.headers)
Expand All @@ -59,6 +63,7 @@ const gimme = opts => {
const path = pathname + search

const params = { auth, headers, hostname, method, path, port, protocol }
if (agent) params.agent = agent

return new Promise((resolve, reject) => {
const respond = res => {
Expand Down Expand Up @@ -90,6 +95,7 @@ const gimme = opts => {
const req = requester.request(params, respond)

req.on('error', reject)
req.setTimeout(timeout, req.abort.bind(req))
mgreystone marked this conversation as resolved.
Show resolved Hide resolved

if (method !== 'GET' && body) {
if (is(Readable, body)) {
Expand Down
1 change: 1 addition & 0 deletions test/00-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ beforeEach(() => {
nock(url).get('/error').query(true).reply(400)
nock(url).get('/no-length').query(true).reply(200, { foo: 'bar' })
nock(url).get('/non-json-error').query(true).reply(400, 'string error')
nock(url).get('/timeout').query(true).socketDelay(1500).reply(200)
})

afterEach(() =>
Expand Down
30 changes: 30 additions & 0 deletions test/agent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const { Agent } = require('http')
const { expect } = require('chai')
const property = require('prop-factory')

const gimme = require('..')
const { url } = require('./00-setup')

describe('agent', () => {
let agent
const res = property()

beforeEach(() => {
agent = new Agent({ keepAlive: true })
res(undefined)
})

afterEach(() => {
agent.destroy()
})

describe('when supplied', () => {
beforeEach(() =>
gimme({ url, agent }).then(res)
)

it('is allowed', () => {
expect(res().statusCode).to.equal(200)
})
})
})
34 changes: 34 additions & 0 deletions test/timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const { expect } = require('chai')
const property = require('prop-factory')

const gimme = require('..')
const { url } = require('./00-setup')

describe('timeout', () => {
const res = property()

beforeEach(() => {
res(undefined)
})

describe('when not supplied', () => {
beforeEach(() =>
gimme({ url: `${url}/timeout` }).then(res)
)

it('defaults to 0 (no timeout)', () => {
expect(res().statusCode).to.equal(200)
})
})

describe('when supplied', () => {
beforeEach(() =>
gimme({ url: `${url}/timeout`, timeout: 250 }).catch(res)
)

it('applies the timeout to the socket', () => {
expect(res()).to.be.an('Error')
expect(res().code).to.equal('ECONNRESET')
})
})
})