diff --git a/docs/communication.md b/docs/communication.md index 6a286925..6866036e 100644 --- a/docs/communication.md +++ b/docs/communication.md @@ -43,6 +43,7 @@ Plugin classes also supply a `plugin` name. `Link.register` registers request and event classes and compiles their payload validators. Core classes are registered by the link module; `registerPluginMessages` registers each plugin's `messages` array. +It also registers the plugin's `errors` array with `Link.registerError`, so an error thrown by a request handler is rejected on the requester as the same class. Incoming messages invoke handlers attached with `link.handle(MessageClass, handler)`. Plugins attach these handlers from their entrypoint functions rather than relying on method-name conventions. diff --git a/docs/config-system.md b/docs/config-system.md index 4ac100ab..1102d43e 100644 --- a/docs/config-system.md +++ b/docs/config-system.md @@ -116,7 +116,7 @@ E.g.: let value = configInstance.fields["group.field"] The set method also takes an extra argument, which is the value to set the field to. -If the value is not valid this will throw an InvalidValue exception, if the field doesn't exist it will throw an InvalidField exception. +If the value is not valid this will throw an InvalidValueError exception, if the field doesn't exist it will throw an InvalidFieldError exception. ## Config Lifecycle diff --git a/docs/devs/plugin-migration-to-2.0.md b/docs/devs/plugin-migration-to-2.0.md index aae91383..0919fd6d 100644 --- a/docs/devs/plugin-migration-to-2.0.md +++ b/docs/devs/plugin-migration-to-2.0.md @@ -305,6 +305,30 @@ The remaining uses of "control" for clusterioctl were renamed to "ctl": The term "control" is still used in message `src` and `dst` and in config field locations, where it covers both ctl and the web interface. +## Request errors + +`RequestError` is gone. +Errors thrown by request handlers are now received by the requester as the same class, see [Request errors](/docs/writing-plugins.md#request-errors). +Replace `RequestError` with the class that fits: + +- Invalid input: `ValidationError`. +- Operation disabled by config: `ForbiddenError`. +- Missing instance, user, save etc.: `NotFoundError`. +- Instance running, host not connected, name taken etc.: `ConflictError`. +- Timeouts: `TimeoutError`. + +`catch` blocks that check `err instanceof RequestError` should check `ClientError` instead, or the specific class. +Checks of `err.code` for remote errors can use `instanceof`, e.g. `err instanceof lib.SessionLost`. + +Other renames: + +- `PermissionError` is `PermissionDeniedError`. +- `lib.InvalidValue`, `lib.InvalidField` and `lib.InvalidAccess` are `lib.InvalidValueError`, `lib.InvalidFieldError` and `lib.InvalidAccessError`. + +Errors of classes that are not registered, like a `TypeError` from a bug, are received as `InternalServerError`. +Plugins that define their own error classes need to list them in `errors` of the plugin declaration. + + ## Static assets in the web interface `staticRoot` now includes the `static/` prefix. diff --git a/docs/devs/protocol.md b/docs/devs/protocol.md index 7113d2f5..0f9cf15e 100644 --- a/docs/devs/protocol.md +++ b/docs/devs/protocol.md @@ -223,7 +223,8 @@ Plugins may define their own requests as well. - `src: Address` - Responding endpoint. - `dst: Address` - Requester's address, including the original request ID. - `data.message: string` - Human-readable error description. -- `data.code: string` (optional) - Error code. +- `data.code: string` (optional) - Registered name of the error class, `:` for plugin errors. + Missing or unknown names are received as `InternalServerError`. - `data.stack: string` (optional) - Error stack. diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md index a690bfe2..af781dea 100644 --- a/docs/writing-plugins.md +++ b/docs/writing-plugins.md @@ -154,6 +154,10 @@ The following properties are recognized: Array of request and event classes for this plugin. See guide for [defining link messages](#defining-link-messages) below. +**errors**: + Array of error classes for this plugin. + See [Request errors](#request-errors). + **permissions**: Array of permission definitions for this plugin. See [Plugin Permissions](#plugin-permissions) @@ -641,16 +645,16 @@ Registration rejects control-origin messages with no permission declaration. A callback receives `(user, message)`, where `message` is the raw wire envelope. It runs on the controller before destination payload validation and deserialization, so `message.data` is untrusted and may be missing or malformed. -Check any required payload shape before inspecting fields, and use `user.checkPermission(...)` or throw `PermissionError` for a normal denial. +Check any required payload shape before inspecting fields, and use `user.checkPermission(...)` or throw a `ClientError` such as `PermissionDeniedError` for a normal denial. For example, this checks the payload shape needed to choose a permission; the destination schema still validates the full payload: ```js -import { PermissionError } from "@clusterio/lib"; +import { ValidationError } from "@clusterio/lib"; function checkFrobnicatePermission(user, message) { const data = message.data; if (!data || typeof data !== "object" || typeof data.special !== "boolean") { - throw new PermissionError("Expected a boolean special field"); + throw new ValidationError("Expected a boolean special field"); } user.checkPermission(data.special ? "foo_frobber.special" : "foo_frobber.frobnicate"); } @@ -740,6 +744,53 @@ connection.sendTo({ instanceId: 123 }, message); connection.sendTo({ controlId: 123 }, message); ``` +### Request errors + +An error thrown by a request handler is sent back to the requester, and the promise returned by `.sendTo()` rejects with an instance of the same class. +The error classes follow the HTTP status codes: + +``` +Error +├── ClientError (4xx) +│ ├── ValidationError (400) +│ │ ├── InvalidValueError +│ │ └── InvalidFieldError +│ ├── ForbiddenError (403) +│ │ ├── PermissionDeniedError +│ │ └── InvalidAccessError +│ ├── NotFoundError (404) +│ ├── TimeoutError (408) +│ └── ConflictError (409) +└── ServerError (5xx) + └── InternalServerError (500) +``` + +Throw a `ClientError` subclass when the request can't be done as asked, for example: + +```js +controller.handle(messages.Frobnicate, async request => { + if (!frobnicators.has(request.id)) { + throw new lib.NotFoundError(`Frobnicator ${request.id} does not exist`); + } + // ... +}); +``` + +Errors of a class that is not registered are logged as unexpected by the side that threw them and are received as `InternalServerError`. +Registered errors are not logged. +The message and stack trace of the original error are kept in both cases. + +Plugins can define their own error classes by listing them in `errors` of the plugin declaration. +They need a static `plugin` property set to the plugin name and must take the message as the first constructor argument: + +```js +export class FrobnicatorJammed extends lib.ConflictError { + static plugin = "foo_frobber"; +} +``` + +A subclass that is not registered is received as its closest registered parent class. + ### Handling connection events There are a few connection related events that plugins neeed to repsond to in order to avoid data loss and connection problems. diff --git a/packages/controller/src/BaseConnection.ts b/packages/controller/src/BaseConnection.ts index b04c1946..0d1f3450 100644 --- a/packages/controller/src/BaseConnection.ts +++ b/packages/controller/src/BaseConnection.ts @@ -63,7 +63,7 @@ export default class BaseConnection extends lib.Link { let { id } = request; let modPack = this._controller.modPacks.get(id); if (!modPack) { - throw new lib.RequestError(`Mod pack with ID ${id} does not exist`); + throw new lib.NotFoundError(`Mod pack with ID ${id} does not exist`); } return modPack; } @@ -71,11 +71,11 @@ export default class BaseConnection extends lib.Link { async handleModPackGetDefaultRequest(): Promise { let id = this._controller.config.get("controller.default_mod_pack_id"); if (id === null) { - throw new lib.RequestError("Default mod pack not set on controller"); + throw new lib.ConflictError("Default mod pack not set on controller"); } let modPack = this._controller.modPacks.get(id); if (!modPack) { - throw new lib.RequestError(`Default mod pack configured (${id}) does not exist`); + throw new lib.NotFoundError(`Default mod pack configured (${id}) does not exist`); } return modPack; } @@ -84,10 +84,10 @@ export default class BaseConnection extends lib.Link { let filename = lib.ModInfo.filename(mod.name, mod.version); let modInfo = this._controller.modStore.files.get(filename); if (!modInfo) { - throw new lib.RequestError(`Mod ${filename} does not exist on controller`); + throw new lib.NotFoundError(`Mod ${filename} does not exist on controller`); } if (mod.sha1 && mod.sha1 !== modInfo.sha1) { - throw new lib.RequestError(`Mod ${filename} checksum does not match controller's checksum`); + throw new lib.ConflictError(`Mod ${filename} checksum does not match controller's checksum`); } return modInfo; } diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 28c6bc6a..e8c650f5 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -155,7 +155,7 @@ export default class ControlConnection extends BaseConnection { try { this.checkPermission(message, entry); } catch (err: any) { - this.connector.sendResponseError(new lib.ResponseError(err.message, err.code), message.src); + this.connector.sendResponseError(lib.Link.errorToResponse(err, false), message.src); logger.audit(`Permission denied for ${message.name} by ${this.user.id} from ${this.connector.dst}`); throw err; } @@ -197,7 +197,7 @@ export default class ControlConnection extends BaseConnection { async handleControllerRestartRequest() { if (!this._controller.canRestart) { - throw new lib.RequestError("Cannot restart, controller does not have a process monitor to restart it."); + throw new lib.ConflictError("Cannot restart, controller does not have a process monitor to restart it."); } if (await lib.packageOperationRunning()) { throw new lib.RequestError("Cannot restart while a plugin install or update is running on this machine."); @@ -205,7 +205,7 @@ export default class ControlConnection extends BaseConnection { const downgrade = await this._controller.checkRestartDowngrade(); if (downgrade) { const { installedVersion, runningVersion } = downgrade; - throw new lib.RequestError( + throw new lib.ConflictError( `Cannot restart controller with installed Clusterio version ${installedVersion} because it ` + `is older than running version ${runningVersion}. ` + "Stop the controller before starting the older version manually." @@ -303,13 +303,13 @@ export default class ControlConnection extends BaseConnection { async handleInstanceCreateRequest(request: lib.InstanceCreateRequest) { if ((request.config["instance.assigned_host"] ?? null) !== null) { - throw new lib.RequestError("instance.assigned_host must be set through the assign-host interface"); + throw new lib.ValidationError("instance.assigned_host must be set through the assign-host interface"); } const instanceConfig = new lib.InstanceConfig("controller"); if (request.cloneFromId) { const baseInstance = this._controller.instances.get(request.cloneFromId); if (!baseInstance) { - throw new lib.RequestError(`Instance with ID ${request.cloneFromId} does not exist`); + throw new lib.NotFoundError(`Instance with ID ${request.cloneFromId} does not exist`); } instanceConfig.update(baseInstance.config.toJSON(), false); instanceConfig.set("instance.assigned_host", null); // New instances are unassigned @@ -330,12 +330,12 @@ export default class ControlConnection extends BaseConnection { private static validateInstanceConfigSetField(fieldName: string) { if (fieldName === "instance.assigned_host") { - throw new lib.RequestError("instance.assigned_host must be set through the assign-host interface"); + throw new lib.ValidationError("instance.assigned_host must be set through the assign-host interface"); } if (fieldName === "instance.id") { // XXX is this worth implementing? It's race condition galore. - throw new lib.RequestError("Setting instance.id is not supported"); + throw new lib.ValidationError("Setting instance.id is not supported"); } } @@ -393,7 +393,7 @@ export default class ControlConnection extends BaseConnection { let ready = new Promise((resolve, reject) => { stream.events.on("source", resolve); stream.events.on("timeout", () => reject( - new lib.RequestError("Timed out establishing stream from host") + new lib.TimeoutError("Timed out establishing stream from host") )); }); ready.catch(() => {}); @@ -410,17 +410,17 @@ export default class ControlConnection extends BaseConnection { async handleInstanceTransferSaveRequest(request: lib.InstanceTransferSaveRequest) { if (request.sourceInstanceId === request.targetInstanceId) { - throw new lib.RequestError("Source and target instance may not be the same"); + throw new lib.ValidationError("Source and target instance may not be the same"); } let sourceInstance = this._controller.instances.getForRequest(request.sourceInstanceId); let targetInstance = this._controller.instances.getForRequest(request.targetInstanceId); let sourceHostId = sourceInstance.config.get("instance.assigned_host"); let targetHostId = targetInstance.config.get("instance.assigned_host"); if (sourceHostId === null) { - throw new lib.RequestError("Source instance is not assigned a host"); + throw new lib.ConflictError("Source instance is not assigned a host"); } if (targetHostId === null) { - throw new lib.RequestError("Target instance is not assigned a host"); + throw new lib.ConflictError("Target instance is not assigned a host"); } // Let host handle request if source and target is on the same host. @@ -431,12 +431,12 @@ export default class ControlConnection extends BaseConnection { // Check connectivity let sourceHostConnection = this._controller.wsServer.hostConnections.get(sourceHostId); if (!sourceHostConnection || sourceHostConnection.connector.closing) { - throw new lib.RequestError("Source host is not connected to the controller"); + throw new lib.ConflictError("Source host is not connected to the controller"); } let targetHostConnection = this._controller.wsServer.hostConnections.get(targetHostId); if (!targetHostConnection || targetHostConnection.connector.closing) { - throw new lib.RequestError("Target host is not connected to the controller"); + throw new lib.ConflictError("Target host is not connected to the controller"); } // Create stream to proxy from target to source @@ -445,7 +445,7 @@ export default class ControlConnection extends BaseConnection { if (stream.source) { stream.source.destroy(); } - stream.events.emit("error", new lib.RequestError("Timed out establishing transfer stream")); + stream.events.emit("error", new lib.TimeoutError("Timed out establishing transfer stream")); }); // Ignore errors if not listening for them to avoid crash. @@ -485,10 +485,10 @@ export default class ControlConnection extends BaseConnection { async handleModPackCreateRequest(request: lib.ModPackCreateRequest) { let modPack = request.modPack; if (modPack.id === undefined) { - throw new lib.RequestError("Mod pack need an ID to be created"); + throw new lib.ValidationError("Mod pack need an ID to be created"); } if (this._controller.modPacks.has(modPack.id)) { - throw new lib.RequestError(`Mod pack with ID ${modPack.id} already exist`); + throw new lib.ConflictError(`Mod pack with ID ${modPack.id} already exist`); } this._controller.modPacks.set(modPack); } @@ -496,7 +496,7 @@ export default class ControlConnection extends BaseConnection { async handleModPackUpdateRequest(request: lib.ModPackUpdateRequest) { let modPack = request.modPack; if (modPack.id === undefined || !this._controller.modPacks.has(modPack.id)) { - throw new lib.RequestError(`Mod pack with ID ${modPack.id} does not exist`); + throw new lib.NotFoundError(`Mod pack with ID ${modPack.id} does not exist`); } this._controller.modPacks.set(modPack); } @@ -505,7 +505,7 @@ export default class ControlConnection extends BaseConnection { let { id } = request; let modPack = this._controller.modPacks.getMutable(id); if (!modPack) { - throw new lib.RequestError(`Mod pack with ID ${id} does not exist`); + throw new lib.NotFoundError(`Mod pack with ID ${id} does not exist`); } this._controller.modPacks.delete(modPack); this._controller.deleteNote("mod_pack", id); @@ -587,7 +587,7 @@ export default class ControlConnection extends BaseConnection { author: (a:ModVersions, b:ModVersions) => strcmp(a.versions[0].author, b.versions[0].author), }; if (!Object.prototype.hasOwnProperty.call(sorters, sort)) { - throw new lib.RequestError(`Invalid value for sort: ${sort}`); + throw new lib.ValidationError(`Invalid value for sort: ${sort}`); } resultList.sort(sorters[sort as keyof typeof sorters]); let order = request.sortOrder; @@ -634,7 +634,7 @@ export default class ControlConnection extends BaseConnection { } catch (error: any) { logger.error(`Error fetching all mods from portal (${request.factorioVersion}): ${error}`); // Propagate a user-friendly error back to the client - throw new lib.RequestError(`Portal mod fetch failed: ${error}`); + throw new lib.InternalServerError(`Portal mod fetch failed: ${error}`); } } @@ -652,7 +652,7 @@ export default class ControlConnection extends BaseConnection { const token = this._controller.config.get("controller.factorio_token"); if (!username || username === "" || !token || token === "") { - throw new lib.RequestError("Factorio credentials (username, token) not configured on the controller."); + throw new lib.ConflictError("Factorio credentials (username, token) not configured on the controller."); } try { @@ -661,18 +661,16 @@ export default class ControlConnection extends BaseConnection { } catch (error: any) { logger.error(`Error downloading mods from portal: ${error.message}`); - // Improve error message clarity - let errorMessage = "Mod portal download failed"; - if (error instanceof lib.RequestError) { - errorMessage += `: ${error.message}`; - } else if (error.message.includes("401") || error.message.includes("Unauthorized")) { - errorMessage += ": Authentication failed. Check Factorio username/token in controller config."; - } else if (error.message.includes("404") || error.message.includes("Not Found")) { - errorMessage += ": Mod version not found on the portal."; - } else { - errorMessage += `: ${error.message}`; + const errorMessage = "Mod portal download failed"; + if (error.message.includes("401") || error.message.includes("Unauthorized")) { + throw new lib.ConflictError( + `${errorMessage}: Authentication failed. Check Factorio username/token in controller config.` + ); } - throw new lib.RequestError(errorMessage); + if (error.message.includes("404") || error.message.includes("Not Found")) { + throw new lib.NotFoundError(`${errorMessage}: Mod version not found on the portal.`); + } + throw new lib.InternalServerError(`${errorMessage}: ${error.message}`); } } @@ -682,7 +680,7 @@ export default class ControlConnection extends BaseConnection { for (const mod of request.mods) { const modInfo = installedMods.find(m => m.name === mod.name && mod.version.testVersion(m.version)); if (!modInfo) { - throw new lib.RequestError(`Failed to find mod ${mod.name} in store after download attempt.`); + throw new lib.InternalServerError(`Failed to find mod ${mod.name} in store after download attempt.`); } modInfos.push(modInfo); } @@ -716,7 +714,7 @@ export default class ControlConnection extends BaseConnection { } catch (error: any) { if (!error.message.includes("404") && !error.message.includes("Not Found")) { logger.error(`Error fetching mod from portal: ${error.message}`); - throw new lib.RequestError(`Dependency resolution failed: ${error.message}`); + throw new lib.InternalServerError(`Dependency resolution failed: ${error.message}`); } else { releases = null; } @@ -956,7 +954,7 @@ export default class ControlConnection extends BaseConnection { let { id, name, description, permissions } = request; let role = this._controller.roles.getMutable(id); if (!role) { - throw new lib.RequestError(`Role with ID ${id} does not exist`); + throw new lib.NotFoundError(`Role with ID ${id} does not exist`); } role.name = name; @@ -968,7 +966,7 @@ export default class ControlConnection extends BaseConnection { async handleRoleGrantDefaultPermissionsRequest(request: lib.RoleGrantDefaultPermissionsRequest) { let role = this._controller.roles.get(request.id); if (!role) { - throw new lib.RequestError(`Role with ID ${request.id} does not exist`); + throw new lib.NotFoundError(`Role with ID ${request.id} does not exist`); } role.grantDefaultPermissions(); @@ -980,7 +978,7 @@ export default class ControlConnection extends BaseConnection { const role = this._controller.roles.get(id); if (!role) { - throw new lib.RequestError(`Role with ID ${id} does not exist`); + throw new lib.NotFoundError(`Role with ID ${id} does not exist`); } this._controller.roles.delete(role); this._controller.deleteNote("role", id); @@ -1008,7 +1006,7 @@ export default class ControlConnection extends BaseConnection { let name = request.name; let user = this._controller.users.getByName(name); if (!user) { - throw new lib.RequestError(`User ${name} does not exist`); + throw new lib.NotFoundError(`User ${name} does not exist`); } return user.toUserDetails(); @@ -1025,7 +1023,7 @@ export default class ControlConnection extends BaseConnection { async handleUserRevokeTokenRequest(request: lib.UserRevokeTokenRequest) { let user = this._controller.users.getByName(request.name); if (!user) { - throw new lib.RequestError(`User '${request.name}' does not exist`); + throw new lib.NotFoundError(`User '${request.name}' does not exist`); } if (user.id !== this.user.id) { this.user.checkPermission("core.user.revoke_other_token"); @@ -1042,12 +1040,12 @@ export default class ControlConnection extends BaseConnection { async handleUserUpdateRolesRequest(request: lib.UserUpdateRolesRequest) { let user = this._controller.users.getByName(request.name); if (!user) { - throw new lib.RequestError(`User '${request.name}' does not exist`); + throw new lib.NotFoundError(`User '${request.name}' does not exist`); } for (let roleId of request.roles) { if (!this._controller.roles.has(roleId)) { - throw new lib.RequestError(`Role with ID ${roleId} does not exist`); + throw new lib.NotFoundError(`Role with ID ${roleId} does not exist`); } } @@ -1063,7 +1061,7 @@ export default class ControlConnection extends BaseConnection { this.user.checkPermission("core.user.create"); user = this._controller.users.createUser(name); } else { - throw new lib.RequestError(`User '${name}' does not exist`); + throw new lib.NotFoundError(`User '${name}' does not exist`); } } @@ -1079,7 +1077,7 @@ export default class ControlConnection extends BaseConnection { this.user.checkPermission("core.user.create"); user = this._controller.users.createUser(name); } else { - throw new lib.RequestError(`User '${name}' does not exist`); + throw new lib.NotFoundError(`User '${name}' does not exist`); } } @@ -1097,7 +1095,7 @@ export default class ControlConnection extends BaseConnection { this.user.checkPermission("core.user.create"); user = this._controller.users.createUser(name); } else { - throw new lib.RequestError(`User '${name}' does not exist`); + throw new lib.NotFoundError(`User '${name}' does not exist`); } } @@ -1109,7 +1107,7 @@ export default class ControlConnection extends BaseConnection { let name = request.name; let user = this._controller.users.getByName(name); if (!user) { - throw new lib.RequestError(`User '${name}' does not exist`); + throw new lib.NotFoundError(`User '${name}' does not exist`); } this._controller.users.deleteUser(user); @@ -1340,21 +1338,21 @@ export default class ControlConnection extends BaseConnection { async handleControllerUpdateRequest(request: lib.ControllerUpdateRequest) { if (!this._controller.config.get("controller.allow_remote_updates")) { - throw new lib.RequestError("Remote updates are disabled on this machine"); + throw new lib.ForbiddenError("Remote updates are disabled on this machine"); } return await lib.updatePackage("@clusterio/controller"); } async handlePluginUpdateRequest(request: lib.PluginUpdateRequest) { if (!this._controller.config.get("controller.allow_plugin_updates")) { - throw new lib.RequestError("Plugin updates are disabled on this machine"); + throw new lib.ForbiddenError("Plugin updates are disabled on this machine"); } return await lib.handlePluginUpdate(request.pluginPackage, this._controller.pluginInfos); } async handlePluginInstallRequest(request: lib.PluginInstallRequest) { if (!this._controller.config.get("controller.allow_plugin_install")) { - throw new lib.RequestError("Plugin installs are disabled on this machine"); + throw new lib.ForbiddenError("Plugin installs are disabled on this machine"); } const npmPackage = await lib.handlePluginInstall(request.pluginPackage); // A new plugin is not in pluginInfos, so checkRestartRequired cannot notice it. Version @@ -1366,10 +1364,10 @@ export default class ControlConnection extends BaseConnection { async handleUpdateAllRequest(request: lib.UpdateAllRequest) { if (!this._controller.config.get("controller.allow_remote_updates")) { - throw new lib.RequestError("Remote updates are disabled on this machine"); + throw new lib.ForbiddenError("Remote updates are disabled on this machine"); } if (!this._controller.config.get("controller.allow_plugin_updates")) { - throw new lib.RequestError("Plugin updates are disabled on this machine"); + throw new lib.ForbiddenError("Plugin updates are disabled on this machine"); } return await lib.handleUpdateAll("@clusterio/controller", this._controller.pluginInfos); } diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index a72a678f..e3924b63 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -1465,24 +1465,24 @@ export default class Controller { } else if (dst.type === lib.Address.control) { connection = this.wsServer.controlConnections.get(dst.id); if (!connection) { - throw new lib.RequestError("Target control connection does not exist."); + throw new lib.NotFoundError("Target control connection does not exist."); } } else if (dst.type === lib.Address.instance) { const instance = this.instances.getForRequest(dst.id); let hostId = instance.config.get("instance.assigned_host"); if (hostId === null) { - throw new lib.RequestError("Instance is not assigned to a host"); + throw new lib.ConflictError("Instance is not assigned to a host"); } connection = this.wsServer.hostConnections.get(hostId); if (!connection) { - throw new lib.RequestError("Host containing instance is not connected"); + throw new lib.ConflictError("Host containing instance is not connected"); } } else if (dst.type === lib.Address.host) { connection = this.wsServer.hostConnections.get(dst.id); if (!connection) { - throw new lib.RequestError("Host is not connected"); + throw new lib.ConflictError("Host is not connected"); } } else { @@ -1566,11 +1566,11 @@ export default class Controller { let instance = this.instances.getForRequest(request.instanceId); let hostId = instance.config.get("instance.assigned_host"); if (hostId === null) { - throw new lib.RequestError("Instance is not assigned to a host"); + throw new lib.ConflictError("Instance is not assigned to a host"); } let connection = this.wsServer.hostConnections.get(hostId); if (!connection) { - throw new lib.RequestError("Host containing instance is not connected"); + throw new lib.ConflictError("Host containing instance is not connected"); } return await connection.send(request); } diff --git a/packages/controller/src/ControllerRouter.ts b/packages/controller/src/ControllerRouter.ts index 4b90abb8..7fc579db 100644 --- a/packages/controller/src/ControllerRouter.ts +++ b/packages/controller/src/ControllerRouter.ts @@ -74,7 +74,7 @@ export default class ControllerRouter { return false; } origin.connector.sendResponseError( - new lib.ResponseError(msg || "Unroutable destination"), message.src + new lib.ResponseError(msg || "Unroutable destination", "ConflictError"), message.src ); return true; } diff --git a/packages/controller/src/InstanceManager.ts b/packages/controller/src/InstanceManager.ts index adc3d7d2..c8e08066 100644 --- a/packages/controller/src/InstanceManager.ts +++ b/packages/controller/src/InstanceManager.ts @@ -65,12 +65,12 @@ export default class InstanceManager { * * @param instanceId - ID of instance to get. * @returns The instance record. - * @throws {module:lib.RequestError} if the instance does not exist. + * @throws {module:lib.NotFoundError} if the instance does not exist. */ getForRequest(instanceId: number): InstanceRecord { const instance = this.records.get(instanceId); if (!instance) { - throw new lib.RequestError(`Instance with ID ${instanceId} does not exist`); + throw new lib.NotFoundError(`Instance with ID ${instanceId} does not exist`); } return instance; } @@ -125,7 +125,7 @@ export default class InstanceManager { * @param instanceConfig - Config to base the newly created instance on. * @param _suppressChanges - When true, side effects are suppressed, caller takes ownership. * @returns The created instance record. - * @throws {module:lib.RequestError} if an instance with the same ID already exists. + * @throws {module:lib.ConflictError} if an instance with the same ID already exists. */ async createInstance( instanceConfig: lib.InstanceConfig, @@ -133,7 +133,7 @@ export default class InstanceManager { ): Promise { const instanceId = instanceConfig.get("instance.id"); if (this.records.has(instanceId)) { - throw new lib.RequestError(`Instance with ID ${instanceId} already exists`); + throw new lib.ConflictError(`Instance with ID ${instanceId} already exists`); } const controllerName = this._controller.config.get("controller.name"); @@ -172,8 +172,8 @@ export default class InstanceManager { * @param hostId - ID of host to assign the instance to. If undefined, * the instance will be unassigned. * - * @throws {module:lib.RequestError} if the instance does not exist. - * @throws {module:lib.RequestError} if the target host is not connected. + * @throws {module:lib.NotFoundError} if the instance does not exist. + * @throws {module:lib.ConflictError} if the target host is not connected. */ async assignInstance( instanceId: number, @@ -191,7 +191,7 @@ export default class InstanceManager { if (hostId !== null) { newHostConnection = hostConnections.get(hostId); if (!newHostConnection) { - throw new lib.RequestError("Target host is not connected to the controller"); + throw new lib.ConflictError("Target host is not connected to the controller"); } } @@ -247,7 +247,7 @@ export default class InstanceManager { * * @param instanceId - ID of instance to delete. * - * @throws {module:lib.RequestError} if the instance does not exist. + * @throws {module:lib.NotFoundError} if the instance does not exist. */ async deleteInstance(instanceId: number): Promise { const instance = this.getForRequest(instanceId); diff --git a/packages/controller/src/User.ts b/packages/controller/src/User.ts index c7d50147..9a03af48 100644 --- a/packages/controller/src/User.ts +++ b/packages/controller/src/User.ts @@ -1,7 +1,7 @@ import { IUser, Role, SubscribableDatastore, - PermissionError, PermissionName, permissions, + PermissionDeniedError, PermissionName, permissions, } from "@clusterio/lib"; import UserRecord from "./UserRecord.js"; @@ -141,6 +141,6 @@ export default class User extends UserRecord implements IUser { } } - throw new PermissionError("Permission denied"); + throw new PermissionDeniedError("Permission denied"); } } diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index aeca0590..668e2a48 100644 --- a/packages/controller/src/routes.ts +++ b/packages/controller/src/routes.ts @@ -548,7 +548,7 @@ function checkModName(name: string) { try { lib.checkFilename(name); } catch (err: any) { - throw new lib.RequestError(`Mod name ${err.message}`); + throw new lib.ValidationError(`Mod name ${err.message}`); } } diff --git a/packages/ctl/ctl.ts b/packages/ctl/ctl.ts index d7618efb..7edbe82e 100755 --- a/packages/ctl/ctl.ts +++ b/packages/ctl/ctl.ts @@ -307,12 +307,12 @@ async function startCtl() { logger.error(`Error running command: ${err.message}`); process.exitCode = 1; - } else if (err instanceof lib.RequestError) { - if (err.stack) { - logger.error(`Error sending request:\n${err.stack}`); - } else { - logger.error(`Error sending request: ${err.message}`); - } + } else if (err instanceof lib.ClientError) { + logger.error(`Error sending request: ${err.message}`); + process.exitCode = 1; + + } else if (err instanceof lib.ServerError) { + logger.error(`Error sending request:\n${err.stack ?? err.message}`); process.exitCode = 1; } else { diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index 7700ec35..1f110b78 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -22,7 +22,7 @@ function checkRequestSaveName(name: string) { try { lib.checkFilename(name); } catch (err: any) { - throw new lib.RequestError(`Save name ${err.message}`); + throw new lib.ValidationError(`Save name ${err.message}`); } } @@ -203,7 +203,7 @@ export class HostRouter { return false; } origin.connector.sendResponseError( - new lib.ResponseError(msg || "Instance is not running."), message.src + new lib.ResponseError(msg || "Instance is not running.", "ConflictError"), message.src ); } return true; @@ -265,13 +265,13 @@ export class HostRouter { let dst = message.dst; if (nextHop) { origin.connector.sendResponseError( - new lib.ResponseError("Instance is already running."), message.src + new lib.ResponseError("Instance is already running.", "ConflictError"), message.src ); return; } if (this.host._shuttingDown) { origin.connector.sendResponseError( - new lib.ResponseError("Host is shutting down."), message.src + new lib.ResponseError("Host is shutting down.", "ConflictError"), message.src ); return; } @@ -280,9 +280,7 @@ export class HostRouter { instanceConnection.connector.forward(message); }).catch(err => { logger.error(`Error starting instance:\n${err.stack ?? err.message}`, this.host.instanceLogMeta(dst.id)); - origin.connector.sendResponseError( - new lib.ResponseError(err.message, err.code, err.stack), message.src - ); + origin.connector.sendResponseError(lib.Link.errorToResponse(err), message.src); }); } @@ -295,9 +293,7 @@ export class HostRouter { } } catch (err: any) { if (message.type === "request") { - origin.connector.sendResponseError( - new lib.ResponseError(err.message, err.code, err.stack), message.src - ); + origin.connector.sendResponseError(lib.Link.errorToResponse(err), message.src); } logger.warn(`Failed to deliver ${(message as any).name || "message"} (${message.type}): ${err.message}`); if (err instanceof lib.InvalidMessage && err.errors) { @@ -566,15 +562,15 @@ export default class Host extends lib.Link { async handleHostRestartRequest() { if (!this.canRestart) { - throw new lib.RequestError("Cannot restart, host does not have a process monitor to restart it."); + throw new lib.ConflictError("Cannot restart, host does not have a process monitor to restart it."); } if (await lib.packageOperationRunning()) { - throw new lib.RequestError("Cannot restart while a plugin install or update is running on this machine."); + throw new lib.ConflictError("Cannot restart while a plugin install or update is running on this machine."); } const downgrade = await this.checkRestartDowngrade(); if (downgrade) { const { installedVersion, runningVersion } = downgrade; - throw new lib.RequestError( + throw new lib.ConflictError( `Cannot restart host with installed Clusterio version ${installedVersion} because it is older than ` + `running version ${runningVersion}. Stop the host before starting the older version manually.` ); @@ -595,7 +591,7 @@ export default class Host extends lib.Link { if (fieldName === "host.id") { // Changing host.id at runtime is not worth the effort to // support and will break a lot of things if it is allowed. - throw new lib.RequestError("Setting 'host.id' while host is running is not supported"); + throw new lib.ConflictError("Setting 'host.id' while host is running is not supported"); } } @@ -840,7 +836,7 @@ export default class Host extends lib.Link { getRequestInstance(instanceId: number) { let instance = this.assignedInstances.get(instanceId); if (!instance) { - throw new lib.RequestError(`Instance with ID ${instanceId} does not exist`); + throw new lib.NotFoundError(`Instance with ID ${instanceId} does not exist`); } return instance; } @@ -886,7 +882,7 @@ export default class Host extends lib.Link { } if (!availablePorts.size) { const others = inUse.size ? ` (${inUse.size} in use by other processes)` : ""; - throw new lib.RequestError(`No available port left to assign from host.factorio_port_range${others}`); + throw new lib.ConflictError(`No available port left to assign from host.factorio_port_range${others}`); } const [newPort] = availablePorts; instanceToAssign.config.set("factorio.host_assigned_game_port", newPort); @@ -929,7 +925,7 @@ export default class Host extends lib.Link { async _connectInstance(instanceId: number) { let unloadedInstance = this.getRequestInstance(instanceId); if (this.instanceConnections.has(instanceId) || this._startingInstances.has(instanceId)) { - throw new lib.RequestError(`Instance with ID ${instanceId} is running`); + throw new lib.ConflictError(`Instance with ID ${instanceId} is running`); } let hostAddress = new lib.Address(lib.Address.host, this.config.get("host.id")); @@ -1084,10 +1080,10 @@ export default class Host extends lib.Link { ); } catch (err: any) { if (err.code === "EEXIST") { - throw new lib.RequestError(`${newName} already exists`); + throw new lib.ConflictError(`${newName} already exists`); } if (err.code === "ENOENT") { - throw new lib.RequestError(`${oldName} does not exist`); + throw new lib.NotFoundError(`${oldName} does not exist`); } throw err; } @@ -1107,7 +1103,7 @@ export default class Host extends lib.Link { ); } catch (err: any) { if (err.code === "ENOENT") { - throw new lib.RequestError(`${source} does not exist`); + throw new lib.NotFoundError(`${source} does not exist`); } throw err; } @@ -1156,7 +1152,7 @@ export default class Host extends lib.Link { } catch (err: any) { // ENOENT may also come from a missing target saves folder. if (err.code === "ENOENT" && !await fs.access(sourcePath).then(() => true, () => false)) { - throw new lib.RequestError(`${sourceName} does not exist`); + throw new lib.NotFoundError(`${sourceName} does not exist`); } throw err; } @@ -1188,7 +1184,7 @@ export default class Host extends lib.Link { await fs.unlink(path.join(instance.path, "saves", name)); } catch (err: any) { if (err.code === "ENOENT") { - throw new lib.RequestError(`${name} does not exist`); + throw new lib.NotFoundError(`${name} does not exist`); } throw err; } @@ -1219,7 +1215,7 @@ export default class Host extends lib.Link { content = (await fs.open(path.join(instance.path, "saves", name))).createReadStream(); } catch (err: any) { if (err.code === "ENOENT") { - throw new lib.RequestError(`${name} does not exist`); + throw new lib.NotFoundError(`${name} does not exist`); } throw err; } @@ -1238,12 +1234,12 @@ export default class Host extends lib.Link { async handleInstanceDeleteInternalRequest(request: lib.InstanceDeleteInternalRequest) { let instanceId = request.instanceId; if (this.instanceConnections.has(instanceId)) { - throw new lib.RequestError(`Instance with ID ${instanceId} is running`); + throw new lib.ConflictError(`Instance with ID ${instanceId} is running`); } let instance = this.discoveredInstances.get(instanceId); if (!instance) { - throw new lib.RequestError(`Instance with ID ${instanceId} does not exist`); + throw new lib.NotFoundError(`Instance with ID ${instanceId} does not exist`); } this.discoveredInstances.delete(instanceId); @@ -1253,21 +1249,21 @@ export default class Host extends lib.Link { async handleHostUpdateRequest(request: lib.HostUpdateRequest) { if (!this.config.get("host.allow_remote_updates")) { - throw new lib.RequestError("Remote updates are disabled on this machine"); + throw new lib.ForbiddenError("Remote updates are disabled on this machine"); } return await lib.updatePackage("@clusterio/host"); } async handlePluginUpdateRequest(request: lib.PluginUpdateRequest) { if (!this.config.get("host.allow_plugin_updates")) { - throw new lib.RequestError("Plugin updates are disabled on this machine"); + throw new lib.ForbiddenError("Plugin updates are disabled on this machine"); } return await lib.handlePluginUpdate(request.pluginPackage, this.pluginInfos); } async handlePluginInstallRequest(request: lib.PluginInstallRequest) { if (!this.config.get("host.allow_plugin_install")) { - throw new lib.RequestError("Plugin installs are disabled on this machine"); + throw new lib.ForbiddenError("Plugin installs are disabled on this machine"); } const npmPackage = await lib.handlePluginInstall(request.pluginPackage); // A new plugin is not in pluginInfos, so checkRestartRequired cannot notice it. Version @@ -1279,10 +1275,10 @@ export default class Host extends lib.Link { async handleUpdateAllRequest(request: lib.UpdateAllRequest) { if (!this.config.get("host.allow_remote_updates")) { - throw new lib.RequestError("Remote updates are disabled on this machine"); + throw new lib.ForbiddenError("Remote updates are disabled on this machine"); } if (!this.config.get("host.allow_plugin_updates")) { - throw new lib.RequestError("Plugin updates are disabled on this machine"); + throw new lib.ForbiddenError("Plugin updates are disabled on this machine"); } return await lib.handleUpdateAll("@clusterio/host", this.pluginInfos); } diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index bda1d0c4..1eebe450 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -21,7 +21,7 @@ function checkRequestSaveName(name: string) { try { lib.checkFilename(name); } catch (err: any) { - throw new lib.RequestError(`Save name ${err.message}`); + throw new lib.ValidationError(`Save name ${err.message}`); } } @@ -952,12 +952,12 @@ end`.replace(/\r?\n/g, " "); * Factorio refuses to read it if it is newer than the game. It then * loads default settings instead, which can corrupt the save. * - * @throws {lib.RequestError} if the mod pack version is higher than the server version. + * @throws {lib.ConflictError} if the mod pack version is higher than the server version. */ checkModPackVersion(modPack: lib.ModPack) { const serverVersion = this.server.version; if (serverVersion && modPack.integerFactorioVersion > lib.integerFullVersion(serverVersion)) { - throw new lib.RequestError( + throw new lib.ConflictError( `Mod pack ${modPack.name} is for Factorio ${modPack.factorioVersion} ` + `which is newer than the Factorio ${serverVersion} this instance runs` ); @@ -1360,7 +1360,7 @@ end`.replace(/\r?\n/g, " "); async handleInstanceLoadScenarioRequest(request: lib.InstanceLoadScenarioRequest) { if (this.config.get("factorio.enable_save_patching")) { this.notifyExit(); - throw new lib.RequestError("Load scenario cannot be used with save patching enabled"); + throw new lib.ConflictError("Load scenario cannot be used with save patching enabled"); } try { @@ -1422,7 +1422,7 @@ end`.replace(/\r?\n/g, " "); async handleInstanceSaveGameRequest() { if (this._status !== "running") { - throw new lib.RequestError("Instance is not running"); + throw new lib.ConflictError("Instance is not running"); } const saved = new Promise((resolve, reject) => { @@ -1432,7 +1432,7 @@ end`.replace(/\r?\n/g, " "); }; const onExit = () => { this.server.off("save-finished", onSaved); - reject(new lib.RequestError("Instance stopped before the save finished")); + reject(new lib.ConflictError("Instance stopped before the save finished")); }; this.server.once("save-finished", onSaved); this.server.once("exit", onExit); diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index 64c89655..6e2e9807 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -9,43 +9,12 @@ import * as libSchema from "../schema.js"; import { StringEnum } from "../data/composites.js"; import { safeOutputFile } from "../file_ops.js"; import * as validators from "./validators.js"; -import { RequestError } from "../errors.js"; +import { InvalidAccessError, InvalidFieldError, InvalidValueError } from "../errors.js"; import type { SyncHook } from "../SyncHook.js"; const ConfigLocation = StringEnum(["controller", "host", "control"]); export type ConfigLocation = Static; -/** - * Invalid Access exception - * - * Exception class for when an access attempt is made to a field that - * does not permitt this access is done. - * - * @extends Error - */ -export class InvalidAccess extends RequestError { }; - -/** - * Invalid Value exception - * - * Exception class for when invalid values are attempted to be set on a - * field. - * - * @extends Error - */ -export class InvalidValue extends RequestError { }; - -/** - * Invalid Field exception - * - * Exception class for when a group or field that does not exist is - * attempted to be accessed. - * - * @extends Error - */ -export class InvalidField extends RequestError { }; - - const OldConfigGroupSchema = Type.Object({ "name": Type.String(), "fields": Type.Record(Type.String(), Type.Unknown()), @@ -368,7 +337,7 @@ export class Config< * @param name - Name of the field being validated. * @param def - Definition describing the field type and constraints. * @param value - Value to validate and coerce. - * @throws {InvalidValue} if the value cannot be coerced to the correct type + * @throws {InvalidValueError} if the value cannot be coerced to the correct type * @returns The validated and coerced value. * @private */ @@ -380,7 +349,7 @@ export class Config< // If def is an enum then it must be a value within it if (def.enum && !def.enum.includes(value)) { - throw new InvalidValue(`Expected one of [${def.enum.join(", ")}], not ${value}`); + throw new InvalidValueError(`Expected one of [${def.enum.join(", ")}], not ${value}`); } // If it is a string, then attempt to coerce it @@ -402,7 +371,7 @@ export class Config< try { value = JSON.parse(value); } catch (err: any) { - throw new InvalidValue(`Error parsing value for ${name}: ${err.message}`); + throw new InvalidValueError(`Error parsing value for ${name}: ${err.message}`); } } } @@ -410,12 +379,12 @@ export class Config< // Check if it is allowed to be null if (value === null) { if (!def.optional) { - throw new InvalidValue(`Field ${name} cannot be null`); + throw new InvalidValueError(`Field ${name} cannot be null`); } // Check if is the correct type } else if (basicType(value) !== def.type) { - throw new InvalidValue(`Expected type of ${name} to be ${def.type}, not ${basicType(value)}`); + throw new InvalidValueError(`Expected type of ${name} to be ${def.type}, not ${basicType(value)}`); } return value as FieldType; @@ -429,7 +398,7 @@ export class Config< * @param location - location to check access for. * @param mode - type of access to check for. * @param error - throw on failure to pass check. - * @throws {InvalidAccess} + * @throws {InvalidAccessError} * if access is not granted and error is true. * @returns true if access is granted * @private @@ -448,7 +417,7 @@ export class Config< ) ) { if (error) { - throw new InvalidAccess(`Field '${name}' is not accessible from ${location}`); + throw new InvalidAccessError(`Field '${name}' is not accessible from ${location}`); } else { return false; } @@ -648,7 +617,7 @@ export class Config< } let def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } return this._checkAccess(name, def, location, mode, false); } @@ -663,7 +632,7 @@ export class Config< get(name: Field, remote = this.location) { let def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } // If remote access is requested require it to be able to read the field if (remote !== this.location) { @@ -675,7 +644,7 @@ export class Config< !this._checkAccess(name, def, this.location, ConfigAccess.write, false) && !this._checkAccess(name, def, this.location, ConfigAccess.read, false) ) { - throw new InvalidAccess(`Field '${name}' is not accessible from ${this.location}`); + throw new InvalidAccessError(`Field '${name}' is not accessible from ${this.location}`); } return this.fields[name]; @@ -690,13 +659,13 @@ export class Config< * @param name - Name of field to set. * @param newValue - Value to set for field. * @param remote - Location this field is being set from. - * @throws {InvalidField} if field is not defined. - * @throws {InvalidValue} if value is not allowed for the field. + * @throws {InvalidFieldError} if field is not defined. + * @throws {InvalidValueError} if value is not allowed for the field. */ set(name: Field, newValue: Fields[Field], remote = this.location) { const def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } this._checkAccess(name, def, remote, ConfigAccess.write, true); @@ -714,7 +683,7 @@ export class Config< try { def.validator(value, this); } catch (err: any) { - throw new InvalidValue(`Failed validation of ${name}: ${err.message}`); + throw new InvalidValueError(`Failed validation of ${name}: ${err.message}`); } } } finally { @@ -732,7 +701,7 @@ export class Config< try { dependent.def.validator(this.get(dependent.name as any, remote), this.staging); } catch (err: any) { - throw new InvalidValue(`Failed validation of dependent ${dependent.name}: ${err.message}`); + throw new InvalidValueError(`Failed validation of dependent ${dependent.name}: ${err.message}`); } } } finally { @@ -753,8 +722,8 @@ export class Config< * @param prop - Name of property to set on field. * @param value - the value to set the property to. * @param remote - Location this property is being set from - * @throws {InvalidField} if field is not defined. - * @throws {InvalidValue} if field is not an object. + * @throws {InvalidFieldError} if field is not defined. + * @throws {InvalidValueError} if field is not an object. */ setProp( name: Field, @@ -764,7 +733,7 @@ export class Config< ) { let def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } this._checkAccess(name, def, remote, ConfigAccess.write, true); @@ -773,7 +742,7 @@ export class Config< } if (def.type !== "object") { - throw new InvalidValue(`Cannot set property on non-object field '${name}'`); + throw new InvalidValueError(`Cannot set property on non-object field '${name}'`); } let prev = this.fields[name] as Record; @@ -793,7 +762,7 @@ export class Config< try { def.validator(updated, this); } catch (err: any) { - throw new InvalidValue(`Failed validation of ${name}: ${err.message}`); + throw new InvalidValueError(`Failed validation of ${name}: ${err.message}`); } } } finally { @@ -811,7 +780,7 @@ export class Config< try { dependent.def.validator(this.get(dependent.name as any, remote), this.staging); } catch (err: any) { - throw new InvalidValue(`Failed validation of dependent ${dependent.name}: ${err.message}`); + throw new InvalidValueError(`Failed validation of dependent ${dependent.name}: ${err.message}`); } } } finally { @@ -829,8 +798,8 @@ export class Config< * @param name - Name of field to set. * @param newValue - Value to set for field. * @param remote - Location this field is being set from. - * @throws {InvalidField} if field is not defined. - * @throws {InvalidValue} if value is not allowed for the field. + * @throws {InvalidFieldError} if field is not defined. + * @throws {InvalidValueError} if value is not allowed for the field. */ stage(name: Field, newValue: Fields[Field], remote = this.location) { if (this._readonly) { @@ -839,7 +808,7 @@ export class Config< const def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } this._checkAccess(name, def, remote, ConfigAccess.write, true); @@ -859,8 +828,8 @@ export class Config< * @param prop - Name of property to set on field. * @param value - the value to set the property to. * @param remote - Location this property is being set from - * @throws {InvalidField} if field is not defined. - * @throws {InvalidValue} if field is not an object. + * @throws {InvalidFieldError} if field is not defined. + * @throws {InvalidValueError} if field is not an object. */ stageProp( name: Field, @@ -874,7 +843,7 @@ export class Config< const def = this.constructor.fieldDefinitions[name]; if (!def) { - throw new InvalidField(`No field named '${name}'`); + throw new InvalidFieldError(`No field named '${name}'`); } this._checkAccess(name, def, remote, ConfigAccess.write, true); @@ -883,7 +852,7 @@ export class Config< } if (def.type !== "object") { - throw new InvalidValue(`Cannot set property on non-object field '${name}'`); + throw new InvalidValueError(`Cannot set property on non-object field '${name}'`); } const prev = this.staging.get(name) as Record; @@ -901,7 +870,7 @@ export class Config< /** * Commit all changes in stating after they have been validated. * - * @throws {InvalidValue} if a value is not allowed for its field. + * @throws {InvalidValueError} if a value is not allowed for its field. */ commitStaging() { if (this._readonly) { @@ -923,7 +892,7 @@ export class Config< try { fieldDef.validator(config.fields[fieldName as keyof Fields], config); } catch (err: any) { - throw new InvalidValue(`Failed validation of ${fieldName}: ${err.message}`); + throw new InvalidValueError(`Failed validation of ${fieldName}: ${err.message}`); } } } diff --git a/packages/lib/src/data/UserDetails.ts b/packages/lib/src/data/UserDetails.ts index ffdb2585..b46360c9 100644 --- a/packages/lib/src/data/UserDetails.ts +++ b/packages/lib/src/data/UserDetails.ts @@ -162,7 +162,7 @@ export interface IUser extends UserDetails { * * @param permission - The permission to check for. * @throws {Error} If the given permission does not exist. - * @throws {libErrors.PermissionError} if the user does noh have the given permission. + * @throws {libErrors.PermissionDeniedError} if the user does noh have the given permission. */ checkPermission(permission: PermissionName): void } diff --git a/packages/lib/src/data/messages_user.ts b/packages/lib/src/data/messages_user.ts index 34040965..4564796a 100644 --- a/packages/lib/src/data/messages_user.ts +++ b/packages/lib/src/data/messages_user.ts @@ -2,7 +2,7 @@ import { Type, Static } from "@sinclair/typebox"; import UserDetails, { IUser } from "./UserDetails.js"; import { StringEnum, jsonArray, plainJson } from "./composites.js"; import { MessageRequest } from "./messages_core.js"; -import { PermissionError } from "../errors.js"; +import { ValidationError } from "../errors.js"; export class UserGetRequest { declare ["constructor"]: typeof UserGetRequest; @@ -277,9 +277,9 @@ export class UserBulkImportRequest { user.checkPermission("core.user.set_whitelisted"); break; default: - // Unvalidated input during permission check: deny, don't throw a bare Error + // Unvalidated input during permission check: reject, don't throw a bare Error // @ts-expect-error Unreachable - throw new PermissionError(`Unknown import / restore type: ${data.importType}`); + throw new ValidationError(`Unknown import / restore type: ${data.importType}`); } } } diff --git a/packages/lib/src/errors.ts b/packages/lib/src/errors.ts index f37ebc8f..a17eacdf 100644 --- a/packages/lib/src/errors.ts +++ b/packages/lib/src/errors.ts @@ -18,40 +18,76 @@ export class InstallationError extends Error { } /** - * Thrown from requests sent when an error occured handling it + * Base class for errors caused by the request, such as invalid input, + * missing permissions or a missing resource. Corresponds to the HTTP 4xx + * range. + * + * Link does not log these as unexpected when a request handler throws them. */ -export class RequestError extends Error { - constructor( - message: string, - public code = "RequestError", - public stack?: string, - ) { - super(message); - } -} +export class ClientError extends Error { } /** - * Thrown when a permission check fails. - * - * Is a subclass of RequestError to prevent logging stack traces when - * requests fail due to permission denied. - * + * Thrown when the request contains invalid data (HTTP 400) */ -export class PermissionError extends RequestError { - constructor( - message: string, - code = "PermissionError", - stack?: string, - ) { - super(message, code, stack); - } +export class ValidationError extends ClientError { } -} +/** + * Thrown when a config value cannot be used for its field + */ +export class InvalidValueError extends ValidationError { } + +/** + * Thrown when a config field does not exist + */ +export class InvalidFieldError extends ValidationError { } + +/** + * Thrown when the request is not allowed (HTTP 403) + */ +export class ForbiddenError extends ClientError { } + +/** + * Thrown when a permission check fails + */ +export class PermissionDeniedError extends ForbiddenError { } + +/** + * Thrown when a config field is not accessible from the current location + */ +export class InvalidAccessError extends ForbiddenError { } + +/** + * Thrown when a resource the request refers to does not exist (HTTP 404) + */ +export class NotFoundError extends ClientError { } + +/** + * Thrown when an operation for a request timed out (HTTP 408) + */ +export class TimeoutError extends ClientError { } + +/** + * Thrown when the request conflicts with the current state, such as an + * instance that is running or a host that is not connected (HTTP 409) + */ +export class ConflictError extends ClientError { } + +/** + * Base class for errors caused by the side handling the request. + * Corresponds to the HTTP 5xx range. + */ +export class ServerError extends Error { } + +/** + * Received in place of an error response whose class is not registered, + * such as a TypeError thrown by a request handler (HTTP 500) + */ +export class InternalServerError extends ServerError { } /** * Signal for messages that fail validation */ -export class InvalidMessage extends Error { +export class InvalidMessage extends ValidationError { code = "InvalidMessage"; constructor( message: string, diff --git a/packages/lib/src/link/link.ts b/packages/lib/src/link/link.ts index e29131c7..87616e44 100644 --- a/packages/lib/src/link/link.ts +++ b/packages/lib/src/link/link.ts @@ -3,7 +3,7 @@ import * as libData from "../data/index.js"; import * as libErrors from "../errors.js"; import { logger } from "../logging.js"; import * as libSchema from "../schema.js"; -import { dataClasses } from "./messages.js"; +import { dataClasses, errorClasses } from "./messages.js"; import { BaseConnector, WebSocketBaseConnector } from "./connectors.js"; import { strict as assert } from "assert"; import type { PluginNodeEnvInfo, PluginWebpackEnvInfo } from "../plugin.js"; @@ -38,6 +38,12 @@ export interface Event { } export type EventClass = Event["constructor"]; +export interface ErrorClass { + new (message: string): Error; + name: string; + plugin?: string; +} + export interface RequestEntry { Request: RequestClass; name: string, @@ -156,7 +162,7 @@ export class Link { return this._pendingRequests.size + this._forwardedRequests.size; } - _clearPendingRequests(err: Error & { code?: string }) { + _clearPendingRequests(err: Error) { for (let pending of this._pendingRequests.values()) { pending.reject(err); } @@ -170,9 +176,7 @@ export class Link { ) { continue; } - pending.origin.connector.sendResponseError( - new libData.ResponseError(err.message, err.code), pending.src, pending.dst - ); + pending.origin.connector.sendResponseError(Link.errorToResponse(err, false), pending.src, pending.dst); } this._pendingRequests.clear(); this._forwardedRequests.clear(); @@ -200,10 +204,10 @@ export class Link { try { this.validatePermission(permissionMessage, entry); // Somewhat hacky, defined in ControlConnection } catch (err) { - // A non-PermissionError means the permission check itself is + // A non-ClientError means the permission check itself is // broken. validatePermission (control connections only) already // sent a response error, so log and drop instead of crashing. - if (!(err instanceof libErrors.PermissionError)) { + if (!(err instanceof libErrors.ClientError)) { logger.error( `Unexpected error checking permission for ${permissionMessage.name}:\n` + `${(err as Error).stack ?? (err as Error).message}` @@ -280,7 +284,7 @@ export class Link { this.validateIngress(message); } catch (err: any) { if (message.type === "request") { - this.connector.sendResponseError(new libData.ResponseError(err.message, err.code), message.src); + this.connector.sendResponseError(Link.errorToResponse(err, false), message.src); } throw err; } @@ -290,17 +294,17 @@ export class Link { let entry = Link._requestsByName.get(message.name); if (!entry) { let err = new libErrors.InvalidMessage(`Unrecognized request ${message.name}`); - this.connector.sendResponseError(new libData.ResponseError(err.message, err.code), message.src); + this.connector.sendResponseError(Link.errorToResponse(err, false), message.src); throw err; } if (!entry.allowedSrcTypes.has(message.src.type)) { let err = new libErrors.InvalidMessage(`Source ${message.src} is not allowed for ${message.name}`); - this.connector.sendResponseError(new libData.ResponseError(err.message, err.code), message.src); + this.connector.sendResponseError(Link.errorToResponse(err, false), message.src); throw err; } if (!entry.allowedDstTypes.has(message.dst.type)) { let err = new libErrors.InvalidMessage(`Destination ${message.dst} is not allowed for ${message.name}`); - this.connector.sendResponseError(new libData.ResponseError(err.message, err.code), message.src); + this.connector.sendResponseError(Link.errorToResponse(err, false), message.src); throw err; } return entry; @@ -344,7 +348,7 @@ export class Link { * * @param message - Message to check. * @param entry - Request or Event entry for this Message. - * @throws {libErrors.PermissionError} if unauthorized. + * @throws {libErrors.PermissionDeniedError} if unauthorized. */ validatePermission(message: libData.MessageRequest | libData.MessageEvent, entry: RequestEntry | EventEntry) { } @@ -356,7 +360,7 @@ export class Link { ); if (message.type === "request") { this.connector.sendResponseError( - new libData.ResponseError(err.message, err.code), (message as libData.MessageRequest).src + Link.errorToResponse(err, false), (message as libData.MessageRequest).src ); } throw err; @@ -399,13 +403,7 @@ export class Link { try { response = handler(entry.requestFromJSON(message.data), message.src, message.dst); } catch (err: any) { - if (err.errors) { - logger.error(JSON.stringify(err.errors, null, "\t")); - } - this.connector.sendResponseError( - new libData.ResponseError(err.message, err.code, err.stack), message.src - ); - return; + response = Promise.reject(err); } if (!(response instanceof Promise)) { @@ -424,19 +422,17 @@ export class Link { this.connector.sendResponse(result, message.src, spoofedSrc); } ).catch( - (err: Error & { code?: string }) => { + (err: Error) => { if (err instanceof libErrors.InvalidMessage) { logger.error(err.message); if (err.errors) { logger.error(JSON.stringify(err.errors, null, "\t")); } - } else if (!(err instanceof libErrors.RequestError)) { + } else if (Link.errorName(err) === undefined) { logger.error(`Unexpected error responding to ${message.name}:\n${err.stack}`); } try { - this.connector.sendResponseError( - new libData.ResponseError(err.message, err.code, err.stack), message.src, spoofedSrc - ); + this.connector.sendResponseError(Link.errorToResponse(err), message.src, spoofedSrc); } catch (subErr: any) { logger.error(`Unexpected error sending error response for ${message.name}:\n${subErr.stack}`); } @@ -474,7 +470,7 @@ export class Link { } this._pendingRequests.delete(message.dst.requestId!); - pending.reject(new libErrors.RequestError(message.data.message, message.data.code, message.data.stack)); + pending.reject(Link.errorFromResponse(message.data)); } _processEvent(message: libData.MessageEvent, entry: EventEntry) { @@ -823,11 +819,81 @@ export class Link { this._eventsByName.set(name, entry); this._eventsByClass.set(Event as EventClass, entry); } + + static _errorsByName = new Map(); + static _errorNamesByClass = new Map(); + + /** + * Register an error class to be sent and received over links + * + * Error responses carry the registered name of the thrown error and the + * receiving side rejects the request with an instance of the same class. + * + * @param Class - Error class, must take the message as the first + * constructor argument. + */ + static registerError(Class: ErrorClass) { + const name = Class.plugin ? `${Class.plugin}:${Class.name}` : Class.name; + if (this._errorsByName.has(name)) { + throw new Error(`Error ${name} is already registered`); + } + this._errorsByName.set(name, Class); + this._errorNamesByClass.set(Class, name); + } + + /** + * Registered name of the closest registered class of an error + * + * @param err - Error to look up. + * @returns name or undefined if no class in its prototype chain is registered. + */ + static errorName(err: Error) { + for (let proto = Object.getPrototypeOf(err); proto; proto = Object.getPrototypeOf(proto)) { + const name = this._errorNamesByClass.get(proto.constructor); + if (name) { + return name; + } + } + return undefined; + } + + /** + * Convert an error into the data of an error response + * + * Errors with no registered class are sent as InternalServerError. + * + * @param err - Error to convert. + * @param withStack - Include the stack trace of the error. + */ + static errorToResponse(err: Error, withStack = true) { + return new libData.ResponseError( + err.message, this.errorName(err) ?? "InternalServerError", withStack ? err.stack : undefined + ); + } + + /** + * Convert the data of an error response back into an error + * + * @param response - Received error response data. + * @returns instance of the registered class, or InternalServerError if + * the class is not registered on this side. + */ + static errorFromResponse(response: libData.ResponseError) { + const Class = response.code && this._errorsByName.get(response.code) || libErrors.InternalServerError; + const err = new Class(response.message); + if (response.stack !== undefined) { + err.stack = response.stack; + } + return err; + } } for (let Class of dataClasses) { Link.register(Class as RequestClass); } +for (let Class of errorClasses) { + Link.registerError(Class); +} export function registerPluginMessages(pluginInfos: PluginNodeEnvInfo[]|PluginWebpackEnvInfo[]) { for (let pluginInfo of pluginInfos) { @@ -840,5 +906,14 @@ export function registerPluginMessages(pluginInfos: PluginNodeEnvInfo[]|PluginWe } Link.register(Class); } + for (let Class of pluginInfo.errors || []) { + if (Class.plugin !== pluginInfo.name) { + throw new Error( + `Expected error ${Class.name} from ${pluginInfo.name} to have a static ` + + `plugin property set to "${pluginInfo.name}"` + ); + } + Link.registerError(Class); + } } } diff --git a/packages/lib/src/link/messages.ts b/packages/lib/src/link/messages.ts index 117e4718..8b7d77d9 100644 --- a/packages/lib/src/link/messages.ts +++ b/packages/lib/src/link/messages.ts @@ -8,7 +8,8 @@ import * as role from "../data/messages_role.js"; import * as note from "../data/messages_note.js"; import * as plugin from "../data/messages_plugin.js"; import * as subscriptions from "../subscriptions.js"; -import type { RequestClass, EventClass } from "./link.js"; +import * as errors from "../errors.js"; +import type { RequestClass, EventClass, ErrorClass } from "./link.js"; /** * Event and Request payloads registered with the Link class. Used to decode @@ -151,3 +152,25 @@ export const dataClasses: (RequestClass | EventClass) plugin.PluginLatestVersionsRequest, plugin.UpdateAllRequest, ]; + +/** + * Error classes registered with the Link class. Used to send errors thrown + * by request handlers back as instances of the same class. + * @internal + */ +export const errorClasses: ErrorClass[] = [ + errors.ClientError, + errors.ValidationError, + errors.InvalidValueError, + errors.InvalidFieldError, + errors.ForbiddenError, + errors.PermissionDeniedError, + errors.InvalidAccessError, + errors.NotFoundError, + errors.TimeoutError, + errors.ConflictError, + errors.ServerError, + errors.InternalServerError, + errors.InvalidMessage, + errors.SessionLost, +]; diff --git a/packages/lib/src/plugin.ts b/packages/lib/src/plugin.ts index 7320b608..0acbb84c 100644 --- a/packages/lib/src/plugin.ts +++ b/packages/lib/src/plugin.ts @@ -8,6 +8,7 @@ import type { ConfigSchema, FieldDefinition } from "./config/index.js"; import type { PermissionDefinition } from "./permissions.js"; import type { PlayerStats, Role } from "./data/index.js"; import type { SyncHook } from "./SyncHook.js"; +import type { ErrorClass } from "./link/index.js"; export const PluginFeatureFlags = [ /** The plugin requires module code to be patched into the save */ @@ -42,6 +43,7 @@ export type PluginDeclaration = { features?: (typeof PluginFeatureFlags)[number][]; messages?: any[]; + errors?: ErrorClass[]; permissions?: PermissionDefinition[]; routes?: string[]; } diff --git a/packages/lib/src/rce_ops.ts b/packages/lib/src/rce_ops.ts index e030d57a..617fe051 100644 --- a/packages/lib/src/rce_ops.ts +++ b/packages/lib/src/rce_ops.ts @@ -2,7 +2,7 @@ import util from "util"; import path from "path"; import { exec } from "child_process"; import { logger } from "./logging.js"; -import { RequestError } from "./errors.js"; +import { ConflictError, NotFoundError, UnavailableError } from "./errors.js"; import type { PluginNodeEnvInfo } from "./plugin.js"; import { isNonRegistryPackage } from "./load_plugin_list.js"; import { LockFile, LockFileExistsError, LockFileRecoveryError } from "./LockFile.js"; @@ -31,7 +31,7 @@ let operationInProgress = false; async function logExec(cmd: string, check?: () => Promise) { // Checked and set before the first await so concurrent calls in this process can't interleave if (operationInProgress) { - throw new RequestError(operationInProgressMessage); + throw new ConflictError(operationInProgressMessage); } operationInProgress = true; const lockPath = path.resolve("npm-operation.lock"); @@ -43,13 +43,13 @@ async function logExec(cmd: string, check?: () => Promise) { await lock.acquire(); } catch (err) { if (err instanceof LockFileRecoveryError) { - throw new RequestError(err.message); + throw new UnavailableError(err.message); } if (!(err instanceof LockFileExistsError)) { throw err; } const holder = await lock.holderPid(); - throw new RequestError( + throw new ConflictError( `${operationInProgressMessage}${holder ? ` (${lockPath} is held by process ${holder})` : ""}` ); } @@ -90,7 +90,7 @@ export async function handleUpdateAll(corePackage: string, pluginInfos: PluginNo /** Throws if the dependency by this name is a path, git, url or alias that installing by name would replace */ async function checkNotLocalDependency(name: string) { if (await isNonRegistryPackage(name)) { - throw new RequestError(`${name} is not installed from npm on this machine`); + throw new ConflictError(`${name} is not installed from npm on this machine`); } } @@ -98,7 +98,7 @@ export async function handlePluginUpdate(pluginName: string, pluginInfos: Plugin // Optionally pinned to a version as name@version to update past the range in package.json const { name, version } = parsePackageSpec(pluginName); if (!pluginInfos.some(plugin => plugin.npmPackage === name)) { - throw new RequestError(`Plugin ${name} is not installed on this machine`); + throw new NotFoundError(`Plugin ${name} is not installed on this machine`); } if (version === undefined) { diff --git a/packages/lib/src/shared_commands.ts b/packages/lib/src/shared_commands.ts index d526a7f6..24eb1f78 100644 --- a/packages/lib/src/shared_commands.ts +++ b/packages/lib/src/shared_commands.ts @@ -9,6 +9,7 @@ import { spawn } from "child_process"; import { loadPluginList } from "./load_plugin_list.js"; import * as libConfig from "./config/index.js"; import * as libFileOps from "./file_ops.js"; +import { InvalidFieldError, InvalidValueError } from "./errors.js"; import { logger } from "./logging.js"; import * as libHelpers from "./helpers.js"; import { LockFile } from "./LockFile.js"; @@ -271,7 +272,7 @@ export async function handleConfigCommand( try { print(instance.get(args.field as string)); } catch (err) { - if (err instanceof libConfig.InvalidField) { + if (err instanceof InvalidFieldError) { logger.error(err.message); } else { throw err; @@ -292,7 +293,7 @@ export async function handleConfigCommand( instance.set(args.field as string, args.value); await instance.save(); } catch (err) { - if (err instanceof libConfig.InvalidField || err instanceof libConfig.InvalidValue) { + if (err instanceof InvalidFieldError || err instanceof InvalidValueError) { logger.error(err.message); } else { throw err; diff --git a/packages/lib/src/subscriptions.ts b/packages/lib/src/subscriptions.ts index 6a1da6e6..6fbb8710 100644 --- a/packages/lib/src/subscriptions.ts +++ b/packages/lib/src/subscriptions.ts @@ -3,7 +3,7 @@ import { Link, Event, EventClass, RequestHandler, WebSocketBaseConnector } from import { Address, MessageRequest, IUser, JsonBoolean, StringEnum } from "./data/index.js"; import isDeepStrictEqual from "#is_deep_strict_equal"; import type { PermissionName } from "./permissions.js"; -import { RequestError } from "./errors.js"; +import { SessionLost, ValidationError } from "./errors.js"; import { logger } from "./logging.js"; export type SubscriptionRequestHandler = RequestHandler | null>; @@ -351,7 +351,7 @@ export class SubscriptionController { } } if (count > this._maxFilters) { - throw new RequestError(`Subscription exceeds the limit of ${this._maxFilters} filters`); + throw new ValidationError(`Subscription exceeds the limit of ${this._maxFilters} filters`); } } @@ -685,7 +685,7 @@ export class EventSubscriber { this._notify(null); } } catch (err: any) { - if (!(err instanceof RequestError) || err.code !== "SessionLost") { + if (!(err instanceof SessionLost)) { logger.error(`Unexpected error updating ${entry.name} subscription:\n${err.stack}`); } if (err instanceof Error) { diff --git a/plugins/player_auth/controller.ts b/plugins/player_auth/controller.ts index 38b6d947..574d4ef7 100644 --- a/plugins/player_auth/controller.ts +++ b/plugins/player_auth/controller.ts @@ -5,7 +5,7 @@ import jwt from "jsonwebtoken"; import { Static } from "@sinclair/typebox"; import type { ControllerPluginContext } from "@clusterio/controller"; -import { basicType, RequestError } from "@clusterio/lib"; +import { basicType, NotFoundError } from "@clusterio/lib"; import { FetchPlayerCodeRequest, PlayerAuthServer, SetVerifyCodeRequest } from "./messages.js"; @@ -103,7 +103,7 @@ export default async function loadControllerPlugin(context: ControllerPluginCont let entry = players.get(player); if (!entry || entry.expiresMs < Date.now()) { - throw new RequestError("invalid player"); + throw new NotFoundError("invalid player"); } entry.verifyCode = verifyCode; diff --git a/plugins/player_auth/test/plugin.js b/plugins/player_auth/test/plugin.js index 45043563..d419bad5 100644 --- a/plugins/player_auth/test/plugin.js +++ b/plugins/player_auth/test/plugin.js @@ -332,14 +332,14 @@ describe("player_auth", function() { it("should throw if player does not exist", async function() { await assert.rejects( setVerifyCode(new SetVerifyCodeRequest("invalid", "invalid")), - new lib.RequestError("invalid player") + new lib.NotFoundError("invalid player") ); }); it("should throw if player code has expired", async function() { await addPlayer("expired", { timeout: -1 }); await assert.rejects( setVerifyCode(new SetVerifyCodeRequest("expired", "expired")), - new lib.RequestError("invalid player") + new lib.NotFoundError("invalid player") ); }); }); diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index a6f88b76..166e79c7 100644 --- a/test/controller/ControlConnection.test.js +++ b/test/controller/ControlConnection.test.js @@ -139,7 +139,7 @@ describe("controller/src/ControlConnection", function() { it("rejects instance.assigned_host in the config", async function() { await assert.rejects( create({ "instance.id": 4001, "instance.name": "c1", "instance.assigned_host": 10 }), - new lib.RequestError("instance.assigned_host must be set through the assign-host interface"), + new lib.ValidationError("instance.assigned_host must be set through the assign-host interface"), ); assert.equal(mockController.instances.has(4001), false); }); @@ -197,7 +197,7 @@ describe("controller/src/ControlConnection", function() { await connection.handleLogSetSubscriptionsRequest( new lib.LogSetSubscriptionsRequest(true, false, [], []) ); - connection.user = { checkPermission() { throw new lib.PermissionError("Permission denied"); } }; + connection.user = { checkPermission() { throw new lib.PermissionDeniedError("Permission denied"); } }; connection.checkLogPermission(); assert.equal(mockController.clusterLogger.transports.size, 0); assert.equal(connection.logTransport, null); diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 35a0cf4b..31a66272 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -9,7 +9,7 @@ import express from "express"; import { fileURLToPath } from "node:url"; import { - ControllerConfig, Address, RequestError, + ControllerConfig, Address, ConflictError, NotFoundError, InstanceConfig, SystemInfo, Role, ModPack, AccountUpdateEvent, addPluginConfigFields, renamePermissionPrefix, } from "@clusterio/lib"; @@ -254,33 +254,33 @@ describe("controller/src/Controller", function() { it("should error on invalid controller", function() { assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ controlId: 99 })), - new RequestError("Target control connection does not exist.") + new NotFoundError("Target control connection does not exist.") ); }); it("should error on invalid instance", function() { assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ instanceId: 99 })), - new RequestError("Instance with ID 99 does not exist") + new NotFoundError("Instance with ID 99 does not exist") ); }); it("should error on unassigned instance", function() { mockInstanceConfig.set("instance.assigned_host", null); assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ instanceId: 100 })), - new RequestError("Instance is not assigned to a host") + new ConflictError("Instance is not assigned to a host") ); }); it("should error on instance with disconnected host", function() { mockInstanceConfig.set("instance.assigned_host", 99); assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ instanceId: 100 })), - new RequestError("Host containing instance is not connected") + new ConflictError("Host containing instance is not connected") ); }); it("should error on invalid host", function() { assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ hostId: 99 })), - new RequestError("Host is not connected") + new ConflictError("Host is not connected") ); }); it("should error on invalid broadcast", function() { diff --git a/test/controller/InstanceManager.test.js b/test/controller/InstanceManager.test.js index 237b3dca..b44576f8 100644 --- a/test/controller/InstanceManager.test.js +++ b/test/controller/InstanceManager.test.js @@ -101,7 +101,7 @@ describe("controller/InstanceManager", function () { }); it("should throw when missing", function () { - assert.throws(() => instances.getForRequest(999), lib.RequestError); + assert.throws(() => instances.getForRequest(999), lib.NotFoundError); }); }); @@ -148,7 +148,7 @@ describe("controller/InstanceManager", function () { await assert.rejects( () => instances.createInstance(config), - lib.RequestError + lib.ConflictError ); }); @@ -172,7 +172,7 @@ describe("controller/InstanceManager", function () { it("should throw if host not connected", async function () { await assert.rejects( () => instances.assignInstance(1, 5), - lib.RequestError + lib.ConflictError ); }); @@ -204,7 +204,7 @@ describe("controller/InstanceManager", function () { await assert.rejects( () => instances.assignInstance(1, 6), - lib.RequestError + lib.ConflictError ); assert.equal(unassignRequest, undefined); @@ -312,7 +312,7 @@ describe("controller/InstanceManager", function () { controller.wsServer.hostConnections.set(6, { send: async (request) => { if (failAssign) { - throw new lib.RequestError("EACCES: permission denied"); + throw new lib.InternalServerError("EACCES: permission denied"); } assignRequest = request; }, diff --git a/test/host/Host.test.js b/test/host/Host.test.js index 84c01b8a..9ec08e08 100644 --- a/test/host/Host.test.js +++ b/test/host/Host.test.js @@ -37,8 +37,8 @@ describe("host/src/Host", function() { describe("class HostRouter", function() { describe(".wakeInstance()", function() { it("should log the message of a start error that has no stack", async function() { - // RequestError leaves its stack undefined - const err = new lib.RequestError("No available port left to assign from host.factorio_port_range"); + const err = new lib.ConflictError("No available port left to assign from host.factorio_port_range"); + err.stack = undefined; const mockHost = { _shuttingDown: false, _connectInstance: async () => { throw err; }, @@ -57,6 +57,7 @@ describe("host/src/Host", function() { lib.logger.error = originalError; } assert.equal(responses.length, 1); + assert.equal(responses[0].code, "ConflictError"); assert.equal(logged.length, 1); assert.match(logged[0], /No available port left to assign/); }); diff --git a/test/host/Instance.js b/test/host/Instance.js index 2c39a80e..d4bd4544 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -284,11 +284,11 @@ describe("class Instance", function() { instance.server.version = "1.1.91"; assert.throws( () => instance.checkModPackVersion(modPack("1.1.110")), - new lib.RequestError( + new lib.ConflictError( "Mod pack pack is for Factorio 1.1.110 which is newer than the Factorio 1.1.91 this instance runs" ) ); - assert.throws(() => instance.checkModPackVersion(modPack("2.0")), lib.RequestError); + assert.throws(() => instance.checkModPackVersion(modPack("2.0")), lib.ConflictError); }); it("should skip the check when the server version is unknown", function() { @@ -300,7 +300,7 @@ describe("class Instance", function() { it("should reject a start save outside the saves directory", async function() { await assert.rejects( instance.handleInstanceStartRequest(new lib.InstanceStartRequest("../escape.zip")), - lib.RequestError, + lib.ValidationError, ); }); it("should reject a created save outside the saves directory", async function() { @@ -308,7 +308,7 @@ describe("class Instance", function() { instance.handleInstanceCreateSaveRequest( new lib.InstanceCreateSaveRequest("../escape.zip"), ), - lib.RequestError, + lib.ValidationError, ); }); }); diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 6c7efe82..e2c74fec 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -744,7 +744,7 @@ describe("Integration of Clusterio", function() { slowTest(this); await assert.rejects( execCtl("instance create testClone --id 440 --from 441"), - new lib.RequestError("Instance with ID 441 does not exist") + new lib.NotFoundError("Instance with ID 441 does not exist") ); }); }); diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index c953ea13..ef8299bb 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -652,14 +652,14 @@ describe("lib/config/classes", function() { let testInstance = new TestConfig("local"); assert.throws( () => testInstance.get("test.priv", "remote"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); it("should throw if field is locally inaccessible", function() { let testInstance = new TestConfig("remote"); assert.throws( () => testInstance.get("test.priv"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); }); @@ -677,20 +677,20 @@ describe("lib/config/classes", function() { it("should throw if field does not exist", function() { assert.throws( () => set("test.bar", 1), - new lib.InvalidField("No field named 'test.bar'") + new lib.InvalidFieldError("No field named 'test.bar'") ); }); it("should throw if field is remotely inaccessible", function() { assert.throws( () => set("test.priv", "bad", "remote"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); it("should throw if field is locally inaccessible", function() { replaceConfig(new TestConfig("remote")); assert.throws( () => set("test.priv", "bad", "local"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); it("should throw if field is not in enum", function() { @@ -731,7 +731,7 @@ describe("lib/config/classes", function() { assert.throws( () => set("test.bool", "blah"), - new lib.InvalidValue("Expected type of test.bool to be boolean, not string") + new lib.InvalidValueError("Expected type of test.bool to be boolean, not string") ); }); it("should auto convert string to number if possible", function() { @@ -749,7 +749,7 @@ describe("lib/config/classes", function() { assert.throws( () => set("test.func", "blah"), - new lib.InvalidValue("Expected type of test.func to be number, not string") + new lib.InvalidValueError("Expected type of test.func to be number, not string") ); }); it("should auto convert string to object", function() { @@ -764,7 +764,7 @@ describe("lib/config/classes", function() { } assert.throws( () => set("test.json", "blah"), - new lib.InvalidValue(`Error parsing value for test.json: ${errMsg}`) + new lib.InvalidValueError(`Error parsing value for test.json: ${errMsg}`) ); }); } @@ -815,7 +815,7 @@ describe("lib/config/classes", function() { it("should throw if validator fails", function() { assert.throws( () => testInstance.set("test.validated", -1), - new lib.InvalidValue("Failed validation of test.validated: must be positive") + new lib.InvalidValueError("Failed validation of test.validated: must be positive") ); assert.deepEqual(TestConfig.validatorCalls, [ ["test.validated", -1], // set test.validated @@ -835,7 +835,7 @@ describe("lib/config/classes", function() { it("should throw if dependent validation fails", function() { assert.throws( () => testInstance.set("test.base", 5), - new lib.InvalidValue("Failed validation of dependent test.dependent: dependent must be >= base") + new lib.InvalidValueError("Failed validation of dependent test.dependent: dependent must be >= base") ); assert.deepEqual(TestConfig.validatorCalls, [ ["test.dependent", 1], // set test.base @@ -853,26 +853,26 @@ describe("lib/config/classes", function() { it("should throw if field does not exist", function() { assert.throws( () => setProp("test.bar", 1), - new lib.InvalidField("No field named 'test.bar'") + new lib.InvalidFieldError("No field named 'test.bar'") ); }); it("should throw if field is remotely inaccessible", function() { assert.throws( () => setProp("test.priv", "prop", "bad", "remote"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); it("should throw if field is locally inaccessible", function() { replaceConfig(new TestConfig("remote")); assert.throws( () => setProp("test.priv", "prop", "bad", "local"), - new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + new lib.InvalidAccessError("Field 'test.priv' is not accessible from remote") ); }); it("should throw if field is not an object", function() { assert.throws( () => setProp("test.enum", "prop", "a"), - new lib.InvalidField("Cannot set property on non-object field 'test.enum'") + new lib.InvalidFieldError("Cannot set property on non-object field 'test.enum'") ); }); it("should work if field is an object", function() { @@ -939,7 +939,7 @@ describe("lib/config/classes", function() { it("should throw if object validator fails", function() { assert.throws( () => testInstance.setProp("test.objValidated", "bad", true), - new lib.InvalidValue("Failed validation of test.objValidated: bad key") + new lib.InvalidValueError("Failed validation of test.objValidated: bad key") ); assert.deepEqual(TestConfig.validatorCalls, [ ["test.objValidated", { bad: true }], // setProp test.objValidated bad @@ -957,7 +957,7 @@ describe("lib/config/classes", function() { testInstance.set("test.base", 0); assert.throws( () => testInstance.setProp("test.objDependent", "base", 0), - new lib.InvalidValue("Failed validation of dependent test.dependent: objDependent cannot be 0") + new lib.InvalidValueError("Failed validation of dependent test.dependent: objDependent cannot be 0") ); assert.deepEqual(TestConfig.validatorCalls, [ ["test.dependent", 1], // set test.base @@ -1065,7 +1065,7 @@ describe("lib/config/classes", function() { assert.throws( () => testInstance.commitStaging(), - new lib.InvalidValue("Failed validation of test.validated: must be positive") + new lib.InvalidValueError("Failed validation of test.validated: must be positive") ); assert.deepEqual(TestConfig.validatorCalls, [ // commitStaging, all values are checked once diff --git a/test/lib/data/messages_user.test.js b/test/lib/data/messages_user.test.js index 79c7363d..e4e17cbf 100644 --- a/test/lib/data/messages_user.test.js +++ b/test/lib/data/messages_user.test.js @@ -10,11 +10,11 @@ describe("lib/data/messages_user", function() { return { checked, checkPermission(permission) { checked.push(permission); } }; } - it("should deny an unknown importType with a PermissionError", function() { + it("should deny an unknown importType with a ValidationError", function() { const user = grantAllUser(); assert.throws( () => lib.UserBulkImportRequest.permission(user, { data: { importType: "foo", users: [] } }), - lib.PermissionError, + lib.ValidationError, ); }); diff --git a/test/lib/link/link.js b/test/lib/link/link.js index 5a02dc10..f0584e7d 100644 --- a/test/lib/link/link.js +++ b/test/lib/link/link.js @@ -345,7 +345,7 @@ describe("lib/link/link", function() { assert.deepEqual( await message, [new lib.MessageResponseError( - 1, src, dst, new lib.ResponseError("Error", undefined, "Error") + 1, src, dst, new lib.ResponseError("Error", "InternalServerError", "Error") )] ); }); @@ -361,7 +361,7 @@ describe("lib/link/link", function() { 1, src, dst, new lib.ResponseError( "Request NumberRequest failed validation", - response.data.code, + "InvalidMessage", response.data.stack, ) ) @@ -550,20 +550,20 @@ describe("lib/link/link", function() { return { connector, link, message }; } - it("should silently deny on PermissionError from the permission check", function() { + it("should silently deny on ClientError from the permission check", function() { const { connector, link, message } = controlSetup(); let handled = false; let errored = false; connector.on("error", () => { errored = true; }); link.handle(SimpleRequest, async () => { handled = true; }); - link.validatePermission = () => { throw new lib.PermissionError("Denied"); }; + link.validatePermission = () => { throw new lib.PermissionDeniedError("Denied"); }; connector.emit("message", message); assert(!handled, "handler ran despite denial"); assert(!errored, "connector emitted error on denial"); assert.deepEqual(connector.sentMessages, [], "denial sent a message"); }); - it("should not crash when the permission check throws a non-PermissionError", function() { + it("should not crash when the permission check throws a non-ClientError", function() { const { connector, link, message } = controlSetup(); let handled = false; let errored = false; @@ -578,7 +578,7 @@ describe("lib/link/link", function() { } finally { lib.logger.error = originalError; } - assert(!errored, "non-PermissionError escaped to the connector error path"); + assert(!errored, "non-ClientError escaped to the connector error path"); assert(!handled, "handler ran after the permission check threw"); assert.equal(logged.length, 1); assert(logged[0].startsWith("Unexpected error checking permission for SimpleRequest:\n")); @@ -664,6 +664,109 @@ describe("lib/link/link", function() { }); }); + describe("Error responses", function() { + class ErrorTestRequest { + static type = "request"; + static src = "control"; + static dst = "host"; + static permission = null; + } + lib.Link.register(ErrorTestRequest); + class TestPluginError extends lib.ConflictError { + static plugin = "error_test"; + } + lib.registerPluginMessages([{ name: "error_test", errors: [TestPluginError] }]); + + // control 1 <-> controller <-> host 1, with the controller forwarding between the two + function forwardingSetup() { + const [clientConnector, controlConnector] = lib.VirtualConnector.makePair( + addr({ controlId: 1 }), addr("controller") + ); + const [hostSideConnector, hostConnector] = lib.VirtualConnector.makePair( + addr("controller"), addr({ hostId: 1 }) + ); + const client = new lib.Link(clientConnector); + const controlSide = new lib.Link(controlConnector); + const hostSide = new lib.Link(hostSideConnector); + const host = new lib.Link(hostConnector); + controlSide.router = { forwardMessage(origin, message) { + hostSide.forwardRequest(message, origin); + return true; + } }; + hostSide.router = { forwardMessage(origin, message) { + controlSide.connector.forward(message); + return true; + } }; + return { client, host }; + } + + async function rejectionFor(err) { + const { client, host } = forwardingSetup(); + host.handle(ErrorTestRequest, async () => { throw err; }); + const logged = []; + const originalError = lib.logger.error; + lib.logger.error = msg => { logged.push(msg); }; + try { + await client.sendTo({ hostId: 1 }, new ErrorTestRequest()); + } catch (received) { + return { received, logged }; + } finally { + lib.logger.error = originalError; + } + throw new Error("Request did not reject"); + } + + it("should reject with the class thrown by the handler", async function() { + const err = new lib.NotFoundError("Thing does not exist"); + const { received, logged } = await rejectionFor(err); + assert(received instanceof lib.NotFoundError); + assert.equal(received.message, "Thing does not exist"); + assert.equal(received.stack, err.stack); + assert.deepEqual(logged, []); + }); + it("should reject with a registered plugin error class", async function() { + const { received, logged } = await rejectionFor(new TestPluginError("Plugin conflict")); + assert(received instanceof TestPluginError); + assert.equal(received.message, "Plugin conflict"); + assert.deepEqual(logged, []); + }); + it("should reject with the closest registered class of an unregistered error", async function() { + class UnregisteredError extends lib.ConflictError { } + const { received } = await rejectionFor(new UnregisteredError("Busy")); + assert.equal(received.constructor, lib.ConflictError); + }); + it("should reject with InternalServerError and log for unregistered errors", async function() { + const err = new TypeError("x is not a function"); + const { received, logged } = await rejectionFor(err); + assert(received instanceof lib.InternalServerError); + assert.equal(received.message, "x is not a function"); + assert.equal(received.stack, err.stack); + assert.equal(logged.length, 1); + assert(logged[0].startsWith("Unexpected error responding to ErrorTestRequest:\n")); + }); + it("should reject with InternalServerError for an unknown error name", async function() { + const received = lib.Link.errorFromResponse(new lib.ResponseError("Gone", "other_plugin:GoneError")); + assert(received instanceof lib.InternalServerError); + assert.equal(received.message, "Gone"); + }); + it("should require plugin errors to set the plugin property", function() { + class MissingPluginError extends lib.ClientError { } + assert.throws( + () => lib.registerPluginMessages([{ name: "error_test_2", errors: [MissingPluginError] }]), + new Error( + "Expected error MissingPluginError from error_test_2 to have a static plugin property " + + "set to \"error_test_2\"" + ) + ); + }); + it("should throw when registering an error name twice", function() { + assert.throws( + () => lib.Link.registerError(lib.NotFoundError), + new Error("Error NotFoundError is already registered") + ); + }); + }); + describe(".snoopEvent()", function() { it("should snoop an event", function() { let handled = false; diff --git a/test/lib/subscriptions.test.js b/test/lib/subscriptions.test.js index dcd03a81..64cfdd2a 100644 --- a/test/lib/subscriptions.test.js +++ b/test/lib/subscriptions.test.js @@ -560,7 +560,7 @@ describe("lib/subscriptions", function() { checkPermission(permission) { if (permission === this.denied) { - throw new lib.PermissionError("Permission denied"); + throw new lib.PermissionDeniedError("Permission denied"); } } } @@ -794,7 +794,7 @@ describe("lib/subscriptions", function() { const request1 = new lib.SubscriptionRequest(RegisteredEvent.name, "subscribe", 0, "baz"); await assert.rejects( ownSubscriptions.handleRequest(link, request1, connectorData.dst, connectorData.src), - lib.RequestError, + lib.ValidationError, ); ownSubscriptions.broadcast(new RegisteredEvent(), "baz"); @@ -973,7 +973,7 @@ describe("lib/subscriptions", function() { ); await assert.rejects( ownSubscriptions.handleRequest(link, request, connectorData.dst, connectorData.src), - lib.RequestError, + lib.ValidationError, ); }); }); @@ -1371,15 +1371,13 @@ describe("lib/subscriptions", function() { assert.notEqual(logged, undefined); }); - it("should not log error when sendTo throws SessionLost RequestError", async function() { + it("should not log error when sendTo throws SessionLost", async function() { let logged; const originalError = lib.logger.error; lib.logger.error = msg => { logged = msg; }; mockControl.sendTo = async function() { - const err = new lib.RequestError("Session lost"); - err.code = "SessionLost"; - throw err; + throw new lib.SessionLost("Session lost"); }; try { @@ -1402,26 +1400,26 @@ describe("lib/subscriptions", function() { assert.equal(eventSubscriber.synced, true); mockControl.sendTo = async function() { - throw new lib.PermissionError("Permission denied"); + throw new lib.PermissionDeniedError("Permission denied"); }; eventSubscriber.handleAccountEvent(); await new Promise(r => setImmediate(r)); assert.equal(eventSubscriber.synced, false); assert.deepEqual(calledWith, [null, false]); - assert.ok(eventSubscriber.error instanceof lib.PermissionError); + assert.ok(eventSubscriber.error instanceof lib.PermissionDeniedError); }); it("should expose errors through the snapshot", async function() { eventSubscriber.control.sendTo = function() { - throw new lib.PermissionError("Permission denied"); + throw new lib.PermissionDeniedError("Permission denied"); }; const unsubscribe = eventSubscriber.subscribe(() => {}); await new Promise(r => setImmediate(r)); const [,, error1] = eventSubscriber.getSnapshot(); - assert.ok(error1 instanceof lib.PermissionError); + assert.ok(error1 instanceof lib.PermissionDeniedError); assert.equal(error1.message, "Permission denied"); - assert.equal(error1.code, "PermissionError"); + assert.equal(lib.Link.errorName(error1), "PermissionDeniedError"); eventSubscriber.control.sendTo = function() {}; unsubscribe();