From bd8db702c40d970f47c3aa5c023bf3c632947729 Mon Sep 17 00:00:00 2001 From: manolodewiner Date: Tue, 28 Aug 2018 14:09:26 +0200 Subject: [PATCH] cap backoff time --- src/connection.js | 28 ++++++++++++++------------- src/request.js | 13 ++++++------- src/transport.js | 29 +++++++++++++++++++--------- test/connection/test_connection.js | 2 +- test/integration/test_integration.js | 4 ++-- test/transport/test_transport.js | 14 +------------- 6 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/connection.js b/src/connection.js index ef1ff86..2937fc5 100644 --- a/src/connection.js +++ b/src/connection.js @@ -5,22 +5,21 @@ import Transport from './transport' const HEADER_BLACKLIST = ['content-type'] -const DEFAULT_NODE = 'http://localhost:9984' +const DEFAULT_NODE = 'http://localhost:9984/api/v1/' + /** - * If initialized with ``>1`` nodes, the driver will send successive - requests to different nodes in a round-robin fashion (this will be - customizable in the future) * - * @nodes - * list of - * - * @headers + * @param {String, Array} nodes Nodes for the connection. String possible to be backwards compatible + * with version before 4.1.0 version + * @param {Object} headers Common headers for every request + * @param {float} timeout Optional timeout in secs * * */ + export default class Connection { - constructor(nodes, headers = {}, timeout = null) { - const nodesArray = Array.isArray(nodes) ? nodes : [nodes] + // 20 seconds is the default value for a timeout if not specified + constructor(nodes, headers = {}, timeout = 20000) { // Copy object this.headers = Object.assign({}, headers) @@ -32,13 +31,16 @@ export default class Connection { }) this.normalizedNodes = [] - if (!nodesArray) { + if (!nodes) { this.normalizedNodes.push(Connection.normalizeNode(DEFAULT_NODE, this.headers)) - } else { - nodesArray.forEach(node => { + } else if (Array.isArray(nodes)) { + nodes.forEach(node => { this.normalizedNodes.push(Connection.normalizeNode(node, this.headers)) }) + } else { + this.normalizedNodes.push(Connection.normalizeNode(nodes, this.headers)) } + this.transport = new Transport(this.normalizedNodes, this.headers, timeout) } diff --git a/src/request.js b/src/request.js index 82c4dd6..5dc8d66 100644 --- a/src/request.js +++ b/src/request.js @@ -29,7 +29,7 @@ export default class Request { this.connectionError = null } - async request(endpoint, config, setTimeout) { + async request(endpoint, config, timeout, maxBackoffTime) { // Load default fetch configuration and remove any falsy query parameters const requestConfig = Object.assign({}, this.node.headers, DEFAULT_REQUEST_CONFIG, config, { query: config.query && sanitize(config.query) @@ -58,7 +58,7 @@ export default class Request { const backoffTimedelta = this.getBackoffTimedelta() - if (setTimeout != null && setTimeout < this.backoffTimedelta) { + if (timeout != null && timeout < backoffTimedelta) { const errorObject = { message: 'TimeoutError' } @@ -67,16 +67,15 @@ export default class Request { if (backoffTimedelta > 0) { await Request.sleep(backoffTimedelta) } - this.timeout = setTimeout ? setTimeout - backoffTimedelta : setTimeout + // this.timeout = setTimeout ? setTimeout - backoffTimedelta : setTimeout return baseRequest(apiUrl, requestConfig) .then(res => res.json()) .catch(err => { // ConnectionError this.connectionError = err - // throw err }) .finally(() => { - this.updateBackoffTime() + this.updateBackoffTime(maxBackoffTime) }) } @@ -87,12 +86,12 @@ export default class Request { return (this.backoffTime - Date.now()) } - updateBackoffTime() { + updateBackoffTime(maxBackoffTime) { if (!this.connectionError) { this.retries = 0 this.backoffTime = null } else { - const backoffTimedelta = BACKOFF_DELAY * (2 ** this.retries) + const backoffTimedelta = Math.min(BACKOFF_DELAY * (2 ** this.retries), maxBackoffTime) this.backoffTime = Date.now() + backoffTimedelta this.retries += 1 } diff --git a/src/transport.js b/src/transport.js index 5ddaf90..f0c72be 100644 --- a/src/transport.js +++ b/src/transport.js @@ -5,16 +5,28 @@ import Request from './request' +/** + * + * @private + * If initialized with ``>1`` nodes, the driver will send successive + * requests to different nodes in a round-robin fashion (this will be + * customizable in the future). + */ + + export default class Transport { constructor(nodes, headers, timeout) { this.connectionPool = [] this.timeout = timeout + // the maximum backoff time is 10 seconds + this.maxBackoffTime = timeout ? timeout / 10 : 10000 nodes.forEach(node => { this.connectionPool.push(new Request(node, headers)) }) } - // Select the connection with the earliest backoff time + // Select the connection with the earliest backoff time, in case of a tie, + // prefer the one with the smaller list index pickConnection() { if (this.connectionPool.length === 1) { return this.connectionPool[0] @@ -32,6 +44,7 @@ export default class Transport { async forwardRequest(path, headers) { let response let connection + // A new request will be executed until there is a valid response or timeout < 0 while (!this.timeout || this.timeout > 0) { connection = this.pickConnection() // Date in milliseconds @@ -41,25 +54,23 @@ export default class Transport { response = await connection.request( path, headers, - this.timeout + this.timeout, + this.maxBackoffTime ) const elapsed = Date.now() - startTime if (connection.backoffTime) { - this.timeout += elapsed + this.timeout -= elapsed } else { + // No connection error, the response is valid return response } - - if (connection.retries > 3) { - throw connection.connectionError - } } catch (err) { throw err } } const errorObject = { - message: 'Timeout error', + message: 'TimeoutError', } - throw errorObject + throw connection.connectionError ? connection.connectionError : errorObject } } diff --git a/test/connection/test_connection.js b/test/connection/test_connection.js index 9539f60..602a8d9 100644 --- a/test/connection/test_connection.js +++ b/test/connection/test_connection.js @@ -16,7 +16,7 @@ const conn = new Connection(API_PATH) test('Payload thrown at incorrect API_PATH', async t => { const path = 'http://localhost:9984/api/wrong/' - const connection = new Connection(path) + const connection = new Connection(path, {}, 0) const target = { message: 'HTTP Error: Requested page not reachable', status: '404 NOT FOUND', diff --git a/test/integration/test_integration.js b/test/integration/test_integration.js index a43530e..b19c4a9 100644 --- a/test/integration/test_integration.js +++ b/test/integration/test_integration.js @@ -28,8 +28,8 @@ test('Keypair is created', t => { // TODO: The following tests are a bit messy currently, please do: // // - tidy up dependency on `pollStatusAndFetchTransaction` -test('Valid CREATE transaction', t => { - const conn = new Connection(API_PATH) +test('Valid CREATE transaction with default node', t => { + const conn = new Connection() const tx = Transaction.makeCreateTransaction( asset(), diff --git a/test/transport/test_transport.js b/test/transport/test_transport.js index 5c70ad8..d423066 100644 --- a/test/transport/test_transport.js +++ b/test/transport/test_transport.js @@ -8,25 +8,13 @@ import { Connection } from '../../src' - -test('Pick connection with earliest backoff time', t => { - const path1 = 'http://localhost:9984/api/v1/' - const path2 = 'http://localhost:9984/api/wrong/' - - const conn = new Connection([path1, path2]) - - conn.searchAssets('example') - const connection1 = conn.transport.connectionPool[0] - - t.deepEqual(conn.transport.pickConnection(), connection1) -}) - test('Pick connection with earliest backoff time', async t => { const path1 = 'http://localhost:9984/api/v1/' const path2 = 'http://localhost:9984/api/wrong/' // Reverse order const conn = new Connection([path2, path1]) + // This will trigger the 'forwardRequest' so the correct connection will be taken await conn.searchAssets('example') const connection1 = conn.transport.connectionPool[1]