Skip to content

Commit

Permalink
fix: 1702 - Incorporate review comments
Browse files Browse the repository at this point in the history
Incorporate review comments and enhancing functional documentaion and
applying new Exception-Helper functionality to api-server as demo for
all other code

Closes: hyperledger-cacti#1702
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
  • Loading branch information
m-courtin committed Feb 2, 2022
1 parent 56b5dcc commit 11c5aa4
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 57 deletions.
36 changes: 13 additions & 23 deletions packages/cactus-cmd-api-server/src/main/typescript/api-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export class ApiServer {
} catch (ex) {
this.log.exception(ex, ApiServer.E_POST_CRASH_SHUTDOWN);
}
throw ExceptionHelper.getNewRuntimeErrorFromException(ex, context);
throw ExceptionHelper.newRuntimeError(ex, context);
}
}

Expand Down Expand Up @@ -301,7 +301,7 @@ export class ApiServer {
this.pluginRegistry = new PluginRegistry({ plugins: [] });
const context = "Failed to init PluginRegistry";
this.log.exception(ex, context);
throw ExceptionHelper.getNewRuntimeErrorFromException(ex, context);
throw ExceptionHelper.newRuntimeError(ex, context);
}
}

Expand Down Expand Up @@ -358,15 +358,10 @@ export class ApiServer {
await plugin.onPluginInit();

return plugin;
} catch (error) {
const errorMessage = `${fnTag} failed instantiating plugin '${packageName}' with the instanceId '${options.instanceId}'`;
this.log.error(errorMessage, error);

if (error instanceof Error) {
throw new RuntimeError(errorMessage, error);
} else {
throw new RuntimeError(errorMessage, JSON.stringify(error));
}
} catch (ex) {
const context = `${fnTag} failed instantiating plugin '${packageName}' with the instanceId '${options.instanceId}'`;
this.log.exception(ex, context);
throw ExceptionHelper.newRuntimeError(ex, context);
}
}

Expand All @@ -388,9 +383,9 @@ export class ApiServer {
await fs.mkdirp(pluginPackageDir);
this.log.debug(`${pkgName} plugin package dir: %o`, pluginPackageDir);
} catch (ex) {
const errorMessage =
const context =
"Could not create plugin installation directory, check the file-system permissions.";
throw new RuntimeError(errorMessage, ex);
throw ExceptionHelper.newRuntimeError(ex, context);
}
try {
lmify.setPackageManager("npm");
Expand All @@ -409,18 +404,13 @@ export class ApiServer {
]);
this.log.debug("%o install result: %o", pkgName, out);
if (out.exitCode !== 0) {
throw new RuntimeError("Non-zero exit code: ", JSON.stringify(out));
throw ExceptionHelper.newRuntimeError(out, "Non-zero exit code");
}
this.log.info(`Installed ${pkgName} OK`);
} catch (ex) {
const errorMessage = `${fnTag} failed installing plugin '${pkgName}`;
this.log.error(errorMessage, ex);

if (ex instanceof Error) {
throw new RuntimeError(errorMessage, ex);
} else {
throw new RuntimeError(errorMessage, JSON.stringify(ex));
}
const context = `${fnTag} failed installing plugin '${pkgName}`;
this.log.exception(ex, context);
throw ExceptionHelper.newRuntimeError(ex, context);
}
}

Expand Down Expand Up @@ -457,7 +447,7 @@ export class ApiServer {
await new Promise<void>((resolve, reject) => {
this.grpcServer.tryShutdown((ex?: Error) => {
if (ex) {
this.log.error("Failed to shut down gRPC server: ", ex);
this.log.exception(ex, "Failed to shut down gRPC server");
reject(ex);
} else {
resolve();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class ExceptionHelper {
* @param context providing context about the exception scenario
* @returns a new RuntimeError created from exception to preserve the complete hierarchy of exception information
*/
public static getNewRuntimeErrorFromException(
public static newRuntimeError(
exception: unknown,
context: string,
): RuntimeError {
Expand All @@ -40,7 +40,7 @@ export class ExceptionHelper {
let content = "";
let runtimeError = new RuntimeError(context, exceptionMessageInfo.message);

if (exception instanceof RuntimeError || exception instanceof Error) {
if (exception instanceof Error) {
// scenario 1: exception is already an instance of Error / RuntimeError -> can be used directly
runtimeError = new RuntimeError(context, exception);
} else {
Expand Down
70 changes: 43 additions & 27 deletions packages/cactus-common/src/main/typescript/logging/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,58 +23,74 @@ export interface ILoggerOptions {
}

/**
* STANDARD EXCEPTION HANDLING - EXAMPLE:
* ====================================
* STANDARD EXCEPTION HANDLING - EXAMPLE WITH RE-THROW:
* ====================================================
*
* Use the this logger with the .exception(ex) and hand in whatever exception of whatever type and format
* The logger will take care of it
* After logging it use the ExceptionHelper.newRuntimeError() to re-throw for a fast-fail
*
*
* public doSomething(): void {
* try {
* executeSomething();
* } catch (ex) {
* this.log.exception(ex);
* throw ExceptionHelper.newRuntimeError(ex, "ExecuteSomething failed");
* }
*
*
*
* EXCEPTION HANDLING WITH CONTEXT - EXAMPLE:
* ==========================================
*
* Use the this logger with the .exception(ex, "additional context") and hand in whatever exception of whatever type and format
* The logger will take care of it
* Additionally hand in some information about the exception scenario
* After logging the exception use the ExceptionHelper.newRuntimeError() to re-throw for a fast-fail
*
*
* public doSomething(): void {
* try {
* executeSomething();
* } catch (ex) {
* this.log.exception(ex, "Some additional information / context about the scenario");
* throw ExceptionHelper.newRuntimeError(ex, "ExecuteSomething failed");
* }
*
*
*
* EXCEPTION HANDLING WITH RE-THROW - EXAMPLE:
* ==========================================
* EXCEPTION HANDLING WITH CONDITIONAL HANDLING AND RE-THROW - EXAMPLE:
* ====================================================================
*
* in case you need to do a nested / cascaded exception-handling use the RuntimeError to re-throw and
* provide the previous exception as cause in the new RuntimeError to retain the information
*
* public doSomething(): void {
* try {
* try {
* executeSomething();
* } catch (ex) {
* this.log.exception(
* ex,
* "Some additional information / context about the scenario",
* );
*
* // maybe need to do a shut-down of some component here before re-throwing
*
* throw ExceptionHelper.getNewRuntimeErrorFromException(ex, "ExecuteSomething failed");
* }
*
* // do some further stuff here
* executeFurtherStuff();
* In case you need to do a conditional exception-handling:
* - Use the RuntimeError to re-throw and
* provide the previous exception as cause in the new RuntimeError to retain the information and distinguish
* between an exception you can handle and recover from and one you can't
*
* public async doSomething(): Promise<number> {
* try {
* await doSubTaskThatsAPartOfDoingSomething();
* } catch (ex) {
* this.log.exception(ex, "Execute Further Stuff also failed ...");
* // handle / distinguish
* if (ex instanceof MyErrorThatICanHandleAndRecoverFrom) {
* // An exception with a fixable scenario we can recover from thru an additional handling
* // do something here to handle and fix the issue
* // where "fixing" means that the we end up recovering
* // OK instead of having to crash. Recovery means that
* // we are confident that the second sub-task is safe to proceed with
* // despite of the error that was caught here
* this.log.exception(ex, "We got an failure in 'doSubTaskThatsAPartOfDoingSomething()' but we could fix it and recover to continue");
* } else {
* // An "unexpected exception" where we want to fail immediately
* // to avoid follow-up problems
* const context = "We got an severe failure in 'doSubTaskThatsAPartOfDoingSomething()' and need to stop directly here to avoid follow-up problems";
* this.log.exception(ex, context);
* throw ExceptionHelper.newRuntimeError(ex, context);
* }
* }
* const result = await doSecondAndFinalSubTask();
* return result; // 42
* }
*
*/

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,7 @@ describe("exception-helper tests", () => {
const context = "It happened ...";
const error = new Error(errorMessage);

const result = ExceptionHelper.getNewRuntimeErrorFromException(
error,
context,
);
const result = ExceptionHelper.newRuntimeError(error, context);

expect(result instanceof RuntimeError).toBe(true);
expect(result.message).toBe(context);
Expand All @@ -352,7 +349,7 @@ describe("exception-helper tests", () => {
const context = "It happened ...";
const arbitraryException = { error: errorMessage };

const result = ExceptionHelper.getNewRuntimeErrorFromException(
const result = ExceptionHelper.newRuntimeError(
arbitraryException,
context,
);
Expand Down

0 comments on commit 11c5aa4

Please sign in to comment.