From f87c1fb584a97f76f26e291104e6cfad4874fbf1 Mon Sep 17 00:00:00 2001 From: PalmDevs Date: Thu, 1 May 2025 01:18:36 +0700 Subject: [PATCH] feat(lib/plugins)!: change how plugin system works - Separated plugin flags for statuses - Disallow access to internal plugin flags in plugin APIs - await `handleError` when plugin throws an error - Added safe guards to prevent initing/starting a plugin twice or when it is not enabled - Call GC after plugin stops - Await plugin init and start before stopping --- lib/plugins/src/_internal.ts | 114 +++++++++++++++++++++++--------- lib/plugins/src/constants.ts | 30 ++++----- lib/plugins/src/types.ts | 10 ++- plugins/staff-settings/index.ts | 5 +- plugins/timing-log/index.ts | 5 +- 5 files changed, 111 insertions(+), 53 deletions(-) diff --git a/lib/plugins/src/_internal.ts b/lib/plugins/src/_internal.ts index a29e821..b2d6784 100644 --- a/lib/plugins/src/_internal.ts +++ b/lib/plugins/src/_internal.ts @@ -4,7 +4,7 @@ import * as PatcherLibrary from '@revenge-mod/patcher' import * as PluginsLibrary from '@revenge-mod/plugins' import * as ReactLibrary from '@revenge-mod/react' -const { PluginFlags } = PluginsLibrary +const { PluginFlags, PluginStatus } = PluginsLibrary export const _uapi: PluginsLibrary.UnscopedInitPluginApi | PluginsLibrary.UnscopedPluginApi = { discord: DiscordLibrary, @@ -16,89 +16,129 @@ export const _uapi: PluginsLibrary.UnscopedInitPluginApi | PluginsLibrary.Unscop } const _plugins = new Map() +const _pluginIFlags = new Map() export function registerPlugin( manifest: PluginsLibrary.PluginManifest, lifecycles: PluginsLibrary.PluginLifecycles, - flags: PluginsLibrary.PluginFlags = 0, + flags: PluginsLibrary.PluginFlags, + iFlags: InternalPluginFlags, ) { // TODO(plugins): verify plugin manifest if (_plugins.has(manifest.id)) throw new Error(`Plugin with ID "${manifest.id}" already registered`) + _pluginIFlags.set(manifest.id, iFlags) + const handleError = (e: unknown) => { plugin.errors.push(e) plugin.flags |= PluginFlags.Errored - return plugin.disable() + if (plugin.flags & PluginFlags.Enabled) return plugin.disable() } const plugin: InternalPlugin = { + promises: [], cleanups: [], errors: [], manifest, lifecycles, + status: 0, flags, prepareStart: () => { // const sapi = api as PluginsLibrary.PluginApi // sapi.settings ??= ... }, async init() { - plugin.flags |= PluginFlags.StatusInit - plugin.flags |= PluginFlags.StatusActioning + if (!(plugin.flags & PluginFlags.Enabled)) throw new Error(`Plugin "${manifest.id}" is not enabled`) + if (plugin.status & (PluginStatus.Initing | PluginStatus.Inited)) + throw new Error(`Plugin "${manifest.id}" is initializing or already initialized`) + + plugin.status |= PluginStatus.Initing try { - await lifecycles.init?.(api) - } catch (e) { - handleError(e) - } + const prom = lifecycles.init?.(api) + plugin.promises.push(prom) + await prom - // Not in finally block because plugin.disable() already handles - plugin.flags &= ~PluginFlags.StatusInit - plugin.flags &= ~PluginFlags.StatusActioning + // plugin.disable() already handles, so it's the try block + plugin.status |= PluginStatus.Inited + plugin.status &= ~PluginStatus.Initing + } catch (e) { + await handleError(e) + } }, async start() { - plugin.flags |= PluginFlags.StatusStart - plugin.flags |= PluginFlags.StatusActioning + if (!(plugin.flags & PluginFlags.Enabled)) throw new Error(`Plugin "${manifest.id}" is not enabled`) + if (plugin.status & (PluginStatus.Starting | PluginStatus.Started)) + throw new Error(`Plugin "${manifest.id}" is starting or already started`) + + // Clear errors from previous runs + plugin.errors = [] + plugin.status &= ~PluginFlags.Errored + + plugin.status |= PluginStatus.Starting plugin.prepareStart() try { - await lifecycles.start?.(api as PluginsLibrary.PluginApi) - } catch (e) { - handleError(e) - } + const prom = lifecycles.start?.(api as PluginsLibrary.PluginApi) + plugin.promises.push(prom) + await prom - // Not in finally block because plugin.disable() already handles - plugin.flags &= ~PluginFlags.StatusStart - plugin.flags &= ~PluginFlags.StatusActioning + // plugin.disable() already handles, so it's in the try block + plugin.status |= PluginStatus.Started + plugin.status &= ~PluginStatus.Starting + } catch (e) { + await handleError(e) + } }, async stop() { - plugin.flags &= ~PluginFlags.StatusInit - plugin.flags &= ~PluginFlags.StatusStart - plugin.flags |= PluginFlags.StatusActioning + if (!(plugin.flags & PluginFlags.Enabled)) throw new Error(`Plugin "${manifest.id}" is not enabled`) + + if (plugin.status & PluginStatus.Stopping) throw new Error(`Plugin "${manifest.id}" is stopping`) + + // If the plugin is initializing or starting, we need to wait for it to finish, then we'll stop it + if (plugin.status & (PluginStatus.Initing | PluginStatus.Starting)) await Promise.all(plugin.promises) + + if (plugin.status & (PluginStatus.Inited | PluginStatus.Started)) + throw new Error(`Plugin "${manifest.id}" is not initialized started`) // In case the plugin only has init() and stop() plugin.prepareStart() + plugin.status |= PluginStatus.Stopping + try { await lifecycles.stop?.(api as PluginsLibrary.PluginApi) } catch (e) { - handleError(e) + await handleError(e) } finally { // Run cleanups const results = await Promise.allSettled(plugin.cleanups.map(cleanup => cleanup())) - for (const result of results) if (result.status === 'rejected') plugin.errors.push(result.reason) + for (const result of results) + if (result.status === 'rejected') { + await handleError(result.reason) + // Some cleanup was unsuccessful, so we need to reload the app + plugin.flags |= PluginFlags.ReloadRequired + } - plugin.flags &= ~PluginFlags.StatusActioning + // Clear unnecessary data + plugin.promises = [] + plugin.cleanups = [] + plugin.status = 0 + + // Call garbage collector to free up memory + gc() + gc() } }, async enable() { plugin.flags |= PluginFlags.Enabled - if (lifecycles.init) plugin.flags |= PluginFlags.ReloadRequired }, async disable() { plugin.flags &= ~PluginFlags.Enabled - // If plugin is initialized/started, we need to stop it - if (plugin.flags & (PluginFlags.StatusInit | PluginFlags.StatusStart)) await plugin.stop() + + // If plugin is not stopped, and is also not stopping, we need to stop it + if (plugin.status && !(plugin.status & PluginStatus.Stopping)) await plugin.stop() }, } @@ -129,9 +169,23 @@ export async function startPlugins() { } export interface InternalPlugin extends PluginsLibrary.Plugin { + promises: Promise[] cleanups: PluginsLibrary.PluginCleanup[] prepareStart(): void init(): Promise start(): Promise stop(): Promise } + +export const InternalPluginFlags = { + /** + * Marks the plugin as internal. + */ + Internal: 1 << 0, + /** + * Marks the plugin as essential. This means it should not be removed, disabled, or stopped by normal means. + */ + Essential: 1 << 1, +} + +export type InternalPluginFlags = (typeof InternalPluginFlags)[keyof typeof InternalPluginFlags] diff --git a/lib/plugins/src/constants.ts b/lib/plugins/src/constants.ts index c01a96a..0e7e033 100644 --- a/lib/plugins/src/constants.ts +++ b/lib/plugins/src/constants.ts @@ -3,30 +3,24 @@ export const PluginFlags = { * The plugin is enabled. */ Enabled: 1 << 0, - /** - * The plugin is an internal plugin. - */ - Internal: 1 << 1, /** * The plugin requires a reload to apply changes. */ - ReloadRequired: 1 << 2, - /** - * The plugin is initialized or initializing. - */ - StatusInit: 1 << 3, - /** - * The plugin is started or starting. - */ - StatusStart: 1 << 4, - /** - * The plugin is actioning something, such as initializing or starting. - */ - StatusActioning: 1 << 5, + ReloadRequired: 1 << 1, /** * The plugin has errors. */ - Errored: 1 << 6, + Errored: 1 << 2, +} + +export const PluginStatus = { + Initing: 1 << 0, + Inited: 1 << 1, + Starting: 1 << 2, + Started: 1 << 3, + Stopping: 1 << 4, } export type PluginFlags = (typeof PluginFlags)[keyof typeof PluginFlags] + +export type PluginStatus = (typeof PluginStatus)[keyof typeof PluginStatus] diff --git a/lib/plugins/src/types.ts b/lib/plugins/src/types.ts index 6c31fa9..04ffc2a 100644 --- a/lib/plugins/src/types.ts +++ b/lib/plugins/src/types.ts @@ -1,6 +1,6 @@ import type { DiscordModules } from '@revenge-mod/discord/types' -import type { PluginFlags } from './constants' +import type { PluginFlags, PluginStatus } from './constants' /** * The unscoped plugin API (limited). This API is available as a global for plugins. @@ -57,6 +57,10 @@ export interface PluginApi extends InitPluginApi { // * The author of the plugin bundle. // */ // author: string +// /** +// * The URL of the plugin bundle. +// */ +// url: string // } export interface PluginManifest { @@ -121,6 +125,10 @@ export interface Plugin { * The plugin flags. */ flags: PluginFlags + /** + * The plugin status. + */ + status: PluginStatus /** * Errors encountered during the plugin lifecycles. */ diff --git a/plugins/staff-settings/index.ts b/plugins/staff-settings/index.ts index 8675dbd..b63903c 100644 --- a/plugins/staff-settings/index.ts +++ b/plugins/staff-settings/index.ts @@ -4,7 +4,7 @@ import { byProps } from '@revenge-mod/modules/finders/filters' import { after } from '@revenge-mod/patcher' import { PluginFlags } from '@revenge-mod/plugins' -import { registerPlugin } from '@revenge-mod/plugins/_' +import { InternalPluginFlags, registerPlugin } from '@revenge-mod/plugins/_' registerPlugin( { @@ -19,5 +19,6 @@ registerPlugin( cleanup(after(UserStoreUtils, 'isStaffEnv', () => true)) }, }, - PluginFlags.Internal | PluginFlags.Enabled, + PluginFlags.Enabled, + InternalPluginFlags.Internal, ) diff --git a/plugins/timing-log/index.ts b/plugins/timing-log/index.ts index d9dfef5..ffa4364 100644 --- a/plugins/timing-log/index.ts +++ b/plugins/timing-log/index.ts @@ -1,7 +1,7 @@ import { AppStartPerformance } from '@revenge-mod/discord' import { PluginFlags } from '@revenge-mod/plugins' -import { registerPlugin } from '@revenge-mod/plugins/_' +import { InternalPluginFlags, registerPlugin } from '@revenge-mod/plugins/_' AppStartPerformance.mark('👊', 'Plugins register') @@ -23,5 +23,6 @@ registerPlugin( AppStartPerformance.mark('👊', 'Plugins start', performance.now() - tsInit) }, }, - PluginFlags.Internal | PluginFlags.Enabled, + PluginFlags.Enabled, + InternalPluginFlags.Internal, )