From 3f6a81518873395eabe1f4760835d46cc03ae000 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Thu, 12 Aug 2021 15:31:34 -0400 Subject: [PATCH 01/10] Add an HttpClient interface and NodeHttpClient implementation. --- lib/StripeResource.js | 318 +++++++++++++++----------------- lib/net/HttpClient.js | 60 ++++++ lib/net/NodeHttpClient.js | 122 ++++++++++++ lib/stripe.js | 9 +- test/StripeResource.spec.js | 176 +++++++++++++++++- test/net/NodeHttpClient.spec.js | 215 +++++++++++++++++++++ types/2020-08-27/index.d.ts | 1 + types/lib.d.ts | 10 +- types/net/net.d.ts | 70 +++++++ types/test/typescriptTest.ts | 23 +++ 10 files changed, 828 insertions(+), 176 deletions(-) create mode 100644 lib/net/HttpClient.js create mode 100644 lib/net/NodeHttpClient.js create mode 100644 test/net/NodeHttpClient.spec.js create mode 100644 types/net/net.d.ts diff --git a/lib/StripeResource.js b/lib/StripeResource.js index 472fca6749..e16bd90120 100644 --- a/lib/StripeResource.js +++ b/lib/StripeResource.js @@ -1,7 +1,5 @@ 'use strict'; -const http = require('http'); -const https = require('https'); const path = require('path'); const utils = require('./utils'); @@ -14,8 +12,7 @@ const { StripeAPIError, } = require('./Error'); -const defaultHttpAgent = new http.Agent({keepAlive: true}); -const defaultHttpsAgent = new https.Agent({keepAlive: true}); +const HttpClient = require('./net/HttpClient'); // Provide extension mechanism for Stripe Resource Sub-Classes StripeResource.extend = utils.protoExtend; @@ -104,35 +101,39 @@ StripeResource.prototype = { }; }, - _addHeadersDirectlyToResponse(res, headers) { + _addHeadersDirectlyToObject(obj, headers) { // For convenience, make some headers easily accessible on // lastResponse. // NOTE: Stripe responds with lowercase header names/keys. - res.requestId = headers['request-id']; - res.stripeAccount = res.stripeAccount || headers['stripe-account']; - res.apiVersion = res.apiVersion || headers['stripe-version']; - res.idempotencyKey = res.idempotencyKey || headers['idempotency-key']; + obj.requestId = headers['request-id']; + obj.stripeAccount = obj.stripeAccount || headers['stripe-account']; + obj.apiVersion = obj.apiVersion || headers['stripe-version']; + obj.idempotencyKey = obj.idempotencyKey || headers['idempotency-key']; }, - _makeResponseEvent(req, res, headers) { + _makeResponseEvent(requestEvent, statusCode, headers) { const requestEndTime = Date.now(); - const requestDurationMs = requestEndTime - req._requestStart; + const requestDurationMs = requestEndTime - requestEvent.request_start_time; return utils.removeNullish({ api_version: headers['stripe-version'], account: headers['stripe-account'], idempotency_key: headers['idempotency-key'], - method: req._requestEvent.method, - path: req._requestEvent.path, - status: res.statusCode, - request_id: res.requestId, + method: requestEvent.method, + path: requestEvent.path, + status: statusCode, + request_id: this._getRequestId(headers), elapsed: requestDurationMs, - request_start_time: req._requestStart, + request_start_time: requestEvent.request_start_time, request_end_time: requestEndTime, }); }, + _getRequestId(headers) { + return headers['request-id']; + }, + /** * Used by methods with spec.streaming === true. For these methods, we do not * buffer successful responses into memory or do parse them into stripe @@ -143,17 +144,31 @@ StripeResource.prototype = { * still be buffered/parsed and handled by _jsonResponseHandler -- see * makeRequest) */ - _streamingResponseHandler(req, callback) { + _streamingResponseHandler(requestEvent, callback) { return (res) => { - const headers = res.headers || {}; - this._addHeadersDirectlyToResponse(res, headers); + const headers = res.getHeaders(); - res.once('end', () => { - const responseEvent = this._makeResponseEvent(req, res, headers); + const streamCompleteCallback = () => { + const responseEvent = this._makeResponseEvent( + requestEvent, + res.getStatusCode(), + headers + ); this._stripe._emitter.emit('response', responseEvent); - this._recordRequestMetrics(res.requestId, responseEvent.elapsed); - }); - return callback(null, res); + this._recordRequestMetrics( + this._getRequestId(headers), + responseEvent.elapsed + ); + }; + + const stream = res.toStream(streamCompleteCallback); + + // This is here for backwards compatibility, as the stream is a raw + // HTTP response in Node and the legacy behavior was to mutate this + // response. + this._addHeadersDirectlyToObject(stream, headers); + + return callback(null, stream); }; }, @@ -162,73 +177,79 @@ StripeResource.prototype = { * parses the JSON and returns it (i.e. passes it to the callback) if there * is no "error" field. Otherwise constructs/passes an appropriate Error. */ - _jsonResponseHandler(req, callback) { + _jsonResponseHandler(requestEvent, callback) { return (res) => { - let response = ''; - - res.setEncoding('utf8'); - res.on('data', (chunk) => { - response += chunk; - }); - res.once('end', () => { - const headers = res.headers || {}; - this._addHeadersDirectlyToResponse(res, headers); - const responseEvent = this._makeResponseEvent(req, res, headers); - this._stripe._emitter.emit('response', responseEvent); - - try { - response = JSON.parse(response); - - if (response.error) { - let err; - - // Convert OAuth error responses into a standard format - // so that the rest of the error logic can be shared - if (typeof response.error === 'string') { - response.error = { - type: response.error, - message: response.error_description, - }; + const headers = res.getHeaders(); + const requestId = this._getRequestId(headers); + const statusCode = res.getStatusCode(); + + const responseEvent = this._makeResponseEvent( + requestEvent, + statusCode, + headers + ); + this._stripe._emitter.emit('response', responseEvent); + + res + .toJSON() + .then( + (jsonResponse) => { + if (jsonResponse.error) { + let err; + + // Convert OAuth error responses into a standard format + // so that the rest of the error logic can be shared + if (typeof jsonResponse.error === 'string') { + jsonResponse.error = { + type: jsonResponse.error, + message: jsonResponse.error_description, + }; + } + + jsonResponse.error.headers = headers; + jsonResponse.error.statusCode = statusCode; + jsonResponse.error.requestId = requestId; + + if (statusCode === 401) { + err = new StripeAuthenticationError(jsonResponse.error); + } else if (statusCode === 403) { + err = new StripePermissionError(jsonResponse.error); + } else if (statusCode === 429) { + err = new StripeRateLimitError(jsonResponse.error); + } else { + err = StripeError.generate(jsonResponse.error); + } + + throw err; } - response.error.headers = headers; - response.error.statusCode = res.statusCode; - response.error.requestId = res.requestId; - - if (res.statusCode === 401) { - err = new StripeAuthenticationError(response.error); - } else if (res.statusCode === 403) { - err = new StripePermissionError(response.error); - } else if (res.statusCode === 429) { - err = new StripeRateLimitError(response.error); - } else { - err = StripeError.generate(response.error); - } - return callback.call(this, err, null); - } - } catch (e) { - return callback.call( - this, - new StripeAPIError({ + return jsonResponse; + }, + (e) => { + throw new StripeAPIError({ message: 'Invalid JSON received from the Stripe API', - response, exception: e, requestId: headers['request-id'], - }), - null - ); - } - - this._recordRequestMetrics(res.requestId, responseEvent.elapsed); - - // Expose res object - Object.defineProperty(response, 'lastResponse', { - enumerable: false, - writable: false, - value: res, - }); - callback.call(this, null, response); - }); + }); + } + ) + .then( + (jsonResponse) => { + this._recordRequestMetrics(requestId, responseEvent.elapsed); + + // Expose raw response object. + const rawResponse = res.getRawResponse(); + this._addHeadersDirectlyToObject(rawResponse, headers); + Object.defineProperty(jsonResponse, 'lastResponse', { + enumerable: false, + writable: false, + value: rawResponse, + }); + + callback.call(this, null, jsonResponse); + }, + (e) => callback.call(this, e, null) + ); }; }, @@ -265,15 +286,15 @@ StripeResource.prototype = { // The API may ask us not to retry (e.g., if doing so would be a no-op) // or advise us to retry (e.g., in cases of lock timeouts); we defer to that. - if (res.headers && res.headers['stripe-should-retry'] === 'false') { + if (res.getHeaders()['stripe-should-retry'] === 'false') { return false; } - if (res.headers && res.headers['stripe-should-retry'] === 'true') { + if (res.getHeaders()['stripe-should-retry'] === 'true') { return true; } // Retry on conflict errors. - if (res.statusCode === 409) { + if (res.getStatusCode() === 409) { return true; } @@ -282,7 +303,7 @@ StripeResource.prototype = { // Note that we expect the stripe-should-retry header to be false // in most cases when a 500 is returned, since our idempotency framework // would typically replay it anyway. - if (res.statusCode >= 500) { + if (res.getStatusCode() >= 500) { return true; } @@ -434,22 +455,18 @@ StripeResource.prototype = { ? options.settings.timeout : this._stripe.getApiField('timeout'); - const isInsecureConnection = - this._stripe.getApiField('protocol') === 'http'; - let agent = this._stripe.getApiField('agent'); - if (agent == null) { - agent = isInsecureConnection ? defaultHttpAgent : defaultHttpsAgent; - } - - const req = (isInsecureConnection ? http : https).request({ - host: host || this._stripe.getApiField('host'), - port: this._stripe.getApiField('port'), - path, - method, - agent, - headers, - ciphers: 'DEFAULT:!aNULL:!eNULL:!LOW:!EXPORT:!SSLv2:!MD5', - }); + const req = this._stripe + .getApiField('httpClient') + .makeRequest( + host || this._stripe.getApiField('host'), + this._stripe.getApiField('port'), + path, + method, + headers, + requestData, + this._stripe.getApiField('protocol'), + timeout + ); const requestStartTime = Date.now(); @@ -466,77 +483,48 @@ StripeResource.prototype = { const maxRetries = this._getMaxNetworkRetries(options.settings); - req._requestEvent = requestEvent; - - req._requestStart = requestStartTime; - this._stripe._emitter.emit('request', requestEvent); - req.setTimeout(timeout, this._timeoutHandler(timeout, req, callback)); - - req.once('response', (res) => { - if (this._shouldRetry(res, requestRetries, maxRetries)) { - return retryRequest( - makeRequest, - apiVersion, - headers, - requestRetries, - ((res || {}).headers || {})['retry-after'] - ); - } else { - if (options.streaming && res.statusCode < 400) { - return this._streamingResponseHandler(req, callback)(res); + req + .then((res) => { + if (this._shouldRetry(res, requestRetries, maxRetries)) { + return retryRequest( + makeRequest, + apiVersion, + headers, + requestRetries, + res.getHeaders()['retry-after'] + ); + } else if (options.streaming && res.getStatusCode() < 400) { + return this._streamingResponseHandler(requestEvent, callback)(res); + } else { + return this._jsonResponseHandler(requestEvent, callback)(res); } - return this._jsonResponseHandler(req, callback)(res); - } - }); + }) + .catch((error) => { + if (this._shouldRetry(null, requestRetries, maxRetries)) { + return retryRequest( + makeRequest, + apiVersion, + headers, + requestRetries, + null + ); + } else { + const isTimeoutError = + error.code && error.code === HttpClient.TIMEOUT_ERROR_CODE; - req.on('error', (error) => { - if (this._shouldRetry(null, requestRetries, maxRetries)) { - return retryRequest( - makeRequest, - apiVersion, - headers, - requestRetries, - null - ); - } else { - if (error.code === 'ETIMEDOUT') { return callback.call( this, new StripeConnectionError({ - message: `Request aborted due to timeout being reached (${timeout}ms)`, + message: isTimeoutError + ? `Request aborted due to timeout being reached (${timeout}ms)` + : this._generateConnectionErrorMessage(requestRetries), detail: error, }) ); } - return callback.call( - this, - new StripeConnectionError({ - message: this._generateConnectionErrorMessage(requestRetries), - detail: error, - }), - null - ); - } - }); - - req.once('socket', (socket) => { - if (socket.connecting) { - socket.once( - isInsecureConnection ? 'connect' : 'secureConnect', - () => { - // Send payload; we're safe: - req.write(requestData); - req.end(); - } - ); - } else { - // we're already connected - req.write(requestData); - req.end(); - } - }); + }); }; const prepareAndMakeRequest = (error, data) => { diff --git a/lib/net/HttpClient.js b/lib/net/HttpClient.js new file mode 100644 index 0000000000..d9ad35bc13 --- /dev/null +++ b/lib/net/HttpClient.js @@ -0,0 +1,60 @@ +'use strict'; + +/* eslint-disable class-methods-use-this */ + +/** + * Encapsulates the logic for issuing a request to the Stripe API. This is an + * experimental interface and is not yet stable. + */ +class HttpClient { + makeRequest( + host, + port, + path, + method, + headers, + requestData, + protocol, + timeout + ) { + throw new Error('makeRequest not implemented.'); + } + + /** Helper to make a consistent timeout error across implementations. */ + static makeTimeoutError() { + const timeoutErr = new TypeError(HttpClient.TIMEOUT_ERROR_CODE); + timeoutErr.code = HttpClient.TIMEOUT_ERROR_CODE; + return timeoutErr; + } +} + +HttpClient.TIMEOUT_ERROR_CODE = 'ETIMEDOUT'; + +HttpClient.Response = class { + constructor(statusCode, headers) { + this._statusCode = statusCode; + this._headers = headers; + } + + getStatusCode() { + return this._statusCode; + } + + getHeaders() { + return this._headers; + } + + getRawResponse() { + throw new Error('getRawResponse not implemented.'); + } + + toStream(streamCompleteCallback) { + throw new Error('toStream not implemented.'); + } + + toJSON() { + throw new Error('toJSON not implemented.'); + } +}; + +module.exports = HttpClient; diff --git a/lib/net/NodeHttpClient.js b/lib/net/NodeHttpClient.js new file mode 100644 index 0000000000..164c31d0ad --- /dev/null +++ b/lib/net/NodeHttpClient.js @@ -0,0 +1,122 @@ +'use strict'; + +const http = require('http'); +const https = require('https'); + +const HttpClient = require('./HttpClient'); + +const defaultHttpAgent = new http.Agent({keepAlive: true}); +const defaultHttpsAgent = new https.Agent({keepAlive: true}); + +/** + * HTTP client which uses the Node `http` and `https` packages to issue + * requests.` + */ +class NodeHttpClient extends HttpClient { + constructor(agent) { + super(); + this._agent = agent; + } + + makeRequest( + host, + port, + path, + method, + headers, + requestData, + protocol, + timeout + ) { + const isInsecureConnection = protocol === 'http'; + + let agent = this._agent; + if (!agent) { + agent = isInsecureConnection ? defaultHttpAgent : defaultHttpsAgent; + } + + const requestPromise = new Promise((resolve, reject) => { + const req = (isInsecureConnection ? http : https).request({ + host: host, + port: port, + path, + method, + agent, + headers, + ciphers: 'DEFAULT:!aNULL:!eNULL:!LOW:!EXPORT:!SSLv2:!MD5', + }); + + req.setTimeout(timeout, () => { + req.destroy(HttpClient.makeTimeoutError()); + }); + + req.on('response', (res) => { + resolve(new NodeHttpResponse(res)); + }); + + req.on('error', (error) => { + reject(error); + }); + + req.once('socket', (socket) => { + if (socket.connecting) { + socket.once( + isInsecureConnection ? 'connect' : 'secureConnect', + () => { + // Send payload; we're safe: + req.write(requestData); + req.end(); + } + ); + } else { + // we're already connected + req.write(requestData); + req.end(); + } + }); + }); + + return requestPromise; + } +} + +class NodeHttpResponse extends HttpClient.Response { + constructor(res) { + super(res.statusCode, res.headers || {}); + this._res = res; + } + + getRawResponse() { + return this._res; + } + + toStream(streamCompleteCallback) { + // The raw response is itself the stream, so we just return that. To be + // backwards comaptible, we should invoke the streamCompleteCallback only + // once the stream has been fully consumed. + this._res.once('end', () => streamCompleteCallback()); + return this._res; + } + + toJSON() { + return new Promise((resolve, reject) => { + let response = ''; + + this._res.setEncoding('utf8'); + this._res.on('data', (chunk) => { + response += chunk; + }); + this._res.once('end', () => { + try { + resolve(JSON.parse(response)); + } catch (e) { + reject(e); + } + }); + }); + } +} + +NodeHttpClient.Response = NodeHttpResponse; + +module.exports = NodeHttpClient; diff --git a/lib/stripe.js b/lib/stripe.js index ab83c37950..b18432a4db 100644 --- a/lib/stripe.js +++ b/lib/stripe.js @@ -33,6 +33,7 @@ const ALLOWED_CONFIG_PROPERTIES = [ 'typescript', 'maxNetworkRetries', 'httpAgent', + 'httpClient', 'timeout', 'host', 'port', @@ -49,6 +50,9 @@ const {emitWarning} = utils; Stripe.StripeResource = require('./StripeResource'); Stripe.resources = resources; +Stripe.HttpClient = require('./net/HttpClient'); +Stripe.NodeHttpClient = require('./net/NodeHttpClient'); + function Stripe(key, config = {}) { if (!(this instanceof Stripe)) { return new Stripe(key, config); @@ -79,6 +83,8 @@ function Stripe(key, config = {}) { ); } + const agent = props.httpAgent || null; + this._api = { auth: null, host: props.host || DEFAULT_HOST, @@ -92,7 +98,8 @@ function Stripe(key, config = {}) { props.maxNetworkRetries, 0 ), - agent: props.httpAgent || null, + agent: agent, + httpClient: props.httpClient || new Stripe.NodeHttpClient(agent), dev: false, stripeAccount: props.stripeAccount || null, }; diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index 2f5ec6ccf9..9b79ec9979 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -1,16 +1,24 @@ 'use strict'; const utils = require('../testUtils'); - const nock = require('nock'); const stripe = require('../testUtils').getSpyableStripe(); const expect = require('chai').expect; const testUtils = require('../testUtils'); +const HttpClient = require('../lib/net/HttpClient'); const StripeResource = require('../lib/StripeResource'); const stripeMethod = StripeResource.method; +const { + StripeAuthenticationError, + StripeIdempotencyError, + StripePermissionError, + StripeRateLimitError, + StripeError, +} = require('../lib/Error'); + describe('StripeResource', () => { describe('createResourcePathWithSymbols', () => { it('Generates a path', () => { @@ -208,6 +216,98 @@ describe('StripeResource', () => { ); }); + it('throws an error on invalid JSON', (done) => { + return utils.getTestServerStripe( + {}, + (req, res) => { + // Write back JSON to close out the server. + res.write('invalidjson{}'); + res.end(); + }, + (err, stripe, closeServer) => { + if (err) { + return done(err); + } + stripe.charges.create(options.data, (err, result) => { + expect(err.message).to.deep.equal( + 'Invalid JSON received from the Stripe API' + ); + closeServer(); + done(); + }); + } + ); + }); + + it('throws a StripeAuthenticationError on 401', (done) => { + nock(`https://${options.host}`) + .post(options.path, options.params) + .reply(401, { + error: { + message: 'message', + }, + }); + + realStripe.charges.create(options.data, (err) => { + expect(err).to.be.an.instanceOf(StripeAuthenticationError); + expect(err.message).to.be.equal('message'); + done(); + }); + }); + + it('throws a StripePermissionError on 403', (done) => { + nock(`https://${options.host}`) + .post(options.path, options.params) + .reply(403, { + error: { + message: 'message', + }, + }); + + realStripe.charges.create(options.data, (err) => { + expect(err).to.be.an.instanceOf(StripePermissionError); + expect(err.message).to.be.equal('message'); + done(); + }); + }); + + it('throws a StripeRateLimitError on 429', (done) => { + nock(`https://${options.host}`) + .post(options.path, options.params) + .reply(429, { + error: { + message: 'message', + }, + }); + + realStripe.charges.create(options.data, (err) => { + expect(err).to.be.an.instanceOf(StripeRateLimitError); + expect(err.message).to.be.equal('message'); + done(); + }); + }); + + it('throws a StripeError based on the underlying error type', (done) => { + const error = { + type: 'idempotency_error', + }; + + expect(StripeError.generate(error)).to.be.an.instanceOf( + StripeIdempotencyError + ); + + nock(`https://${options.host}`) + .post(options.path, options.params) + .reply(400, { + error, + }); + + realStripe.charges.create(options.data, (err) => { + expect(err).to.be.an.instanceOf(StripeIdempotencyError); + done(); + }); + }); + it('retries connection timeout errors', (done) => { let nRequestsReceived = 0; return utils.getTestServerStripe( @@ -557,27 +657,88 @@ describe('StripeResource', () => { } ); }); + + it('invokes the callback with successful results', (done) => { + const returnedCharge = { + id: 'ch_123', + }; + return utils.getTestServerStripe( + {}, + (req, res) => { + res.write(JSON.stringify(returnedCharge)); + res.end(); + }, + (err, stripe, closeServer) => { + if (err) { + return done(err); + } + stripe.charges.create(options.data, (err, result) => { + expect(result).to.deep.equal(returnedCharge); + closeServer(); + done(); + }); + } + ); + }); + + it('returns successful results to await', (done) => { + const returnedCharge = { + id: 'ch_123', + }; + return utils.getTestServerStripe( + {}, + (req, res) => { + res.write(JSON.stringify(returnedCharge)); + res.end(); + }, + async (err, stripe, closeServer) => { + if (err) { + return done(err); + } + const result = await stripe.charges.create(options.data); + expect(result).to.deep.equal(returnedCharge); + closeServer(); + done(); + } + ); + }); }); describe('_shouldRetry', () => { it("should return false if we've reached maximum retries", () => { - const res = stripe.invoices._shouldRetry({statusCode: 409}, 1, 1); + const res = stripe.invoices._shouldRetry( + new HttpClient.Response(409, {}), + 1, + 1 + ); expect(res).to.equal(false); }); it('should return true if we have more retries available', () => { - const res = stripe.invoices._shouldRetry({statusCode: 409}, 0, 1); + const res = stripe.invoices._shouldRetry( + new HttpClient.Response(409, {}), + 0, + 1 + ); expect(res).to.equal(true); }); it('should return true if the error code is either 409 or 503', () => { - let res = stripe.invoices._shouldRetry({statusCode: 409}, 0, 1); + let res = stripe.invoices._shouldRetry( + new HttpClient.Response(409, {}), + 0, + 1 + ); expect(res).to.equal(true); - res = stripe.invoices._shouldRetry({statusCode: 503}, 0, 1); + res = stripe.invoices._shouldRetry( + new HttpClient.Response(503, {}), + 0, + 1 + ); expect(res).to.equal(true); }); @@ -585,10 +746,7 @@ describe('StripeResource', () => { it('should return false if the status is 200', () => { // mocking that we're on our 2nd request const res = stripe.invoices._shouldRetry( - { - statusCode: 200, - req: {_requestEvent: {method: 'POST'}}, - }, + new HttpClient.Response(200, {}), 1, 2 ); diff --git a/test/net/NodeHttpClient.spec.js b/test/net/NodeHttpClient.spec.js new file mode 100644 index 0000000000..d42eb346f3 --- /dev/null +++ b/test/net/NodeHttpClient.spec.js @@ -0,0 +1,215 @@ +'use strict'; + +const {Readable} = require('stream'); + +const http = require('http'); +const nock = require('nock'); +const expect = require('chai').expect; +// const {fail} = require('chai').assert; + +const {NodeHttpClient} = require('../../lib/Stripe'); +const utils = require('../../lib/utils'); +const {fail} = require('assert'); + +/** + * Readable stream which will emit a data event for each value in the array + * passed. Readable.from accomplishes this beyond Node 10.17. + */ +class ArrayReadable extends Readable { + constructor(values) { + super(); + this._index = 0; + this._values = values; + } + + _read() { + if (this._index == this._values.length) { + // Destroy the stream once we've read all values. + this.push(null); + } else { + this.push(Buffer.from(this._values[this._index], 'utf8')); + this._index += 1; + } + } +} + +describe('NodeHttpClient', () => { + const setupNock = () => { + return nock('http://stripe.com').get('/test'); + }; + + const sendRequest = (options) => { + options = options || {}; + return new NodeHttpClient().makeRequest( + 'stripe.com', + options.port || 80, + '/test', + options.method || 'GET', + options.headers || {}, + options.requestData, + 'http', + options.timeout || 1000 + ); + }; + + afterEach(() => { + nock.cleanAll(); + }); + + describe('makeRequest', () => { + it('rejects with a timeout error', async () => { + setupNock() + .delayConnection(31) + .reply(200, 'hello, world!'); + + try { + await sendRequest({timeout: 30}); + fail(); + } catch (e) { + expect(e.code).to.be.equal('ETIMEDOUT'); + } + }); + + it('forwards any error', async () => { + setupNock().replyWithError('sample error'); + + try { + await sendRequest(); + fail(); + } catch (e) { + expect(e.message).to.be.equal('sample error'); + } + }); + + it('sends request headers', async () => { + nock('http://stripe.com', { + reqheaders: { + sample: 'value', + }, + }) + .get('/test') + .reply(200); + + await sendRequest({headers: {sample: 'value'}}); + }); + + it('sends request data (POST)', (done) => { + const expectedData = utils.stringifyRequestData({id: 'test'}); + + nock('http://stripe.com') + .post('/test') + .reply(200, (uri, requestBody) => { + expect(requestBody).to.equal(expectedData); + done(); + }); + + sendRequest({method: 'POST', requestData: expectedData}); + }); + + it('custom port', async () => { + nock('http://stripe.com:1234') + .get('/test') + .reply(200); + await sendRequest({port: 1234}); + }); + + describe('Response', () => { + it('getStatusCode()', async () => { + setupNock().reply(418, 'hello, world!'); + + const response = await sendRequest(); + + expect(response.getStatusCode()).to.be.equal(418); + }); + + it('getHeaders()', async () => { + setupNock().reply(200, 'hello, world!', { + 'X-Header-1': '123', + 'X-Header-2': 'test', + }); + + const response = await sendRequest(); + + // Headers get transformed into lower case. + expect(response.getHeaders()).to.be.deep.equal({ + 'x-header-1': '123', + 'x-header-2': 'test', + }); + }); + + it('getRawResponse()', async () => { + setupNock().reply(200); + + const response = await sendRequest(); + + expect(response.getRawResponse()).to.be.an.instanceOf( + http.IncomingMessage + ); + }); + + it('toStream returns a readable stream', async () => { + setupNock().reply(200, () => new ArrayReadable(['hello, world!'])); + + const response = await sendRequest(); + + return new Promise((resolve) => { + const stream = response.toStream(() => true); + + let streamedContent = ''; + stream.on('data', (chunk) => { + streamedContent += chunk; + }); + stream.on('end', () => { + expect(streamedContent).to.equal('hello, world!'); + resolve(); + }); + }); + }); + + it('toStream invokes the streamCompleteCallback', async () => { + setupNock().reply(200, () => new ArrayReadable(['hello, world!'])); + + const response = await sendRequest(); + + return new Promise((resolve) => { + let streamedContent = ''; + + const stream = response.toStream(() => { + expect(streamedContent).to.equal('hello, world!'); + resolve(); + }); + + stream.on('data', (chunk) => { + streamedContent += chunk; + }); + }); + }); + + it('toJSON accumulates all data chunks in utf-8 encoding', async () => { + setupNock().reply( + 200, + () => new ArrayReadable(['{"a', 'bc":', '"∑ 123', '"}']) + ); + + const response = await sendRequest(); + + const json = await response.toJSON(); + + expect(json).to.deep.equal({abc: '∑ 123'}); + }); + + it('toJSON throws when JSON parsing fails', async () => { + setupNock().reply(200, '{"a'); + + const response = await sendRequest(); + + try { + await response.toJSON(); + fail(); + } catch (e) { + expect(e.message).to.be.equal('Unexpected end of JSON input'); + } + }); + }); + }); +}); diff --git a/types/2020-08-27/index.d.ts b/types/2020-08-27/index.d.ts index 5c7f8f1594..94dd90936e 100644 --- a/types/2020-08-27/index.d.ts +++ b/types/2020-08-27/index.d.ts @@ -1,6 +1,7 @@ // File generated from our OpenAPI spec /// +/// /// /// /// diff --git a/types/lib.d.ts b/types/lib.d.ts index 4154564225..53d11355aa 100644 --- a/types/lib.d.ts +++ b/types/lib.d.ts @@ -45,6 +45,7 @@ declare module 'stripe' { } export type LatestApiVersion = '2020-08-27'; export type HttpAgent = Agent; + export type HttpProtocol = 'http' | 'https'; export interface StripeConfig { /** @@ -80,6 +81,13 @@ declare module 'stripe' { */ httpAgent?: HttpAgent; + /** + * Use a custom http client, rather than relying on Node libraries. + * Useful for making requests in contexts other than NodeJS (eg. using + * `fetch`). + */ + httpClient?: HttpClient; + /** * Request timeout in milliseconds. * The default is 80000 @@ -90,7 +98,7 @@ declare module 'stripe' { port?: string | number; - protocol?: 'https' | 'http'; + protocol?: HttpProtocol; /** * Pass `telemetry: false` to disable headers that provide Stripe diff --git a/types/net/net.d.ts b/types/net/net.d.ts new file mode 100644 index 0000000000..0235036c1b --- /dev/null +++ b/types/net/net.d.ts @@ -0,0 +1,70 @@ +declare module 'stripe' { + namespace Stripe { + export namespace HttpClient { + /** + * Abstract representation of an HTTP response. This is an experimental + * interface and is not yet stable. + */ + interface Response { + /** The numeric HTTP status code for the response. */ + getStatusCode(): number; + + /** The response headers. */ + getHeaders(): {[key: string]: string}; + + /** This returns the underlying raw response object for the client. */ + getRawResponse(): any; //eslint-disable-line @typescript-eslint/no-explicit-any + + /** + * This returns the content as a stream. This is implementation-specific + * and so we cannot use a given type as different platforms have + * different stream primitives.The expectation is that content + * will not have been buffered into memory at this point by the client. + * + * The streamCompleteCallback should be invoked by the response + * implementation when the stream has been consumed. + */ + toStream(streamCompleteCallback: () => void): any; //eslint-disable-line @typescript-eslint/no-explicit-any + + /** + * Converts the response content into a JSON object, failing if JSON + * couldn't be parsed. + */ + toJSON(): Promise; + } + } + + /** + * Encapsulates the logic for issuing a request to the Stripe API. This is + * an experimental interface and is not yet stable. + */ + + export interface HttpClient { + makeRequest( + host: string, + port: string | number, + path: string, + method: 'GET' | 'POST' | 'PUT' | 'DELETE', + headers: object, + requestData: string | null, + protocol: Stripe.HttpProtocol, + timeout: number + ): Promise; + } + + export class NodeHttpClient implements HttpClient { + constructor(agent?: Stripe.HttpAgent); + + makeRequest( + host: string, + port: string | number, + path: string, + method: 'GET' | 'POST' | 'PUT' | 'DELETE', + headers: {[key: string]: string}, + requestData: string | null, + protocol: Stripe.HttpProtocol, + timeout: number + ): Promise; + } + } +} diff --git a/types/test/typescriptTest.ts b/types/test/typescriptTest.ts index 721cc25622..4a629b67ba 100644 --- a/types/test/typescriptTest.ts +++ b/types/test/typescriptTest.ts @@ -34,6 +34,7 @@ stripe = new Stripe('sk_test_123', { host: 'api.example.com', port: 123, telemetry: true, + httpClient: new Stripe.NodeHttpClient(), }); stripe.setTimeout(3000); @@ -188,3 +189,25 @@ Stripe.StripeResource.extend({ const maxBufferedRequestMetrics: number = Stripe.StripeResource.MAX_BUFFERED_REQUEST_METRICS; + +// Test HttpClient request processing. +import {Agent} from 'http'; +async (): Promise => { + const client: Stripe.HttpClient = new Stripe.NodeHttpClient(new Agent()); + + const response: Stripe.HttpClient.Response = await client.makeRequest( + 'api.stripe.com', + '443', + '/test', + 'POST', + { + 'Stripe-Account': 'account', + 'Content-Length': 123, + }, + 'requestdata', + 'https', + 80000 + ); + + const jsonResponse = await response.toJSON(); +}; From 90293d35391aae653cde2d14fc62d444923f5260 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Thu, 12 Aug 2021 18:04:27 -0400 Subject: [PATCH 02/10] Update stale makeRequest definition. --- types/net/net.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/net/net.d.ts b/types/net/net.d.ts index 0235036c1b..47885dc25c 100644 --- a/types/net/net.d.ts +++ b/types/net/net.d.ts @@ -60,7 +60,7 @@ declare module 'stripe' { port: string | number, path: string, method: 'GET' | 'POST' | 'PUT' | 'DELETE', - headers: {[key: string]: string}, + headers: object, requestData: string | null, protocol: Stripe.HttpProtocol, timeout: number From c23b6e64610ddf6551cd96ddc947c31f1bfe2b9f Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Thu, 12 Aug 2021 18:22:25 -0400 Subject: [PATCH 03/10] Add createNodeHttpClient method. --- lib/stripe.js | 9 ++++++--- test/net/NodeHttpClient.spec.js | 4 ++-- types/net/net.d.ts | 15 +-------------- types/test/typescriptTest.ts | 4 ++-- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/stripe.js b/lib/stripe.js index b18432a4db..ea7431e41b 100644 --- a/lib/stripe.js +++ b/lib/stripe.js @@ -51,8 +51,6 @@ Stripe.StripeResource = require('./StripeResource'); Stripe.resources = resources; Stripe.HttpClient = require('./net/HttpClient'); -Stripe.NodeHttpClient = require('./net/NodeHttpClient'); - function Stripe(key, config = {}) { if (!(this instanceof Stripe)) { return new Stripe(key, config); @@ -99,7 +97,7 @@ function Stripe(key, config = {}) { 0 ), agent: agent, - httpClient: props.httpClient || new Stripe.NodeHttpClient(agent), + httpClient: props.httpClient || Stripe.createNodeHttpClient(agent), dev: false, stripeAccount: props.stripeAccount || null, }; @@ -133,6 +131,11 @@ function Stripe(key, config = {}) { Stripe.errors = require('./Error'); Stripe.webhooks = require('./Webhooks'); +Stripe.createNodeHttpClient = (agent) => { + const NodeHttpClient = require('./net/NodeHttpClient'); + return new NodeHttpClient(agent); +}; + Stripe.prototype = { /** * @deprecated will be removed in a future major version. Use the config object instead: diff --git a/test/net/NodeHttpClient.spec.js b/test/net/NodeHttpClient.spec.js index d42eb346f3..7b1fbbd9b3 100644 --- a/test/net/NodeHttpClient.spec.js +++ b/test/net/NodeHttpClient.spec.js @@ -7,7 +7,7 @@ const nock = require('nock'); const expect = require('chai').expect; // const {fail} = require('chai').assert; -const {NodeHttpClient} = require('../../lib/Stripe'); +const {createNodeHttpClient} = require('../../lib/Stripe'); const utils = require('../../lib/utils'); const {fail} = require('assert'); @@ -40,7 +40,7 @@ describe('NodeHttpClient', () => { const sendRequest = (options) => { options = options || {}; - return new NodeHttpClient().makeRequest( + return createNodeHttpClient().makeRequest( 'stripe.com', options.port || 80, '/test', diff --git a/types/net/net.d.ts b/types/net/net.d.ts index 47885dc25c..8acc803a86 100644 --- a/types/net/net.d.ts +++ b/types/net/net.d.ts @@ -52,19 +52,6 @@ declare module 'stripe' { ): Promise; } - export class NodeHttpClient implements HttpClient { - constructor(agent?: Stripe.HttpAgent); - - makeRequest( - host: string, - port: string | number, - path: string, - method: 'GET' | 'POST' | 'PUT' | 'DELETE', - headers: object, - requestData: string | null, - protocol: Stripe.HttpProtocol, - timeout: number - ): Promise; - } + export const createNodeHttpClient: (agent?: HttpAgent | null) => HttpClient; } } diff --git a/types/test/typescriptTest.ts b/types/test/typescriptTest.ts index 4a629b67ba..3098fd3d31 100644 --- a/types/test/typescriptTest.ts +++ b/types/test/typescriptTest.ts @@ -34,7 +34,7 @@ stripe = new Stripe('sk_test_123', { host: 'api.example.com', port: 123, telemetry: true, - httpClient: new Stripe.NodeHttpClient(), + httpClient: Stripe.createNodeHttpClient(), }); stripe.setTimeout(3000); @@ -193,7 +193,7 @@ const maxBufferedRequestMetrics: number = // Test HttpClient request processing. import {Agent} from 'http'; async (): Promise => { - const client: Stripe.HttpClient = new Stripe.NodeHttpClient(new Agent()); + const client: Stripe.HttpClient = Stripe.createNodeHttpClient(new Agent()); const response: Stripe.HttpClient.Response = await client.makeRequest( 'api.stripe.com', From ca74ca96863b4bfa422935d6adca90e2d22089bf Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 10:40:46 -0400 Subject: [PATCH 04/10] Split out Response into own top-level class. --- lib/net/HttpClient.js | 6 +-- lib/net/NodeHttpClient.js | 10 ++--- lib/stripe.js | 7 +++- test/StripeResource.spec.js | 12 +++--- test/net/NodeHttpClient.spec.js | 2 +- types/net/net.d.ts | 69 ++++++++++++++++----------------- types/test/typescriptTest.ts | 2 +- 7 files changed, 53 insertions(+), 55 deletions(-) diff --git a/lib/net/HttpClient.js b/lib/net/HttpClient.js index d9ad35bc13..b4c758929f 100644 --- a/lib/net/HttpClient.js +++ b/lib/net/HttpClient.js @@ -30,7 +30,7 @@ class HttpClient { HttpClient.TIMEOUT_ERROR_CODE = 'ETIMEDOUT'; -HttpClient.Response = class { +class HttpClientResponse { constructor(statusCode, headers) { this._statusCode = statusCode; this._headers = headers; @@ -55,6 +55,6 @@ HttpClient.Response = class { toJSON() { throw new Error('toJSON not implemented.'); } -}; +} -module.exports = HttpClient; +module.exports = {HttpClient, HttpClientResponse}; diff --git a/lib/net/NodeHttpClient.js b/lib/net/NodeHttpClient.js index 164c31d0ad..da45ab9cf0 100644 --- a/lib/net/NodeHttpClient.js +++ b/lib/net/NodeHttpClient.js @@ -3,7 +3,7 @@ const http = require('http'); const https = require('https'); -const HttpClient = require('./HttpClient'); +const {HttpClient, HttpClientResponse} = require('./HttpClient'); const defaultHttpAgent = new http.Agent({keepAlive: true}); const defaultHttpsAgent = new https.Agent({keepAlive: true}); @@ -51,7 +51,7 @@ class NodeHttpClient extends HttpClient { }); req.on('response', (res) => { - resolve(new NodeHttpResponse(res)); + resolve(new NodeHttpClientResponse(res)); }); req.on('error', (error) => { @@ -80,7 +80,7 @@ class NodeHttpClient extends HttpClient { } } -class NodeHttpResponse extends HttpClient.Response { +class NodeHttpClientResponse extends HttpClientResponse { constructor(res) { super(res.statusCode, res.headers || {}); this._res = res; @@ -117,6 +117,4 @@ class NodeHttpResponse extends HttpClient.Response { } } -NodeHttpClient.Response = NodeHttpResponse; - -module.exports = NodeHttpClient; +module.exports = {NodeHttpClient, NodeHttpClientResponse}; diff --git a/lib/stripe.js b/lib/stripe.js index ea7431e41b..14751435aa 100644 --- a/lib/stripe.js +++ b/lib/stripe.js @@ -50,7 +50,10 @@ const {emitWarning} = utils; Stripe.StripeResource = require('./StripeResource'); Stripe.resources = resources; -Stripe.HttpClient = require('./net/HttpClient'); +const {HttpClient, HttpClientResponse} = require('./net/HttpClient'); +Stripe.HttpClient = HttpClient; +Stripe.HttpClientResponse = HttpClientResponse; + function Stripe(key, config = {}) { if (!(this instanceof Stripe)) { return new Stripe(key, config); @@ -132,7 +135,7 @@ Stripe.errors = require('./Error'); Stripe.webhooks = require('./Webhooks'); Stripe.createNodeHttpClient = (agent) => { - const NodeHttpClient = require('./net/NodeHttpClient'); + const {NodeHttpClient} = require('./net/NodeHttpClient'); return new NodeHttpClient(agent); }; diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index 9b79ec9979..9f9f8aea83 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -7,7 +7,7 @@ const stripe = require('../testUtils').getSpyableStripe(); const expect = require('chai').expect; const testUtils = require('../testUtils'); -const HttpClient = require('../lib/net/HttpClient'); +const {HttpClientResponse} = require('../lib/net/HttpClient'); const StripeResource = require('../lib/StripeResource'); const stripeMethod = StripeResource.method; @@ -707,7 +707,7 @@ describe('StripeResource', () => { describe('_shouldRetry', () => { it("should return false if we've reached maximum retries", () => { const res = stripe.invoices._shouldRetry( - new HttpClient.Response(409, {}), + new HttpClientResponse(409, {}), 1, 1 ); @@ -717,7 +717,7 @@ describe('StripeResource', () => { it('should return true if we have more retries available', () => { const res = stripe.invoices._shouldRetry( - new HttpClient.Response(409, {}), + new HttpClientResponse(409, {}), 0, 1 ); @@ -727,7 +727,7 @@ describe('StripeResource', () => { it('should return true if the error code is either 409 or 503', () => { let res = stripe.invoices._shouldRetry( - new HttpClient.Response(409, {}), + new HttpClientResponse(409, {}), 0, 1 ); @@ -735,7 +735,7 @@ describe('StripeResource', () => { expect(res).to.equal(true); res = stripe.invoices._shouldRetry( - new HttpClient.Response(503, {}), + new HttpClientResponse(503, {}), 0, 1 ); @@ -746,7 +746,7 @@ describe('StripeResource', () => { it('should return false if the status is 200', () => { // mocking that we're on our 2nd request const res = stripe.invoices._shouldRetry( - new HttpClient.Response(200, {}), + new HttpClientResponse(200, {}), 1, 2 ); diff --git a/test/net/NodeHttpClient.spec.js b/test/net/NodeHttpClient.spec.js index 7b1fbbd9b3..0100f9cec7 100644 --- a/test/net/NodeHttpClient.spec.js +++ b/test/net/NodeHttpClient.spec.js @@ -113,7 +113,7 @@ describe('NodeHttpClient', () => { await sendRequest({port: 1234}); }); - describe('Response', () => { + describe('NodeHttpClientResponse', () => { it('getStatusCode()', async () => { setupNock().reply(418, 'hello, world!'); diff --git a/types/net/net.d.ts b/types/net/net.d.ts index 8acc803a86..acac9609d9 100644 --- a/types/net/net.d.ts +++ b/types/net/net.d.ts @@ -1,44 +1,9 @@ declare module 'stripe' { namespace Stripe { - export namespace HttpClient { - /** - * Abstract representation of an HTTP response. This is an experimental - * interface and is not yet stable. - */ - interface Response { - /** The numeric HTTP status code for the response. */ - getStatusCode(): number; - - /** The response headers. */ - getHeaders(): {[key: string]: string}; - - /** This returns the underlying raw response object for the client. */ - getRawResponse(): any; //eslint-disable-line @typescript-eslint/no-explicit-any - - /** - * This returns the content as a stream. This is implementation-specific - * and so we cannot use a given type as different platforms have - * different stream primitives.The expectation is that content - * will not have been buffered into memory at this point by the client. - * - * The streamCompleteCallback should be invoked by the response - * implementation when the stream has been consumed. - */ - toStream(streamCompleteCallback: () => void): any; //eslint-disable-line @typescript-eslint/no-explicit-any - - /** - * Converts the response content into a JSON object, failing if JSON - * couldn't be parsed. - */ - toJSON(): Promise; - } - } - /** * Encapsulates the logic for issuing a request to the Stripe API. This is * an experimental interface and is not yet stable. */ - export interface HttpClient { makeRequest( host: string, @@ -49,7 +14,39 @@ declare module 'stripe' { requestData: string | null, protocol: Stripe.HttpProtocol, timeout: number - ): Promise; + ): Promise; + } + + /** + * Abstract representation of an HTTP response. This is an experimental + * interface and is not yet stable. + */ + interface HttpClientResponse { + /** The numeric HTTP status code for the response. */ + getStatusCode(): number; + + /** The response headers. */ + getHeaders(): {[key: string]: string}; + + /** This returns the underlying raw response object for the client. */ + getRawResponse(): any; //eslint-disable-line @typescript-eslint/no-explicit-any + + /** + * This returns the content as a stream. This is implementation-specific + * and so we cannot use a given type as different platforms have + * different stream primitives.The expectation is that content + * will not have been buffered into memory at this point by the client. + * + * The streamCompleteCallback should be invoked by the response + * implementation when the stream has been consumed. + */ + toStream(streamCompleteCallback: () => void): any; //eslint-disable-line @typescript-eslint/no-explicit-any + + /** + * Converts the response content into a JSON object, failing if JSON + * couldn't be parsed. + */ + toJSON(): Promise; } export const createNodeHttpClient: (agent?: HttpAgent | null) => HttpClient; diff --git a/types/test/typescriptTest.ts b/types/test/typescriptTest.ts index 3098fd3d31..e9ea2baa4f 100644 --- a/types/test/typescriptTest.ts +++ b/types/test/typescriptTest.ts @@ -195,7 +195,7 @@ import {Agent} from 'http'; async (): Promise => { const client: Stripe.HttpClient = Stripe.createNodeHttpClient(new Agent()); - const response: Stripe.HttpClient.Response = await client.makeRequest( + const response: Stripe.HttpClientResponse = await client.makeRequest( 'api.stripe.com', '443', '/test', From f68ec30ff7fcc9bd531a2951c27fc7639d2130c9 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 12:02:24 -0400 Subject: [PATCH 05/10] Update getTestServerStripe to use an agent that does not keep connections alive. --- test/stripe.spec.js | 4 ++-- testUtils/index.js | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/stripe.spec.js b/test/stripe.spec.js index 215c8ae1e7..366a461416 100644 --- a/test/stripe.spec.js +++ b/test/stripe.spec.js @@ -484,7 +484,7 @@ describe('Stripe Module', function() { let headers; let stripeClient; let closeServer; - before((callback) => { + beforeEach((callback) => { testUtils.getTestServerStripe( { stripeAccount: 'my_stripe_account', @@ -505,7 +505,7 @@ describe('Stripe Module', function() { } ); }); - after(() => closeServer()); + afterEach(() => closeServer()); it('is respected', (callback) => { stripeClient.customers.create((err) => { closeServer(); diff --git a/testUtils/index.js b/testUtils/index.js index e230262334..145fc2f4d0 100644 --- a/testUtils/index.js +++ b/testUtils/index.js @@ -10,6 +10,8 @@ const http = require('http'); const ResourceNamespace = require('../lib/ResourceNamespace').ResourceNamespace; +const testingHttpAgent = new http.Agent({keepAlive: false}); + const utils = (module.exports = { getTestServerStripe: (clientOptions, handler, callback) => { const server = http.createServer((req, res) => { @@ -28,6 +30,7 @@ const utils = (module.exports = { host: 'localhost', port, protocol: 'http', + httpAgent: testingHttpAgent, ...clientOptions, } ); From 066004bd644100c5c2d9310efae42bb7250a8c42 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 13:02:52 -0400 Subject: [PATCH 06/10] Add test where server responds with header and partial body before failing. --- test/StripeResource.spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index 9f9f8aea83..7f7f68e90a 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -17,6 +17,7 @@ const { StripePermissionError, StripeRateLimitError, StripeError, + StripeConnectionError, } = require('../lib/Error'); describe('StripeResource', () => { @@ -238,6 +239,27 @@ describe('StripeResource', () => { } ); }); + it('throws an valid headers but connection error', (done) => { + return utils.getTestServerStripe( + {}, + (req, res) => { + // Send out valid headers and a partial response. We then interrupt + // the response with an error. + res.writeHead(200); + res.write('{"ab'); + res.destroy(new Error('something happened')); + }, + (err, stripe, closeServer) => { + if (err) { + return done(err); + } + stripe.charges.create(options.data, (err, result) => { + expect(err).to.be.an.instanceOf(StripeConnectionError); + done(); + }); + } + ); + }); it('throws a StripeAuthenticationError on 401', (done) => { nock(`https://${options.host}`) From 861da4ff3907c87b128d5915eb485d3578776622 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 13:37:11 -0400 Subject: [PATCH 07/10] Add generics. --- types/net/net.d.ts | 25 ++++++++++++++++--------- types/test/typescriptTest.ts | 13 +++++++++---- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/types/net/net.d.ts b/types/net/net.d.ts index acac9609d9..609bdd8dfb 100644 --- a/types/net/net.d.ts +++ b/types/net/net.d.ts @@ -1,10 +1,15 @@ +/// + +import {IncomingMessage} from 'http'; declare module 'stripe' { namespace Stripe { /** * Encapsulates the logic for issuing a request to the Stripe API. This is * an experimental interface and is not yet stable. */ - export interface HttpClient { + export interface HttpClient< + ResponseType extends HttpClientResponse = HttpClientResponse + > { makeRequest( host: string, port: string | number, @@ -14,14 +19,14 @@ declare module 'stripe' { requestData: string | null, protocol: Stripe.HttpProtocol, timeout: number - ): Promise; + ): Promise; } /** * Abstract representation of an HTTP response. This is an experimental * interface and is not yet stable. */ - interface HttpClientResponse { + interface HttpClientResponse { /** The numeric HTTP status code for the response. */ getStatusCode(): number; @@ -29,18 +34,16 @@ declare module 'stripe' { getHeaders(): {[key: string]: string}; /** This returns the underlying raw response object for the client. */ - getRawResponse(): any; //eslint-disable-line @typescript-eslint/no-explicit-any + getRawResponse(): RawResponseType; /** - * This returns the content as a stream. This is implementation-specific - * and so we cannot use a given type as different platforms have - * different stream primitives.The expectation is that content + * This returns the content as a stream. The expectation is that content * will not have been buffered into memory at this point by the client. * * The streamCompleteCallback should be invoked by the response * implementation when the stream has been consumed. */ - toStream(streamCompleteCallback: () => void): any; //eslint-disable-line @typescript-eslint/no-explicit-any + toStream(streamCompleteCallback: () => void): StreamType; /** * Converts the response content into a JSON object, failing if JSON @@ -49,6 +52,10 @@ declare module 'stripe' { toJSON(): Promise; } - export const createNodeHttpClient: (agent?: HttpAgent | null) => HttpClient; + export const createNodeHttpClient: ( + agent?: HttpAgent | null + ) => HttpClient< + HttpClientResponse + >; } } diff --git a/types/test/typescriptTest.ts b/types/test/typescriptTest.ts index e9ea2baa4f..2a3fdb42d7 100644 --- a/types/test/typescriptTest.ts +++ b/types/test/typescriptTest.ts @@ -190,12 +190,12 @@ Stripe.StripeResource.extend({ const maxBufferedRequestMetrics: number = Stripe.StripeResource.MAX_BUFFERED_REQUEST_METRICS; -// Test HttpClient request processing. +// Test NodeHttpClient request processing. import {Agent} from 'http'; async (): Promise => { - const client: Stripe.HttpClient = Stripe.createNodeHttpClient(new Agent()); + const client = Stripe.createNodeHttpClient(new Agent()); - const response: Stripe.HttpClientResponse = await client.makeRequest( + const response = await client.makeRequest( 'api.stripe.com', '443', '/test', @@ -209,5 +209,10 @@ async (): Promise => { 80000 ); - const jsonResponse = await response.toJSON(); + const stream: Stripe.StripeStreamResponse = response.toStream(() => { + return; + }); + stream.setEncoding('utf8'); + + const jsonResponse: object = await response.toJSON(); }; From f83a0436384adcfda185f5a190df62424f0c3c9d Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 13:44:01 -0400 Subject: [PATCH 08/10] Add missing catch. --- test/StripeResource.spec.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index 7f7f68e90a..f3d4600ad3 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -717,10 +717,14 @@ describe('StripeResource', () => { if (err) { return done(err); } - const result = await stripe.charges.create(options.data); - expect(result).to.deep.equal(returnedCharge); - closeServer(); - done(); + try { + const result = await stripe.charges.create(options.data); + expect(result).to.deep.equal(returnedCharge); + closeServer(); + done(); + } catch (err) { + done(err); + } } ); }); From 5ca47263c9066483e6791012987426a6ea37c50d Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy <78050200+dcr-stripe@users.noreply.github.com> Date: Fri, 13 Aug 2021 14:03:13 -0400 Subject: [PATCH 09/10] Update test/net/NodeHttpClient.spec.js Co-authored-by: Richard Marmorstein <52928443+richardm-stripe@users.noreply.github.com> --- test/net/NodeHttpClient.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/net/NodeHttpClient.spec.js b/test/net/NodeHttpClient.spec.js index 0100f9cec7..84855ee036 100644 --- a/test/net/NodeHttpClient.spec.js +++ b/test/net/NodeHttpClient.spec.js @@ -23,7 +23,7 @@ class ArrayReadable extends Readable { } _read() { - if (this._index == this._values.length) { + if (this._index === this._values.length) { // Destroy the stream once we've read all values. this.push(null); } else { From e07699910c799bf8c1632f4d2a1ff579af53abb8 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 13 Aug 2021 14:08:01 -0400 Subject: [PATCH 10/10] Fix typescript warning about any. --- types/net/net.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/types/net/net.d.ts b/types/net/net.d.ts index 609bdd8dfb..f17af0ca16 100644 --- a/types/net/net.d.ts +++ b/types/net/net.d.ts @@ -26,6 +26,7 @@ declare module 'stripe' { * Abstract representation of an HTTP response. This is an experimental * interface and is not yet stable. */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any interface HttpClientResponse { /** The numeric HTTP status code for the response. */ getStatusCode(): number;