From 0009c33461307660584ef5124b06d0943010529d Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 15:07:55 +0100 Subject: [PATCH 1/9] Throw the error outside the requestFn --- src/http.ts | 55 ++++++++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/http.ts b/src/http.ts index 2ef81116..9a407f0f 100644 --- a/src/http.ts +++ b/src/http.ts @@ -18,7 +18,7 @@ let lastRequestId = 0; * @param {string} noEncoding Set to true to disable encoding, and return a Buffer. Defaults to false * @returns {Promise} Resolves to the response (body), rejected if a non-2xx status code was returned. */ -export function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"|"DELETE", endpoint: string, qs = null, body = null, headers = {}, timeout = 60000, raw = false, contentType = "application/json", noEncoding = false): Promise { +export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"|"DELETE", endpoint: string, qs = null, body = null, headers = {}, timeout = 60000, raw = false, contentType = "application/json", noEncoding = false): Promise { if (!endpoint.startsWith('/')) { endpoint = '/' + endpoint; } @@ -62,8 +62,8 @@ export function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"|"DELET } } - return new Promise((resolve, reject) => { - getRequestFn()(params, (err, response, resBody) => { + const {response, resBody} = await new Promise<{response: any, resBody: any}>((resolve, reject) => { + getRequestFn()(params, (err, res, resBody) => { if (err) { LogService.error("MatrixHttpClient", "(REQ-" + requestId + ")", err); reject(err); @@ -77,37 +77,40 @@ export function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"|"DELET } } - if (typeof (response.body) === 'string') { + if (typeof (res.body) === 'string') { try { - response.body = JSON.parse(response.body); + res.body = JSON.parse(res.body); } catch (e) { } } - const respIsBuffer = (response.body instanceof Buffer); + resolve({response: res, resBody}); + }); + }); - // Check for errors. - const errBody = response.body || resBody; - if (errBody && 'errcode' in errBody) { - const redactedBody = respIsBuffer ? '' : redactObjectForLogging(errBody); - LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); - reject(new MatrixError(errBody, response.statusCode)); - return; - } + const respIsBuffer = (response.body instanceof Buffer); - // Don't log the body unless we're in debug mode. They can be large. - if (LogService.level.includes(LogLevel.TRACE)) { - const redactedBody = respIsBuffer ? '' : redactObjectForLogging(response.body); - LogService.trace("MatrixHttpClient (REQ-" + requestId + " RESP-H" + response.statusCode + ")", redactedBody); - } + // Check for errors. + const errBody = response.body || resBody; + if (errBody && 'errcode' in errBody) { + const redactedBody = respIsBuffer ? '' : redactObjectForLogging(errBody); + LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); + throw new MatrixError(errBody, response.statusCode); + return; + } - if (response.statusCode < 200 || response.statusCode >= 300) { - const redactedBody = respIsBuffer ? '' : redactObjectForLogging(response.body); - LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); - reject(response); - } else resolve(raw ? response : resBody); - }); - }); + // Don't log the body unless we're in debug mode. They can be large. + if (LogService.level.includes(LogLevel.TRACE)) { + const redactedBody = respIsBuffer ? '' : redactObjectForLogging(response.body); + LogService.trace("MatrixHttpClient (REQ-" + requestId + " RESP-H" + response.statusCode + ")", redactedBody); + } + + if (response.statusCode < 200 || response.statusCode >= 300) { + const redactedBody = respIsBuffer ? '' : redactObjectForLogging(response.body); + LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); + throw response; + } + return (raw ? response : resBody); } export function redactObjectForLogging(input: any): any { From 04e7489d621f39b6024f24903fcf38e8103340bb Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 15:50:28 +0100 Subject: [PATCH 2/9] Use async for descriptor funcs --- src/metrics/decorators.ts | 111 ++++++++++++-------------------------- 1 file changed, 35 insertions(+), 76 deletions(-) diff --git a/src/metrics/decorators.ts b/src/metrics/decorators.ts index d7aea425..9173aff3 100644 --- a/src/metrics/decorators.ts +++ b/src/metrics/decorators.ts @@ -16,40 +16,25 @@ import { IdentityClientCallContext, IntentCallContext, MatrixClientCallContext } * @category Metrics */ export function timedMatrixClientFunctionCall() { - return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) { + return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; - descriptor.value = function (...args: any[]) { - const metrics = this.metrics; - - const context = metrics.assignUniqueContextId({ - functionName: propertyKey, + descriptor.value = async function (...args) { + const context = this.metrics.assignUniqueContextId({ + functionName, client: this, }); - metrics.start(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context); - - let result; - let exception; - + this.metrics.start(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context); try { - result = originalMethod.apply(this, args); + const result = await originalMethod.apply(this, args); + this.metrics.increment(METRIC_MATRIX_CLIENT_SUCCESSFUL_FUNCTION_CALL, context, 1); + return result; } catch (e) { - exception = e; - result = Promise.reject(e); - } - - let promise = result; - if (!(result instanceof Promise) && result !== null && result !== undefined) { - promise = Promise.resolve(result); + this.metrics.increment(METRIC_MATRIX_CLIENT_FAILED_FUNCTION_CALL, context, 1); + throw e; + } finally { + this.metrics.end(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context) } - - promise - .then(() => metrics.increment(METRIC_MATRIX_CLIENT_SUCCESSFUL_FUNCTION_CALL, context, 1)) - .catch(() => metrics.increment(METRIC_MATRIX_CLIENT_FAILED_FUNCTION_CALL, context, 1)) - .finally(() => metrics.end(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context)); - - if (exception) throw exception; - return result; - } + }; }; } @@ -58,39 +43,26 @@ export function timedMatrixClientFunctionCall() { * @category Metrics */ export function timedIdentityClientFunctionCall() { - return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) { + return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; - descriptor.value = function (...args: any[]) { + descriptor.value = async function (...args: any[]) { const metrics = this.metrics; const context = metrics.assignUniqueContextId({ - functionName: propertyKey, + functionName, client: this, }); - metrics.start(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context); - - let result; - let exception; - + this.metrics.start(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context); try { - result = originalMethod.apply(this, args); + const result = await originalMethod.apply(this, args); + this.metrics.increment(METRIC_IDENTITY_CLIENT_SUCCESSFUL_FUNCTION_CALL, context, 1); + return result; } catch (e) { - exception = e; - result = Promise.reject(e); + this.metrics.increment(METRIC_IDENTITY_CLIENT_FAILED_FUNCTION_CALL, context, 1); + throw e; + } finally { + this.metrics.end(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context) } - - let promise = result; - if (!(result instanceof Promise) && result !== null && result !== undefined) { - promise = Promise.resolve(result); - } - - promise - .then(() => metrics.increment(METRIC_IDENTITY_CLIENT_SUCCESSFUL_FUNCTION_CALL, context, 1)) - .catch(() => metrics.increment(METRIC_IDENTITY_CLIENT_FAILED_FUNCTION_CALL, context, 1)) - .finally(() => metrics.end(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context)); - - if (exception) throw exception; - return result; } }; } @@ -100,40 +72,27 @@ export function timedIdentityClientFunctionCall() { * @category Metrics */ export function timedIntentFunctionCall() { - return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) { + return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; - descriptor.value = function (...args: any[]) { + descriptor.value = async function (...args: any[]) { const metrics = this.metrics; const context = metrics.assignUniqueContextId({ - functionName: propertyKey, + functionName, client: this.client, intent: this, }); - metrics.start(METRIC_INTENT_FUNCTION_CALL, context); - - let result; - let exception; - + this.metrics.start(METRIC_INTENT_FUNCTION_CALL, context); try { - result = originalMethod.apply(this, args); + const result = await originalMethod.apply(this, args); + this.metrics.increment(METRIC_INTENT_SUCCESSFUL_FUNCTION_CALL, context, 1); + return result; } catch (e) { - exception = e; - result = Promise.reject(e); - } - - let promise = result; - if (!(result instanceof Promise) && result !== null && result !== undefined) { - promise = Promise.resolve(result); + this.metrics.increment(METRIC_INTENT_FAILED_FUNCTION_CALL, context, 1); + throw e; + } finally { + this.metrics.end(METRIC_INTENT_FUNCTION_CALL, context) } - - promise - .then(() => metrics.increment(METRIC_INTENT_SUCCESSFUL_FUNCTION_CALL, context, 1)) - .catch(() => metrics.increment(METRIC_INTENT_FAILED_FUNCTION_CALL, context, 1)) - .finally(() => metrics.end(METRIC_INTENT_FUNCTION_CALL, context)); - - if (exception) throw exception; - return result; } }; } From bc97d2f4a675947e9b60abb5a98786be4740eee5 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 15:50:47 +0100 Subject: [PATCH 3/9] A few error tidyups --- src/http.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/http.ts b/src/http.ts index 9a407f0f..ad2bebd8 100644 --- a/src/http.ts +++ b/src/http.ts @@ -63,16 +63,16 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| } const {response, resBody} = await new Promise<{response: any, resBody: any}>((resolve, reject) => { - getRequestFn()(params, (err, res, resBody) => { + getRequestFn()(params, (err, res, rBody) => { if (err) { LogService.error("MatrixHttpClient", "(REQ-" + requestId + ")", err); reject(err); return; } - if (typeof (resBody) === 'string') { + if (typeof (rBody) === 'string') { try { - resBody = JSON.parse(resBody); + rBody = JSON.parse(resBody); } catch (e) { } } @@ -84,7 +84,7 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| } } - resolve({response: res, resBody}); + resolve({response: res, resBody: rBody}); }); }); @@ -92,7 +92,7 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| // Check for errors. const errBody = response.body || resBody; - if (errBody && 'errcode' in errBody) { + if (typeof (errBody) === "object" && 'errcode' in errBody) { const redactedBody = respIsBuffer ? '' : redactObjectForLogging(errBody); LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); throw new MatrixError(errBody, response.statusCode); From eca6c422576c7698d67c9dcc98817dda7dd97b20 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 15:52:06 +0100 Subject: [PATCH 4/9] Use latest ES version supported by Node 14 (ES2020) --- tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index ec4b9a15..9afe056a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "emitDecoratorMetadata": true, "module": "commonjs", "moduleResolution": "node", - "target": "es2015", + "target": "es2020", "noImplicitAny": false, "sourceMap": false, "outDir": "./lib", From fed8994c0242e743952ffddad8f9c86a557b8cd1 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 5 May 2022 16:07:38 +0100 Subject: [PATCH 5/9] Update src/http.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tadeusz SoĊ›nierz --- src/http.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/http.ts b/src/http.ts index ad2bebd8..f6e59560 100644 --- a/src/http.ts +++ b/src/http.ts @@ -96,7 +96,6 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| const redactedBody = respIsBuffer ? '' : redactObjectForLogging(errBody); LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); throw new MatrixError(errBody, response.statusCode); - return; } // Don't log the body unless we're in debug mode. They can be large. From b9673e0d27075e2541aacecf71f69b6fd1f71d31 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 16:17:11 +0100 Subject: [PATCH 6/9] Use unknown --- src/metrics/decorators.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/metrics/decorators.ts b/src/metrics/decorators.ts index 9173aff3..f5d19895 100644 --- a/src/metrics/decorators.ts +++ b/src/metrics/decorators.ts @@ -16,7 +16,7 @@ import { IdentityClientCallContext, IntentCallContext, MatrixClientCallContext } * @category Metrics */ export function timedMatrixClientFunctionCall() { - return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { + return function (_target: unknown, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; descriptor.value = async function (...args) { const context = this.metrics.assignUniqueContextId({ @@ -46,9 +46,7 @@ export function timedIdentityClientFunctionCall() { return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; descriptor.value = async function (...args: any[]) { - const metrics = this.metrics; - - const context = metrics.assignUniqueContextId({ + const context = this.metrics.assignUniqueContextId({ functionName, client: this, }); @@ -75,9 +73,7 @@ export function timedIntentFunctionCall() { return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; descriptor.value = async function (...args: any[]) { - const metrics = this.metrics; - - const context = metrics.assignUniqueContextId({ + const context = this.metrics.assignUniqueContextId({ functionName, client: this.client, intent: this, From 30fe377c6be66ab0675d251e36167c653da2e45f Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 17:46:30 +0100 Subject: [PATCH 7/9] Further fixes --- src/metrics/decorators.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metrics/decorators.ts b/src/metrics/decorators.ts index f5d19895..cc2393fc 100644 --- a/src/metrics/decorators.ts +++ b/src/metrics/decorators.ts @@ -43,7 +43,7 @@ export function timedMatrixClientFunctionCall() { * @category Metrics */ export function timedIdentityClientFunctionCall() { - return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { + return function (_target: unknown, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; descriptor.value = async function (...args: any[]) { const context = this.metrics.assignUniqueContextId({ @@ -70,7 +70,7 @@ export function timedIdentityClientFunctionCall() { * @category Metrics */ export function timedIntentFunctionCall() { - return function (_target: never, functionName: string, descriptor: PropertyDescriptor) { + return function (_target: unknown, functionName: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value; descriptor.value = async function (...args: any[]) { const context = this.metrics.assignUniqueContextId({ From dd9dd78bc0edf291564def87efdd68e3a9e92dec Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 5 May 2022 17:54:17 +0100 Subject: [PATCH 8/9] Fix rBody --- src/http.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/http.ts b/src/http.ts index f6e59560..53eddff9 100644 --- a/src/http.ts +++ b/src/http.ts @@ -72,7 +72,7 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| if (typeof (rBody) === 'string') { try { - rBody = JSON.parse(resBody); + rBody = JSON.parse(rBody); } catch (e) { } } @@ -109,7 +109,7 @@ export async function doHttpRequest(baseUrl: string, method: "GET"|"POST"|"PUT"| LogService.error("MatrixHttpClient (REQ-" + requestId + ")", redactedBody); throw response; } - return (raw ? response : resBody); + return raw ? response : resBody; } export function redactObjectForLogging(input: any): any { From 5986a093c3d16b4dd2088f74ae873539fb3421f9 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 28 May 2022 20:39:57 -0600 Subject: [PATCH 9/9] Apply suggestions from code review --- src/metrics/decorators.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/metrics/decorators.ts b/src/metrics/decorators.ts index cc2393fc..f0064fa2 100644 --- a/src/metrics/decorators.ts +++ b/src/metrics/decorators.ts @@ -32,7 +32,7 @@ export function timedMatrixClientFunctionCall() { this.metrics.increment(METRIC_MATRIX_CLIENT_FAILED_FUNCTION_CALL, context, 1); throw e; } finally { - this.metrics.end(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context) + this.metrics.end(METRIC_MATRIX_CLIENT_FUNCTION_CALL, context); } }; }; @@ -59,7 +59,7 @@ export function timedIdentityClientFunctionCall() { this.metrics.increment(METRIC_IDENTITY_CLIENT_FAILED_FUNCTION_CALL, context, 1); throw e; } finally { - this.metrics.end(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context) + this.metrics.end(METRIC_IDENTITY_CLIENT_FUNCTION_CALL, context); } } }; @@ -87,7 +87,7 @@ export function timedIntentFunctionCall() { this.metrics.increment(METRIC_INTENT_FAILED_FUNCTION_CALL, context, 1); throw e; } finally { - this.metrics.end(METRIC_INTENT_FUNCTION_CALL, context) + this.metrics.end(METRIC_INTENT_FUNCTION_CALL, context); } } };