From ae028b2ae20f3ad82aba4c79c515d5cd6fbbd5bc Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 11 Mar 2019 17:13:02 +0100 Subject: [PATCH] Consistency for the win - The result object contains also the metadata about the request - The events emits the same object of the API response - The errors, where possible, exposes the APi response object under the meta key --- lib/Transport.js | 49 ++++++++++++++++++++++++++++-------------------- lib/errors.js | 36 +++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/lib/Transport.js b/lib/Transport.js index 0e45581c0..61ee70c44 100644 --- a/lib/Transport.js +++ b/lib/Transport.js @@ -88,20 +88,24 @@ class Transport { } callback = once(callback) - // TODO: return in the result the metadata const meta = { + request: { + params: null, + options: null + }, connection: null, - request: null, - response: null, attempts: 0, aborted: false } + const result = { body: null, statusCode: null, headers: null, - warnings: options.warnings || null + warnings: options.warnings || null, + meta } + const maxRetries = options.maxRetries || this.maxRetries const compression = options.compression || this.compression var request = { abort: noop } @@ -163,12 +167,13 @@ class Transport { params.headers = headers // serializes the querystring params.querystring = this.serializer.qserialize(params.querystring) + + meta.request.params = params + meta.request.options = options + this.emit('request', null, result) + // handles request timeout params.timeout = toMs(options.requestTimeout || this.requestTimeout) - - meta.request = params - this.emit('request', null, meta) - if (options.asStream === true) params.asStream = true // perform the actual http request return meta.connection.request(params, onResponse) @@ -194,9 +199,13 @@ class Transport { const error = err instanceof TimeoutError ? err - : new ConnectionError(err.message, params) + : new ConnectionError(err.message, result) - this.emit('response', error, meta) + if (err.name === 'TimeoutError') { + err.meta = result + } + + this.emit('response', error, result) return callback(error, result) } @@ -211,8 +220,7 @@ class Transport { if (options.asStream === true) { result.body = response - meta.response = result - this.emit('response', null, meta) + this.emit('response', null, result) callback(null, result) return } @@ -223,8 +231,8 @@ class Transport { response.on('data', chunk => { payload += chunk }) /* istanbul ignore next */ response.on('error', err => { - const error = new ConnectionError(err.message, params) - this.emit('response', error, meta) + const error = new ConnectionError(err.message, result) + this.emit('response', error, result) callback(error, result) }) response.on('end', () => { @@ -241,7 +249,7 @@ class Transport { try { result.body = this.serializer.deserialize(payload) } catch (err) { - this.emit('response', err, meta) + this.emit('response', err, result) return callback(err, result) } } else { @@ -272,17 +280,16 @@ class Transport { this.connectionPool.markAlive(meta.connection) } - meta.response = result if (ignoreStatusCode === false && statusCode >= 400) { const error = new ResponseError(result) - this.emit('response', error, meta) + this.emit('response', error, result) callback(error, result) } else { // cast to boolean if the request method was HEAD if (isHead === true && statusCode === 404) { result.body = false } - this.emit('response', null, meta) + this.emit('response', null, result) callback(null, result) } }) @@ -334,7 +341,8 @@ class Transport { if (err != null) { debug('Sniffing errored', err) - this.emit('sniff', err, { hosts: [], reason }) + result.meta.sniff = { hosts: [], reason } + this.emit('sniff', err, result) return callback(err) } @@ -342,7 +350,8 @@ class Transport { const hosts = this.connectionPool.nodesToHost(result.body.nodes) this.connectionPool.update(hosts) - this.emit('sniff', null, { hosts, reason }) + result.meta.sniff = { hosts, reason } + this.emit('sniff', null, result) callback(null, hosts) }) } diff --git a/lib/errors.js b/lib/errors.js index b611c47a5..3f4fddb39 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -27,31 +27,32 @@ class ElasticsearchClientError extends Error { } class TimeoutError extends ElasticsearchClientError { - constructor (message, request) { + constructor (message, meta) { super(message) Error.captureStackTrace(this, TimeoutError) this.name = 'TimeoutError' this.message = message || 'Timeout Error' - this.request = request + this.meta = meta } } class ConnectionError extends ElasticsearchClientError { - constructor (message, request) { + constructor (message, meta) { super(message) Error.captureStackTrace(this, ConnectionError) this.name = 'ConnectionError' this.message = message || 'Connection Error' - this.request = request + this.meta = meta } } class NoLivingConnectionsError extends ElasticsearchClientError { - constructor (message) { + constructor (message, meta) { super(message) Error.captureStackTrace(this, NoLivingConnectionsError) this.name = 'NoLivingConnectionsError' this.message = message || 'No Living Connections Error' + this.meta = meta } } @@ -83,16 +84,27 @@ class ConfigurationError extends ElasticsearchClientError { } class ResponseError extends ElasticsearchClientError { - constructor ({ body, statusCode, headers }) { + constructor (meta) { super('Response Error') Error.captureStackTrace(this, ResponseError) this.name = 'ResponseError' - this.message = (body && body.error && body.error.type) || 'Response Error' - this.body = body - this.statusCode = body && typeof body.status === 'number' - ? body.status - : statusCode - this.headers = headers + this.message = (meta.body && meta.body.error && meta.body.error.type) || 'Response Error' + this.meta = meta + } + + get body () { + return this.meta.body + } + + get statusCode () { + if (this.meta.body && typeof this.meta.body.status === 'number') { + return this.meta.body.status + } + return this.meta.statusCode + } + + get headers () { + return this.meta.headers } }