From 3ec37e3fe8233982e40e643e64a31318e381cbcf Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 5 Sep 2023 18:00:59 +0100 Subject: [PATCH] Introduce concept of Known and Unknown exceptions. Sometimes we don't need exceptions to be reported on the error level. This is especially true when the user does something out of order and there's a permission error. If we're aware of that possibility as developers and have accounted for it, then it's unnecessary for it to be treated the same way as a fatal error. We also decided to log CommandExceptions and CommandErrors as early as possible. https://github.com/Gnuxie/Draupnir/pull/93/ --- src/commands/CommandHandler.ts | 4 +-- src/commands/Rooms.tsx | 9 ++++-- .../interface-manager/CommandException.ts | 30 +++++++++++++++++-- src/commands/interface-manager/Validation.ts | 7 ++++- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/commands/CommandHandler.ts b/src/commands/CommandHandler.ts index df39c90c..f2212db3 100644 --- a/src/commands/CommandHandler.ts +++ b/src/commands/CommandHandler.ts @@ -48,7 +48,7 @@ import { BaseFunction, CommandTable, defineCommandTable, findCommandTable, findT import { findMatrixInterfaceAdaptor, MatrixContext } from "./interface-manager/MatrixInterfaceAdaptor"; import { ArgumentStream } from "./interface-manager/ParameterParsing"; import { CommandResult } from "./interface-manager/Validation"; -import { CommandException } from "./interface-manager/CommandException"; +import { CommandException, CommandExceptionKind } from "./interface-manager/CommandException"; import { tickCrossRenderer } from "./interface-manager/MatrixHelpRenderer"; import "./interface-manager/MatrixPresentations"; @@ -133,7 +133,7 @@ export async function handleCommand(roomId: string, event: { content: { body: st try { return await adaptor.invoke(mjolnirContext, mjolnirContext, ...stream.rest()); } catch (e) { - const commandError = new CommandException(e, 'Unknown Unexpected Error'); + const commandError = new CommandException(CommandExceptionKind.Unknown, e, 'Unknown Unexpected Error'); await tickCrossRenderer.call(mjolnirContext, mjolnir.client, roomId, event, CommandResult.Err(commandError)); } } diff --git a/src/commands/Rooms.tsx b/src/commands/Rooms.tsx index d5cd70a2..eaf80613 100644 --- a/src/commands/Rooms.tsx +++ b/src/commands/Rooms.tsx @@ -30,7 +30,7 @@ import { findPresentationType, parameters } from "./interface-manager/ParameterP import { MjolnirContext } from "./CommandHandler"; import { MatrixRoomID, MatrixRoomReference } from "./interface-manager/MatrixRoomReference"; import { CommandResult } from "./interface-manager/Validation"; -import { CommandException } from "./interface-manager/CommandException"; +import { CommandException, CommandExceptionKind } from "./interface-manager/CommandException"; import { defineMatrixInterfaceAdaptor } from "./interface-manager/MatrixInterfaceAdaptor"; import { tickCrossRenderer } from "./interface-manager/MatrixHelpRenderer"; import { DocumentNode } from "./interface-manager/DeadDocument"; @@ -92,7 +92,7 @@ defineInterfaceCommand({ return CommandException.Result( `The homeserver that Draupnir is hosted on cannot join this room using the room reference provided.\ Try an alias or the "share room" button in your client to obtain a valid reference to the room.`, - { exception: e } + { exception: e, exceptionKind: CommandExceptionKind.Unknown } ); } })(); @@ -121,7 +121,10 @@ defineInterfaceCommand({ try { await this.mjolnir.client.leaveRoom(roomID.toRoomIdOrAlias()); } catch (exception) { - return CommandException.Result(`Failed to leave ${roomRef.toPermalink()} - the room is no longer being protected, but the bot could not leave.`, { exception }); + return CommandException.Result( + `Failed to leave ${roomRef.toPermalink()} - the room is no longer being protected, but the bot could not leave.`, + { exceptionKind: CommandExceptionKind.Unknown, exception } + ); } return CommandResult.Ok(undefined); }, diff --git a/src/commands/interface-manager/CommandException.ts b/src/commands/interface-manager/CommandException.ts index ba8ee132..2dfa8ec2 100644 --- a/src/commands/interface-manager/CommandException.ts +++ b/src/commands/interface-manager/CommandException.ts @@ -5,6 +5,22 @@ import { randomUUID } from "crypto"; import { CommandError, CommandResult } from "./Validation"; +import { LogService } from "matrix-bot-sdk"; + +export enum CommandExceptionKind { + /** + * This class is for exceptions that need to be reported to the user, + * but are mostly irrelevant to the developers because the behaviour is well + * understood and expected. These exceptions will never be logged to the error + * level. + */ + Known = 'Known', + /** + * This class is to be used for reporting unexpected or unknown exceptions + * that the developers need to know about. + */ + Unknown = 'Unknown', +} // FIXME: I wonder if we could allow message to be JSX? // Then room references could be put into the DM and actually mean something. @@ -12,12 +28,22 @@ export class CommandException extends CommandError { public readonly uuid = randomUUID(); constructor( + public readonly exceptionKind: CommandExceptionKind, public readonly exception: Error|unknown, message: string) { super(message) } - public static Result(message: string, options: { exception: Error }): CommandResult { - return CommandResult.Err(new CommandException(options.exception, message)); + public static Result(message: string, options: { exception: Error, exceptionKind: CommandExceptionKind }): CommandResult { + return CommandResult.Err(new CommandException(options.exceptionKind, options.exception, message)); + } + + protected log(): void { + const logArguments: Parameters = ["CommandException", this.exceptionKind, this.uuid, this.message, this.exception]; + if (this.exceptionKind === CommandExceptionKind.Known) { + LogService.info(...logArguments); + } else { + LogService.error(...logArguments); + } } } diff --git a/src/commands/interface-manager/Validation.ts b/src/commands/interface-manager/Validation.ts index 7ee60a31..65582ab6 100644 --- a/src/commands/interface-manager/Validation.ts +++ b/src/commands/interface-manager/Validation.ts @@ -24,6 +24,7 @@ limitations under the License. * However, this file is modified and the modifications in this file * are NOT distributed, contributed, committed, or licensed under the Apache License. */ +import { LogService } from "matrix-bot-sdk"; type ValidationMatchExpression = { ok?: (ok: Ok) => any, err?: (err: Err) => any}; @@ -89,7 +90,7 @@ export class CommandError { public constructor( public readonly message: string, ) { - + this.log(); } /** @@ -101,4 +102,8 @@ export class CommandError { public static Result(message: string, _options = {}): CommandResult { return CommandResult.Err(new CommandError(message)); } + + protected log(): void { + LogService.info("CommandError", this.message); + } }