Skip to content

Commit

Permalink
Migrate request to got (part 1 of many) (#6160)
Browse files Browse the repository at this point in the history
* install new packages

* migrate request to got

* update dynamic json test

This is a behavioural difference between request and got

request will send the request, then we'll get a
`400 Bad Request` back and re-throw at as invalid

got will pick up that the URL is invalid and throw
`RequestError: URI malformed` before attempting to send it
which we'll re-throw as inaccessible

* fix OPM service

* fix wordpress querystring

Got doesn't natively support assmebling a querystring
from nested objects because it uses node's URLSearchParams
internally. Use qs and pass qs a string.

Wordpress is the only service that needs this,
so we could build the string manually in this case
if we don't want to take qs as a prod dependency.
It is mostly hard-coded values anyway.

* fix wercker

got overwrites any ?foo=bar in the URL string if
searchParams is also passed whereas request appends
see https://github.com/sindresorhus/got#url

* fix keybase

* add tests for got wrapper

* bootstrap global agent in server start
  • Loading branch information
chris48s committed Mar 9, 2021
1 parent b2f224c commit 2359eb2
Show file tree
Hide file tree
Showing 15 changed files with 381 additions and 96 deletions.
5 changes: 4 additions & 1 deletion core/base-service/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const {
Deprecated,
} = require('./errors')
const { validateExample, transformExample } = require('./examples')
const { fetchFactory } = require('./got')
const {
makeFullUrl,
assertValidRoute,
Expand Down Expand Up @@ -430,6 +431,8 @@ class BaseService {
ServiceClass: this,
})

const fetcher = fetchFactory(fetchLimitBytes)

camp.route(
regex,
handleRequest(cacheHeaderConfig, {
Expand All @@ -440,7 +443,7 @@ class BaseService {
const namedParams = namedParamsForMatch(captureNames, match, this)
const serviceData = await this.invoke(
{
sendAndCacheRequest: request.asPromise,
sendAndCacheRequest: fetcher,
sendAndCacheRequestWithCallbacks: request,
githubApiProvider,
metricHelper,
Expand Down
97 changes: 97 additions & 0 deletions core/base-service/got.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
'use strict'

const got = require('got')
const { Inaccessible, InvalidResponse } = require('./errors')

function requestOptions2GotOptions(options) {
const requestOptions = Object.assign({}, options)
const gotOptions = {}
const interchangableOptions = ['body', 'form', 'headers', 'method', 'url']

interchangableOptions.forEach(function (opt) {
if (opt in requestOptions) {
gotOptions[opt] = requestOptions[opt]
delete requestOptions[opt]
}
})

if ('qs' in requestOptions) {
gotOptions.searchParams = requestOptions.qs
delete requestOptions.qs
}

if ('gzip' in requestOptions) {
gotOptions.decompress = requestOptions.gzip
delete requestOptions.gzip
}

if ('strictSSL' in requestOptions) {
gotOptions.https = {
rejectUnauthorized: requestOptions.strictSSL,
}
delete requestOptions.strictSSL
}

if ('auth' in requestOptions) {
gotOptions.username = requestOptions.auth.user
gotOptions.password = requestOptions.auth.pass
delete requestOptions.auth
}

if (Object.keys(requestOptions).length > 0) {
throw new Error(`Found unrecognised options ${Object.keys(requestOptions)}`)
}

return gotOptions
}

async function sendRequest(gotWrapper, url, options) {
const gotOptions = requestOptions2GotOptions(options)
gotOptions.throwHttpErrors = false
gotOptions.retry = 0
try {
const resp = await gotWrapper(url, gotOptions)
return { res: resp, buffer: resp.body }
} catch (err) {
if (err instanceof got.CancelError) {
throw new InvalidResponse({
underlyingError: new Error('Maximum response size exceeded'),
})
}
throw new Inaccessible({ underlyingError: err })
}
}

function fetchFactory(fetchLimitBytes) {
const gotWithLimit = got.extend({
handlers: [
(options, next) => {
const promiseOrStream = next(options)
promiseOrStream.on('downloadProgress', progress => {
if (
progress.transferred > fetchLimitBytes &&
// just accept the file if we've already finished downloading
// the entire file before we went over the limit
progress.percent !== 1
) {
/*
TODO: we should be able to pass cancel() a message
https://github.com/sindresorhus/got/blob/main/documentation/advanced-creation.md#examples
but by the time we catch it, err.message is just "Promise was canceled"
*/
promiseOrStream.cancel('Maximum response size exceeded')
}
})

return promiseOrStream
},
],
})

return sendRequest.bind(sendRequest, gotWithLimit)
}

module.exports = {
requestOptions2GotOptions,
fetchFactory,
}
89 changes: 89 additions & 0 deletions core/base-service/got.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
'use strict'

const { expect } = require('chai')
const nock = require('nock')
const { requestOptions2GotOptions, fetchFactory } = require('./got')
const { Inaccessible, InvalidResponse } = require('./errors')

describe('requestOptions2GotOptions function', function () {
it('translates valid options', function () {
expect(
requestOptions2GotOptions({
body: 'body',
form: 'form',
headers: 'headers',
method: 'method',
url: 'url',
qs: 'qs',
gzip: 'gzip',
strictSSL: 'strictSSL',
auth: { user: 'user', pass: 'pass' },
})
).to.deep.equal({
body: 'body',
form: 'form',
headers: 'headers',
method: 'method',
url: 'url',
searchParams: 'qs',
decompress: 'gzip',
https: { rejectUnauthorized: 'strictSSL' },
username: 'user',
password: 'pass',
})
})

it('throws if unrecognised options are found', function () {
expect(() =>
requestOptions2GotOptions({ body: 'body', foobar: 'foobar' })
).to.throw(Error, 'Found unrecognised options foobar')
})
})

describe('got wrapper', function () {
it('should not throw an error if the response <= fetchLimitBytes', async function () {
nock('https://www.google.com')
.get('/foo/bar')
.once()
.reply(200, 'x'.repeat(100))
const sendRequest = fetchFactory(100)
const { res } = await sendRequest('https://www.google.com/foo/bar')
expect(res.statusCode).to.equal(200)
})

it('should throw an InvalidResponse error if the response is > fetchLimitBytes', async function () {
nock('https://www.google.com')
.get('/foo/bar')
.once()
.reply(200, 'x'.repeat(101))
const sendRequest = fetchFactory(100)
return expect(
sendRequest('https://www.google.com/foo/bar')
).to.be.rejectedWith(InvalidResponse, 'Maximum response size exceeded')
})

it('should throw an Inaccessible error if the request throws a (non-HTTP) error', async function () {
nock('https://www.google.com').get('/foo/bar').replyWithError('oh no')
const sendRequest = fetchFactory(1024)
return expect(
sendRequest('https://www.google.com/foo/bar')
).to.be.rejectedWith(Inaccessible, 'oh no')
})

it('should throw an Inaccessible error if the host can not be accessed', async function () {
this.timeout(5000)
nock.disableNetConnect()
const sendRequest = fetchFactory(1024)
return expect(
sendRequest('https://www.google.com/foo/bar')
).to.be.rejectedWith(
Inaccessible,
'Nock: Disallowed net connect for "www.google.com:443/foo/bar"'
)
})

afterEach(function () {
nock.cleanAll()
nock.enableNetConnect()
})
})
22 changes: 22 additions & 0 deletions core/server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const path = require('path')
const url = require('url')
const { bootstrap } = require('global-agent')
const { URL } = url
const cloudflareMiddleware = require('cloudflare-middleware')
const bytes = require('bytes')
Expand Down Expand Up @@ -421,6 +422,25 @@ class Server {
)
}

bootstrapAgent() {
/*
Bootstrap global agent.
This allows self-hosting users to configure a proxy with
HTTP_PROXY, HTTPS_PROXY, NO_PROXY variables
*/
if (!('GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE' in process.env)) {
process.env.GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE = ''
}

const proxyPrefix = process.env.GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE
const HTTP_PROXY = process.env[`${proxyPrefix}HTTP_PROXY`] || null
const HTTPS_PROXY = process.env[`${proxyPrefix}HTTPS_PROXY`] || null

if (HTTP_PROXY || HTTPS_PROXY) {
bootstrap()
}
}

/**
* Start the HTTP server:
* Bootstrap Scoutcamp,
Expand All @@ -436,6 +456,8 @@ class Server {
requireCloudflare,
} = this.config.public

this.bootstrapAgent()

log(`Server is starting up: ${this.baseUrl}`)

const camp = (this.camp = Camp.create({
Expand Down
Loading

0 comments on commit 2359eb2

Please sign in to comment.