From 1c7c00b03309f7046f38b2ded0ad3075d2697757 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:08:44 +0000 Subject: [PATCH 1/3] Keep error classes across links Error responses now carry the registered name of the thrown error's class, and the requester rejects with an instance of that class instead of a generic RequestError. Error classes are registered like message classes, and plugins list their own in `errors`. RequestError is replaced by a hierarchy following the HTTP status codes: ClientError (ValidationError, ForbiddenError, NotFoundError, TimeoutError, ConflictError) and ServerError (InternalServerError). PermissionError is PermissionDeniedError and the config errors InvalidValue, InvalidField and InvalidAccess gain an Error suffix. Link logs only errors whose class is not registered. These arrive on the other side as InternalServerError with the original message and stack, so an error forwarded through the controller is logged once, where it was thrown. Co-Authored-By: Claude Opus 5.5 --- docs/communication.md | 1 + docs/config-system.md | 2 +- docs/devs/plugin-migration-to-2.0.md | 24 ++++ docs/devs/protocol.md | 3 +- docs/writing-plugins.md | 57 ++++++++- packages/controller/src/BaseConnection.ts | 10 +- packages/controller/src/ControlConnection.ts | 98 +++++++------- packages/controller/src/Controller.ts | 12 +- packages/controller/src/ControllerRouter.ts | 2 +- packages/controller/src/InstanceManager.ts | 16 +-- packages/controller/src/User.ts | 4 +- packages/controller/src/routes.ts | 2 +- packages/ctl/ctl.ts | 12 +- packages/host/src/Host.ts | 56 ++++---- packages/host/src/Instance.ts | 12 +- packages/lib/src/config/classes.ts | 93 +++++--------- packages/lib/src/data/UserDetails.ts | 2 +- packages/lib/src/data/messages_user.ts | 6 +- packages/lib/src/errors.ts | 86 +++++++++---- packages/lib/src/link/link.ts | 127 +++++++++++++++---- packages/lib/src/link/messages.ts | 25 +++- packages/lib/src/plugin.ts | 2 + packages/lib/src/rce_ops.ts | 12 +- packages/lib/src/shared_commands.ts | 5 +- packages/lib/src/subscriptions.ts | 6 +- plugins/player_auth/controller.ts | 4 +- plugins/player_auth/test/plugin.js | 4 +- test/controller/ControlConnection.test.js | 4 +- test/controller/Controller.js | 12 +- test/controller/InstanceManager.test.js | 10 +- test/host/Host.test.js | 5 +- test/host/Instance.js | 8 +- test/integration/clusterio.js | 2 +- test/lib/config/classes.js | 34 ++--- test/lib/data/messages_user.test.js | 4 +- test/lib/link/link.js | 115 ++++++++++++++++- test/lib/subscriptions.test.js | 22 ++-- 37 files changed, 589 insertions(+), 310 deletions(-) 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(); From 54e9340e616a72e2af8f86b6108a9fc6f6ad8605 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:39:02 +0000 Subject: [PATCH 2/3] Add UnavailableError and keep the PermissionError name Host not connected, host shutting down and instance not running are now UnavailableError (503) instead of ConflictError. The request error migration notes move to a new 2.1 guide, since this ships in 2.1. Co-Authored-By: Claude Opus 5.5 --- docs/decisions.md | 2 +- docs/devs/plugin-migration-to-2.0.md | 24 ------------------ docs/devs/plugin-migration-to-2.1.md | 26 ++++++++++++++++++++ docs/writing-plugins.md | 9 ++++--- packages/controller/src/ControlConnection.ts | 6 ++--- packages/controller/src/Controller.ts | 6 ++--- packages/controller/src/ControllerRouter.ts | 2 +- packages/controller/src/InstanceManager.ts | 4 +-- packages/controller/src/User.ts | 4 +-- packages/ctl/ctl.ts | 2 +- packages/host/src/Host.ts | 4 +-- packages/host/src/Instance.ts | 4 +-- packages/lib/src/data/UserDetails.ts | 2 +- packages/lib/src/errors.ts | 10 ++++++-- packages/lib/src/link/link.ts | 2 +- packages/lib/src/link/messages.ts | 3 ++- test/controller/ControlConnection.test.js | 2 +- test/controller/Controller.js | 6 ++--- test/controller/InstanceManager.test.js | 4 +-- test/lib/config/classes.js | 4 ++- test/lib/link/link.js | 2 +- test/lib/subscriptions.test.js | 12 ++++----- test/messages/note.test.js | 4 +-- 23 files changed, 78 insertions(+), 66 deletions(-) create mode 100644 docs/devs/plugin-migration-to-2.1.md diff --git a/docs/decisions.md b/docs/decisions.md index d96be0e2..8c2a8181 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -370,7 +370,7 @@ We aim to be highly transparent with breaking changes and maintain compatibility During the alpha stage every alpha was allowed to contain breaking changes, as long as they were documented and provided migration actions. From 2.0 onwards changes which require action from people running a cluster, such as changes to configs, stored data or the command line, are held back for the next major version. Changes to the plugin API may still be made in minor versions. -Plugin authors should check the changelog when a new minor version is released, non-trivial changes come with a migration guide such as [Migrating Plugins to 2.0](/docs/devs/plugin-migration-to-2.0.md). +Plugin authors should check the changelog when a new minor version is released, non-trivial changes come with a migration guide such as [Migrating Plugins to 2.1](/docs/devs/plugin-migration-to-2.1.md). ### Distribution diff --git a/docs/devs/plugin-migration-to-2.0.md b/docs/devs/plugin-migration-to-2.0.md index 0919fd6d..aae91383 100644 --- a/docs/devs/plugin-migration-to-2.0.md +++ b/docs/devs/plugin-migration-to-2.0.md @@ -305,30 +305,6 @@ 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/plugin-migration-to-2.1.md b/docs/devs/plugin-migration-to-2.1.md new file mode 100644 index 00000000..aac0a789 --- /dev/null +++ b/docs/devs/plugin-migration-to-2.1.md @@ -0,0 +1,26 @@ +# Migrating Plugins to 2.1 + +This guide covers the changes needed to bring a plugin written for 2.0 up to 2.1. +Plugins still on an alpha should start with the [2.0 guide](plugin-migration-to-2.0.md). + + +## 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, name taken etc.: `ConflictError`. +- Host not connected, instance not running: `UnavailableError`. +- 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`. + +The config errors `lib.InvalidValue`, `lib.InvalidField` and `lib.InvalidAccess` were renamed to `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. diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md index af781dea..d31d14e5 100644 --- a/docs/writing-plugins.md +++ b/docs/writing-plugins.md @@ -645,7 +645,7 @@ 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 a `ClientError` such as `PermissionDeniedError` for a normal denial. +Check any required payload shape before inspecting fields, and use `user.checkPermission(...)` or throw a `ClientError` such as `PermissionError` 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 @@ -756,16 +756,17 @@ Error │ │ ├── InvalidValueError │ │ └── InvalidFieldError │ ├── ForbiddenError (403) -│ │ ├── PermissionDeniedError +│ │ ├── PermissionError │ │ └── InvalidAccessError │ ├── NotFoundError (404) │ ├── TimeoutError (408) │ └── ConflictError (409) └── ServerError (5xx) - └── InternalServerError (500) + ├── InternalServerError (500) + └── UnavailableError (503) ``` -Throw a `ClientError` subclass when the request can't be done as asked, for example: +Throw a `ClientError` subclass when the request can't be done as asked, and `UnavailableError` when the host or instance that should handle it is not connected or not running, for example: ```js controller.handle(messages.Frobnicate, async request => { diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index e8c650f5..3772ceec 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -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.ConflictError("Source host is not connected to the controller"); + throw new lib.UnavailableError("Source host is not connected to the controller"); } let targetHostConnection = this._controller.wsServer.hostConnections.get(targetHostId); if (!targetHostConnection || targetHostConnection.connector.closing) { - throw new lib.ConflictError("Target host is not connected to the controller"); + throw new lib.UnavailableError("Target host is not connected to the controller"); } // Create stream to proxy from target to source @@ -997,7 +997,7 @@ export default class ControlConnection extends BaseConnection { async handleNoteSetRequest(request: lib.NoteSetRequest) { const { resourceType, resourceId, content } = request; if (!resourceType || !resourceId) { - throw new lib.RequestError("resourceType and resourceId must not be empty"); + throw new lib.ValidationError("resourceType and resourceId must not be empty"); } this._controller.setNote(resourceType, resourceId, content, this.user.name); } diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index e3924b63..4f319f67 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -1476,13 +1476,13 @@ export default class Controller { } connection = this.wsServer.hostConnections.get(hostId); if (!connection) { - throw new lib.ConflictError("Host containing instance is not connected"); + throw new lib.UnavailableError("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.ConflictError("Host is not connected"); + throw new lib.UnavailableError("Host is not connected"); } } else { @@ -1570,7 +1570,7 @@ export default class Controller { } let connection = this.wsServer.hostConnections.get(hostId); if (!connection) { - throw new lib.ConflictError("Host containing instance is not connected"); + throw new lib.UnavailableError("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 7fc579db..56cce404 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", "ConflictError"), message.src + new lib.ResponseError(msg || "Unroutable destination", "UnavailableError"), message.src ); return true; } diff --git a/packages/controller/src/InstanceManager.ts b/packages/controller/src/InstanceManager.ts index c8e08066..4323d59a 100644 --- a/packages/controller/src/InstanceManager.ts +++ b/packages/controller/src/InstanceManager.ts @@ -173,7 +173,7 @@ export default class InstanceManager { * the instance will be unassigned. * * @throws {module:lib.NotFoundError} if the instance does not exist. - * @throws {module:lib.ConflictError} if the target host is not connected. + * @throws {module:lib.UnavailableError} 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.ConflictError("Target host is not connected to the controller"); + throw new lib.UnavailableError("Target host is not connected to the controller"); } } diff --git a/packages/controller/src/User.ts b/packages/controller/src/User.ts index 9a03af48..c7d50147 100644 --- a/packages/controller/src/User.ts +++ b/packages/controller/src/User.ts @@ -1,7 +1,7 @@ import { IUser, Role, SubscribableDatastore, - PermissionDeniedError, PermissionName, permissions, + PermissionError, PermissionName, permissions, } from "@clusterio/lib"; import UserRecord from "./UserRecord.js"; @@ -141,6 +141,6 @@ export default class User extends UserRecord implements IUser { } } - throw new PermissionDeniedError("Permission denied"); + throw new PermissionError("Permission denied"); } } diff --git a/packages/ctl/ctl.ts b/packages/ctl/ctl.ts index 7edbe82e..d6456cf5 100755 --- a/packages/ctl/ctl.ts +++ b/packages/ctl/ctl.ts @@ -307,7 +307,7 @@ async function startCtl() { logger.error(`Error running command: ${err.message}`); process.exitCode = 1; - } else if (err instanceof lib.ClientError) { + } else if (err instanceof lib.ClientError || err instanceof lib.UnavailableError) { logger.error(`Error sending request: ${err.message}`); process.exitCode = 1; diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index 1f110b78..aca92062 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -203,7 +203,7 @@ export class HostRouter { return false; } origin.connector.sendResponseError( - new lib.ResponseError(msg || "Instance is not running.", "ConflictError"), message.src + new lib.ResponseError(msg || "Instance is not running.", "UnavailableError"), message.src ); } return true; @@ -271,7 +271,7 @@ export class HostRouter { } if (this.host._shuttingDown) { origin.connector.sendResponseError( - new lib.ResponseError("Host is shutting down.", "ConflictError"), message.src + new lib.ResponseError("Host is shutting down.", "UnavailableError"), message.src ); return; } diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 1eebe450..84ea8793 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -1422,7 +1422,7 @@ end`.replace(/\r?\n/g, " "); async handleInstanceSaveGameRequest() { if (this._status !== "running") { - throw new lib.ConflictError("Instance is not running"); + throw new lib.UnavailableError("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.ConflictError("Instance stopped before the save finished")); + reject(new lib.UnavailableError("Instance stopped before the save finished")); }; this.server.once("save-finished", onSaved); this.server.once("exit", onExit); diff --git a/packages/lib/src/data/UserDetails.ts b/packages/lib/src/data/UserDetails.ts index b46360c9..ffdb2585 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.PermissionDeniedError} if the user does noh have the given permission. + * @throws {libErrors.PermissionError} if the user does noh have the given permission. */ checkPermission(permission: PermissionName): void } diff --git a/packages/lib/src/errors.ts b/packages/lib/src/errors.ts index a17eacdf..54a33cf2 100644 --- a/packages/lib/src/errors.ts +++ b/packages/lib/src/errors.ts @@ -49,7 +49,7 @@ export class ForbiddenError extends ClientError { } /** * Thrown when a permission check fails */ -export class PermissionDeniedError extends ForbiddenError { } +export class PermissionError extends ForbiddenError { } /** * Thrown when a config field is not accessible from the current location @@ -68,7 +68,7 @@ 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) + * instance that is running or a name that is taken (HTTP 409) */ export class ConflictError extends ClientError { } @@ -84,6 +84,12 @@ export class ServerError extends Error { } */ export class InternalServerError extends ServerError { } +/** + * Thrown when the host or instance that should handle the request is not + * connected, not running or shutting down (HTTP 503) + */ +export class UnavailableError extends ServerError { } + /** * Signal for messages that fail validation */ diff --git a/packages/lib/src/link/link.ts b/packages/lib/src/link/link.ts index 87616e44..9a8b17bb 100644 --- a/packages/lib/src/link/link.ts +++ b/packages/lib/src/link/link.ts @@ -348,7 +348,7 @@ export class Link { * * @param message - Message to check. * @param entry - Request or Event entry for this Message. - * @throws {libErrors.PermissionDeniedError} if unauthorized. + * @throws {libErrors.PermissionError} if unauthorized. */ validatePermission(message: libData.MessageRequest | libData.MessageEvent, entry: RequestEntry | EventEntry) { } diff --git a/packages/lib/src/link/messages.ts b/packages/lib/src/link/messages.ts index 8b7d77d9..c4d95dcb 100644 --- a/packages/lib/src/link/messages.ts +++ b/packages/lib/src/link/messages.ts @@ -164,13 +164,14 @@ export const errorClasses: ErrorClass[] = [ errors.InvalidValueError, errors.InvalidFieldError, errors.ForbiddenError, - errors.PermissionDeniedError, + errors.PermissionError, errors.InvalidAccessError, errors.NotFoundError, errors.TimeoutError, errors.ConflictError, errors.ServerError, errors.InternalServerError, + errors.UnavailableError, errors.InvalidMessage, errors.SessionLost, ]; diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index 166e79c7..6cb50dd8 100644 --- a/test/controller/ControlConnection.test.js +++ b/test/controller/ControlConnection.test.js @@ -197,7 +197,7 @@ describe("controller/src/ControlConnection", function() { await connection.handleLogSetSubscriptionsRequest( new lib.LogSetSubscriptionsRequest(true, false, [], []) ); - connection.user = { checkPermission() { throw new lib.PermissionDeniedError("Permission denied"); } }; + connection.user = { checkPermission() { throw new lib.PermissionError("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 31a66272..412305e0 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, ConflictError, NotFoundError, + ControllerConfig, Address, ConflictError, NotFoundError, UnavailableError, InstanceConfig, SystemInfo, Role, ModPack, AccountUpdateEvent, addPluginConfigFields, renamePermissionPrefix, } from "@clusterio/lib"; @@ -274,13 +274,13 @@ describe("controller/src/Controller", function() { mockInstanceConfig.set("instance.assigned_host", 99); assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ instanceId: 100 })), - new ConflictError("Host containing instance is not connected") + new UnavailableError("Host containing instance is not connected") ); }); it("should error on invalid host", function() { assert.throws( () => controller.sendRequest(new MockEvent(), Address.fromShorthand({ hostId: 99 })), - new ConflictError("Host is not connected") + new UnavailableError("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 b44576f8..dc254629 100644 --- a/test/controller/InstanceManager.test.js +++ b/test/controller/InstanceManager.test.js @@ -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.ConflictError + lib.UnavailableError ); }); @@ -204,7 +204,7 @@ describe("controller/InstanceManager", function () { await assert.rejects( () => instances.assignInstance(1, 6), - lib.ConflictError + lib.UnavailableError ); assert.equal(unassignRequest, undefined); diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index ef8299bb..538e5f27 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -835,7 +835,9 @@ describe("lib/config/classes", function() { it("should throw if dependent validation fails", function() { assert.throws( () => testInstance.set("test.base", 5), - new lib.InvalidValueError("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 diff --git a/test/lib/link/link.js b/test/lib/link/link.js index f0584e7d..cc7331fb 100644 --- a/test/lib/link/link.js +++ b/test/lib/link/link.js @@ -556,7 +556,7 @@ describe("lib/link/link", function() { let errored = false; connector.on("error", () => { errored = true; }); link.handle(SimpleRequest, async () => { handled = true; }); - link.validatePermission = () => { throw new lib.PermissionDeniedError("Denied"); }; + link.validatePermission = () => { throw new lib.PermissionError("Denied"); }; connector.emit("message", message); assert(!handled, "handler ran despite denial"); assert(!errored, "connector emitted error on denial"); diff --git a/test/lib/subscriptions.test.js b/test/lib/subscriptions.test.js index 64cfdd2a..461ca0ee 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.PermissionDeniedError("Permission denied"); + throw new lib.PermissionError("Permission denied"); } } } @@ -1400,26 +1400,26 @@ describe("lib/subscriptions", function() { assert.equal(eventSubscriber.synced, true); mockControl.sendTo = async function() { - throw new lib.PermissionDeniedError("Permission denied"); + throw new lib.PermissionError("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.PermissionDeniedError); + assert.ok(eventSubscriber.error instanceof lib.PermissionError); }); it("should expose errors through the snapshot", async function() { eventSubscriber.control.sendTo = function() { - throw new lib.PermissionDeniedError("Permission denied"); + throw new lib.PermissionError("Permission denied"); }; const unsubscribe = eventSubscriber.subscribe(() => {}); await new Promise(r => setImmediate(r)); const [,, error1] = eventSubscriber.getSnapshot(); - assert.ok(error1 instanceof lib.PermissionDeniedError); + assert.ok(error1 instanceof lib.PermissionError); assert.equal(error1.message, "Permission denied"); - assert.equal(lib.Link.errorName(error1), "PermissionDeniedError"); + assert.equal(lib.Link.errorName(error1), "PermissionError"); eventSubscriber.control.sendTo = function() {}; unsubscribe(); diff --git a/test/messages/note.test.js b/test/messages/note.test.js index 96cbf5cb..d09faa53 100644 --- a/test/messages/note.test.js +++ b/test/messages/note.test.js @@ -83,11 +83,11 @@ describe("messages/note", function() { it("rejects an empty resource type or id", async function() { await assert.rejects( controlConnection.handleNoteSetRequest(new lib.NoteSetRequest("", "1", "text")), - new lib.RequestError("resourceType and resourceId must not be empty"), + new lib.ValidationError("resourceType and resourceId must not be empty"), ); await assert.rejects( controlConnection.handleNoteSetRequest(new lib.NoteSetRequest("host", "", "text")), - new lib.RequestError("resourceType and resourceId must not be empty"), + new lib.ValidationError("resourceType and resourceId must not be empty"), ); }); }); From 1a03e664793ff20acb552e95a23c751326ee9570 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:05:14 +0000 Subject: [PATCH 3/3] Use the new error classes in code added on master Co-Authored-By: Claude Opus 5.5 --- packages/controller/src/ControlConnection.ts | 2 +- packages/lib/src/npm_registry.ts | 18 +++++++++--------- plugins/player_sync/controller.ts | 6 +++--- plugins/player_sync/test/controller.js | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 3772ceec..d0f7b636 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -200,7 +200,7 @@ export default class ControlConnection extends BaseConnection { 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."); + throw new lib.ConflictError("Cannot restart while a plugin install or update is running on this machine."); } const downgrade = await this._controller.checkRestartDowngrade(); if (downgrade) { diff --git a/packages/lib/src/npm_registry.ts b/packages/lib/src/npm_registry.ts index a36090fd..ad6dbcd6 100644 --- a/packages/lib/src/npm_registry.ts +++ b/packages/lib/src/npm_registry.ts @@ -1,5 +1,5 @@ import { logger } from "./logging.js"; -import { RequestError } from "./errors.js"; +import { NotFoundError, UnavailableError, ValidationError } from "./errors.js"; import { PluginSearchRequest, PluginSearchResult } from "./data/messages_plugin.js"; const registryTimeoutMs = 10e3; @@ -19,10 +19,10 @@ export function parsePackageSpec(spec: string) { ? [spec.slice(0, versionSeparator), spec.slice(versionSeparator + 1)] : [spec, undefined]; if (name.length > 214 || !packageNamePattern.test(name)) { - throw new RequestError(`Invalid plugin name: ${spec}`); + throw new ValidationError(`Invalid plugin name: ${spec}`); } if (version !== undefined && !/^[a-zA-Z0-9][a-zA-Z0-9\-.+]*$/.test(version)) { - throw new RequestError(`Invalid plugin version: ${version}`); + throw new ValidationError(`Invalid plugin version: ${version}`); } return { name, version }; } @@ -37,7 +37,7 @@ export async function checkRegistry(name: string, version?: string) { signal: AbortSignal.timeout(registryTimeoutMs), }); if (!npmRequest.ok) { - throw new RequestError(`Unknown plugin: ${spec}`); + throw new NotFoundError(`Unknown plugin: ${spec}`); } return spec; } @@ -55,12 +55,12 @@ export type LatestVersionCache = Map 500) { - throw new RequestError("Too many packages"); + throw new ValidationError("Too many packages"); } const names = [...new Set(packages.map(packageName => { const { name, version: pinned } = parsePackageSpec(packageName); if (pinned !== undefined) { - throw new RequestError(`Invalid plugin name: ${packageName}`); + throw new ValidationError(`Invalid plugin name: ${packageName}`); } return name; }))]; @@ -117,10 +117,10 @@ const npmSearchMaxSize = 250; // Limit imposed by the registry export async function searchPlugins(query: string, page: number, pageSize: number) { if (query.length > 214) { - throw new RequestError("Search query too long"); + throw new ValidationError("Search query too long"); } if (page < 1 || pageSize < 1 || pageSize > npmSearchMaxSize) { - throw new RequestError("Invalid page or page size"); + throw new ValidationError("Invalid page or page size"); } const url = new URL("https://registry.npmjs.com/-/v1/search"); @@ -130,7 +130,7 @@ export async function searchPlugins(query: string, page: number, pageSize: numbe const response = await fetch(url, { signal: AbortSignal.timeout(registryTimeoutMs) }); if (!response.ok) { - throw new RequestError(`npm registry search failed: ${response.status} ${response.statusText}`); + throw new UnavailableError(`npm registry search failed: ${response.status} ${response.statusText}`); } const json = await response.json() as { diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 513e3a2b..0df18cb1 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -109,7 +109,7 @@ export default async function(context: ControllerPluginContext) { let acquisitionRecord = getAcquisition(playerName); if (acquisitionRecord) { let instanceName = controller.instances.get(acquisitionRecord.instanceId)!.config.get("instance.name"); - throw new lib.RequestError( + throw new lib.ConflictError( `${playerName} is currently acquired by ${instanceName}, ` + "have the player leave or release the lock first" ); @@ -340,7 +340,7 @@ export default async function(context: ControllerPluginContext) { controller.handle(msg.DeletePlayerDataRequest, async (request: msg.DeletePlayerDataRequest) => { let { playerName } = request; if (!playerDatastore.has(playerName)) { - throw new lib.RequestError(`No player data stored for ${playerName}`); + throw new lib.NotFoundError(`No player data stored for ${playerName}`); } checkNotAcquired(playerName); @@ -354,7 +354,7 @@ export default async function(context: ControllerPluginContext) { controller.handle(msg.ForceReleaseRequest, async (request: msg.ForceReleaseRequest) => { let { playerName } = request; if (!acquiredPlayers.delete(playerName)) { - throw new lib.RequestError(`${playerName} is not acquired by any instance`); + throw new lib.ConflictError(`${playerName} is not acquired by any instance`); } logger.info(`Released lock on ${playerName}`); }); diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 76eb0f27..a249aa7c 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -172,7 +172,7 @@ describe("player_sync controller", function() { }); }); - const acquiredError = new lib.RequestError( + const acquiredError = new lib.ConflictError( "stored is currently acquired by Running, have the player leave or release the lock first" ); @@ -323,7 +323,7 @@ describe("player_sync controller", function() { assert.equal((await saved()).get("stored").generation, 4); }); it("should reject unknown players", async function() { - await assert.rejects(del("unknown"), new lib.RequestError("No player data stored for unknown")); + await assert.rejects(del("unknown"), new lib.NotFoundError("No player data stored for unknown")); }); it("should reject when the player is acquired", async function() { await assert.rejects(del("stored"), acquiredError); @@ -371,7 +371,7 @@ describe("player_sync controller", function() { }); it("should reject players that are not acquired", async function() { await assert.rejects( - release("unknown"), new lib.RequestError("unknown is not acquired by any instance"), + release("unknown"), new lib.ConflictError("unknown is not acquired by any instance"), ); }); });