From db5b0daf63f37f295e9898cc18976b6adf89fcfc Mon Sep 17 00:00:00 2001 From: Peter Somogyvari Date: Thu, 9 Nov 2023 20:37:01 +0000 Subject: [PATCH] feat(cactus-core): add handleRestEndpointException utility to public API 1. This is a function that is designed to be called by all the REST API endpoint implementations to (more) correctly handle errors. 2. The problem right now is that we do not differentiate between invalid request errors (e.g. expected exceptions) vs. legitimate crashes (e.g. unexpected exceptions) What the above means is that a lot of our endpoints will crash with an HTTP 500 error code returned to the client even if the problem as user- error (such as a missing parameter that is mandatory). 3. With the new utility function the REST endpoint code can easily apply the decision logic at runtime in their own catch blocks' and set the HTTP response status code based on the information (context) provided by the callee (most often the connector plugin's underlying method that was called) An example usage of this utility method can be described as: 1. Add the necessary dependencies to your plugin (`http-errors`, `@types/http-errors`) 2. `yarn install` (which will update the lock file) 3. Choose the endpoint you wish to update to be using the new handleRestEndpointException function internally when handling HTTP requests that involve the plugin. For example this file: ``` packages/cactus-plugin-ledger-connector-besu/src/main/typescript/ web-services/deploy-contract-solidity-bytecode-endpoint.ts ``` 4. Update the `catch() { ... }` block of the `handleRequest` method to invoke the handleRestEndpointException method: ```typescript public async handleRequest(req: Request, res: Response): Promise { const fnTag = `${this.className}#handleRequest()`; const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`; this.log.debug(reqTag); const reqBody: DeployContractSolidityBytecodeV1Request = req.body; try { const resBody = await this.options.connector.deployContract(reqBody); res.json(resBody); } catch (ex) { const errorMsg = `${reqTag} ${fnTag} Failed to deploy contract:`; handleRestEndpointException({ errorMsg, log: this.log, error: ex, res }); } } ``` Then proceed to also update the implementation of the method that is being called by the REST endpoint request handler such that it uses the HTTP errors according to their intended status codes, e.g. 400 is user error and 5xx is something that is a developer error (e.g. indicating that a bug is in the code of the plugin and should be fixed) ```typescript import createHttpError from "http-errors"; export class SomePluginImplementration { public async deployContract( req: DeployContractSolidityBytecodeV1Request, ): Promise { const fnTag = `${this.className}#deployContract()`; Checks.truthy(req, `${fnTag} req`); if (isWeb3SigningCredentialNone(req.web3SigningCredential)) { throw createHttpError[400]( `${fnTag} Cannot deploy contract with pre-signed TX`, ); } const { keychainId, contractName } = req; if (!keychainId || !req.contractName) { const errorMessage = `${fnTag} Cannot deploy contract without keychainId and the contractName.`; throw createHttpError[400](errorMessage); } const keychainPlugin = this.pluginRegistry.findOneByKeychainId(keychainId); if (!keychainPlugin) { const errorMessage = `${fnTag} The plugin registry does not contain` + ` a keychain plugin for ID:"${req.keychainId}"`; throw createHttpError[400](errorMessage); } if (!keychainPlugin.has(contractName)) { const errorMessage = `${fnTag} Cannot create an instance of the contract instance because` + `the contractName in the request does not exist on the keychain`; throw new createHttpError[400](errorMessage); } // rest of the implementation goes here } ``` [skip ci] Related to, but does NOT conclude: https://github.com/hyperledger/cacti/issues/1747 Signed-off-by: Peter Somogyvari (cherry picked from commit dfa6396554abe6a8148d3d3ef9863db886710e91) --- packages/cactus-core/package.json | 2 + .../src/main/typescript/public-api.ts | 5 ++ .../handle-rest-endpoint-exception.ts | 73 +++++++++++++++++++ yarn.lock | 9 +++ 4 files changed, 89 insertions(+) create mode 100644 packages/cactus-core/src/main/typescript/web-services/handle-rest-endpoint-exception.ts diff --git a/packages/cactus-core/package.json b/packages/cactus-core/package.json index d0732f63a2..99277ebb98 100644 --- a/packages/cactus-core/package.json +++ b/packages/cactus-core/package.json @@ -55,12 +55,14 @@ "express": "4.18.2", "express-jwt-authz": "2.4.1", "express-openapi-validator": "5.0.4", + "http-errors": "2.0.0", "run-time-error-cjs": "1.4.0", "safe-stable-stringify": "2.4.3", "typescript-optional": "2.0.1" }, "devDependencies": { "@types/express": "4.17.19", + "@types/http-errors": "2.0.2", "uuid": "8.3.2" }, "engines": { diff --git a/packages/cactus-core/src/main/typescript/public-api.ts b/packages/cactus-core/src/main/typescript/public-api.ts index 3871206ef8..6e748c5696 100755 --- a/packages/cactus-core/src/main/typescript/public-api.ts +++ b/packages/cactus-core/src/main/typescript/public-api.ts @@ -18,3 +18,8 @@ export { GetOpenApiSpecV1EndpointBase, IGetOpenApiSpecV1EndpointBaseOptions, } from "./web-services/get-open-api-spec-v1-endpoint-base"; + +export { + IHandleRestEndpointExceptionOptions, + handleRestEndpointException, +} from "./web-services/handle-rest-endpoint-exception"; diff --git a/packages/cactus-core/src/main/typescript/web-services/handle-rest-endpoint-exception.ts b/packages/cactus-core/src/main/typescript/web-services/handle-rest-endpoint-exception.ts new file mode 100644 index 0000000000..66d291df8d --- /dev/null +++ b/packages/cactus-core/src/main/typescript/web-services/handle-rest-endpoint-exception.ts @@ -0,0 +1,73 @@ +import { + Logger, + createRuntimeErrorWithCause, +} from "@hyperledger/cactus-common"; +import type { Response } from "express"; +import createHttpError from "http-errors"; + +/** + * An interface describing the object contaiing the contextual information needed by the + * `#handleRestEndpointException()` method to perform its duties. + * + * @param ctx - An object containing options for handling the REST endpoint exception. + * @param ctx.errorMsg - The error message to log (if there will be error logging e.g. HTTP 500) + * @param ctx.log - The logger instance used for logging errors and/or debug messages. + * @param ctx.error - The error object representing the exception that is being handled. + * @param ctx.res - The Express response object to send the HTTP response. + */ +export interface IHandleRestEndpointExceptionOptions { + readonly errorMsg: string; + readonly log: Logger; + readonly error: unknown; + readonly res: Response; +} + +/** + * Handles exceptions thrown during REST endpoint processing and sends an appropriate HTTP response. + * + * If the exception is an instance of `HttpError` from the `http-errors` library, + * it logs the error at the debug level and sends a JSON response with the error details + * and the corresponding HTTP status code. + * + * If the exception is not an instance of `HttpError`, it logs the error at the error level, + * creates a runtime error with the original error as the cause, and sends a JSON response + * with a generic "Internal Server Error" message and a 500 HTTP status code. + * + * @param ctx - An object containing options for handling the REST endpoint exception. + */ +export function handleRestEndpointException( + ctx: Readonly, +): void { + if (createHttpError.isHttpError(ctx.error)) { + ctx.res.status(ctx.error.statusCode); + + // Log either an error or a debug message depending on what the statusCode is + // For 5xx errors we treat it as a production bug that needs to be fixed on + // our side and for everything else we treat it a user error and debug log it. + if (ctx.error.statusCode >= 500) { + ctx.log.debug(ctx.errorMsg, ctx.error); + } else { + ctx.log.error(ctx.errorMsg, ctx.error); + } + + // If the `expose` property is set to true it implies that we can safely + // expose the contents of the exception to the calling client. + if (ctx.error.expose) { + ctx.res.json({ + message: ctx.error.message, + error: ctx.error, + }); + } + } else { + // If the exception was not an http-error then we assume it was an internal + // error (e.g. same behavior as if we had received an HTTP 500 statusCode) + ctx.log.error(ctx.errorMsg, ctx.error); + + const rex = createRuntimeErrorWithCause(ctx.errorMsg, ctx.error); + + ctx.res.status(500).json({ + message: "Internal Server Error", + error: rex, + }); + } +} diff --git a/yarn.lock b/yarn.lock index 0ae821e901..c99bc5ea75 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6817,9 +6817,11 @@ __metadata: "@hyperledger/cactus-common": 2.0.0-alpha.2 "@hyperledger/cactus-core-api": 2.0.0-alpha.2 "@types/express": 4.17.19 + "@types/http-errors": 2.0.2 express: 4.18.2 express-jwt-authz: 2.4.1 express-openapi-validator: 5.0.4 + http-errors: 2.0.0 run-time-error-cjs: 1.4.0 safe-stable-stringify: 2.4.3 typescript-optional: 2.0.1 @@ -12900,6 +12902,13 @@ __metadata: languageName: node linkType: hard +"@types/http-errors@npm:2.0.2": + version: 2.0.2 + resolution: "@types/http-errors@npm:2.0.2" + checksum: d7f14045240ac4b563725130942b8e5c8080bfabc724c8ff3f166ea928ff7ae02c5194763bc8f6aaf21897e8a44049b0492493b9de3e058247e58fdfe0f86692 + languageName: node + linkType: hard + "@types/http-proxy@npm:^1.17.8": version: 1.17.9 resolution: "@types/http-proxy@npm:1.17.9"