From 7bc583b18621c8140232772ca36c6d9b8d8a9cd7 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 24 Jun 2025 15:10:07 -0400 Subject: [PATCH] fix: remote access lifecycle during boot & shutdown (#1422) ## Summary - **New Features** - Introduced a comprehensive Nginx control script for Unraid OS, enabling advanced server management, SSL certificate handling, and dynamic configuration based on system state. https://github.com/unraid/webgui/pull/2269 - Added a utility function to safely execute code with error handling support. - **Improvements** - Enhanced logging across remote access, WAN access, and settings services for improved traceability. - Added initialization and cleanup hooks to remote access and UPnP services for better lifecycle management. - Optimized configuration persistence by batching rapid changes for more efficient updates. - Refined URL resolution logic for improved configuration retrieval and error handling. - Broadened pattern matching for domain keys in Nginx state parsing. - Updated remote access settings to reload the network stack after changes. - Simplified remote access and WAN port configuration logic for clarity and accuracy. - Improved port mapping logic with explicit error handling in UPnP service. - Updated UI and form controls for remote access settings to reflect SSL requirements and access type restrictions. - **Configuration** - Updated the default path for module configuration files. --- api/src/environment.ts | 2 +- .../mutation/connect/setup-remote-access.ts | 0 api/src/store/state-parsers/nginx.ts | 4 +- .../resolvers/settings/settings.resolver.ts | 2 +- .../src/event-handler/wan-access.handler.ts | 13 +- .../src/service/config.persistence.ts | 16 +-- .../src/service/connect-settings.service.ts | 113 +++++++++++++----- .../service/dynamic-remote-access.service.ts | 31 ++++- .../service/static-remote-access.service.ts | 10 +- .../src/service/upnp-remote-access.service.ts | 19 ++- .../src/service/upnp.service.ts | 39 ++++-- .../src/service/url-resolver.service.ts | 82 ++++--------- .../src/test/url-resolver.service.test.ts | 4 +- packages/unraid-shared/src/util/processing.ts | 33 +++++ 14 files changed, 227 insertions(+), 141 deletions(-) delete mode 100644 api/src/graphql/resolvers/mutation/connect/setup-remote-access.ts create mode 100644 packages/unraid-shared/src/util/processing.ts diff --git a/api/src/environment.ts b/api/src/environment.ts index 9f0469352..eb4e81eaa 100644 --- a/api/src/environment.ts +++ b/api/src/environment.ts @@ -101,4 +101,4 @@ export const PM2_PATH = join(import.meta.dirname, '../../', 'node_modules', 'pm2 export const ECOSYSTEM_PATH = join(import.meta.dirname, '../../', 'ecosystem.config.json'); export const PATHS_CONFIG_MODULES = - process.env.PATHS_CONFIG_MODULES ?? '/usr/local/unraid-api/config/modules'; + process.env.PATHS_CONFIG_MODULES ?? '/boot/config/plugins/dynamix.my.servers/configs'; diff --git a/api/src/graphql/resolvers/mutation/connect/setup-remote-access.ts b/api/src/graphql/resolvers/mutation/connect/setup-remote-access.ts deleted file mode 100644 index e69de29bb..000000000 diff --git a/api/src/store/state-parsers/nginx.ts b/api/src/store/state-parsers/nginx.ts index c42603852..1d5385231 100644 --- a/api/src/store/state-parsers/nginx.ts +++ b/api/src/store/state-parsers/nginx.ts @@ -2,8 +2,8 @@ import type { IniStringBooleanOrAuto } from '@app/core/types/ini.js'; import type { StateFileToIniParserMap } from '@app/store/types.js'; import { type FqdnEntry } from '@app/core/types/states/nginx.js'; -// Allow upper or lowercase FQDN6 -const fqdnRegex = /^nginx(.*?)fqdn6?$/i; +// Allow upper or lowercase FQDN6, with optional separators +const fqdnRegex = /^nginx[_-]?(.+?)fqdn6?$/i; export type NginxIni = { nginxCertname: string; diff --git a/api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts b/api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts index 857bc2d2e..251bd2da6 100644 --- a/api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts @@ -89,8 +89,8 @@ export class UnifiedSettingsResolver { ): Promise { this.logger.verbose('Updating Settings %O', input); const { restartRequired, values } = await this.userSettings.updateNamespacedValues(input); + this.logger.verbose('Updated Setting Values %O', values); if (restartRequired) { - this.logger.verbose('Will restart %O', values); // hack: allow time for pending writes to flush this.lifecycleService.restartApi({ delayMs: 300 }); } diff --git a/packages/unraid-api-plugin-connect/src/event-handler/wan-access.handler.ts b/packages/unraid-api-plugin-connect/src/event-handler/wan-access.handler.ts index ee50ba1a1..62c87c815 100644 --- a/packages/unraid-api-plugin-connect/src/event-handler/wan-access.handler.ts +++ b/packages/unraid-api-plugin-connect/src/event-handler/wan-access.handler.ts @@ -1,31 +1,30 @@ -import { Injectable, OnModuleDestroy } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { OnEvent } from '@nestjs/event-emitter'; import { EVENTS } from '../helper/nest-tokens.js'; import { ConfigType } from '../model/connect-config.model.js'; import { NetworkService } from '../service/network.service.js'; -import { UrlResolverService } from '../service/url-resolver.service.js'; @Injectable() -export class WanAccessEventHandler implements OnModuleDestroy { +export class WanAccessEventHandler { + private readonly logger = new Logger(WanAccessEventHandler.name); + constructor( private readonly configService: ConfigService, private readonly networkService: NetworkService ) {} - async onModuleDestroy() { - await this.disableWanAccess(); - } - @OnEvent(EVENTS.ENABLE_WAN_ACCESS, { async: true }) async enableWanAccess() { + this.logger.log('Enabling WAN Access'); this.configService.set('connect.config.wanaccess', true); await this.networkService.reloadNetworkStack(); } @OnEvent(EVENTS.DISABLE_WAN_ACCESS, { async: true }) async disableWanAccess() { + this.logger.log('Disabling WAN Access'); this.configService.set('connect.config.wanaccess', false); await this.networkService.reloadNetworkStack(); } diff --git a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts index d7c272433..f5ede7c27 100644 --- a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts +++ b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts @@ -9,14 +9,14 @@ import { plainToInstance } from 'class-transformer'; import { validateOrReject } from 'class-validator'; import { parse as parseIni } from 'ini'; import { isEqual } from 'lodash-es'; -import { debounceTime } from 'rxjs/operators'; +import { bufferTime } from 'rxjs/operators'; import type { MyServersConfig as LegacyConfig } from '../model/my-servers-config.model.js'; import { ConfigType, MyServersConfig } from '../model/connect-config.model.js'; @Injectable() export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { - constructor(private readonly configService: ConfigService) {} + constructor(private readonly configService: ConfigService) {} private logger = new Logger(ConnectConfigPersister.name); get configPath() { @@ -33,10 +33,10 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { this.logger.verbose(`Config path: ${this.configPath}`); await this.loadOrMigrateConfig(); // Persist changes to the config. - this.configService.changes$.pipe(debounceTime(25)).subscribe({ - next: async ({ newValue, oldValue, path }) => { - if (path.startsWith('connect.config')) { - this.logger.verbose(`Config changed: ${path} from ${oldValue} to ${newValue}`); + this.configService.changes$.pipe(bufferTime(25)).subscribe({ + next: async (changes) => { + const connectConfigChanged = changes.some(({ path }) => path.startsWith('connect.config')); + if (connectConfigChanged) { await this.persist(); } }, @@ -60,7 +60,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { return false; } } catch (error) { - this.logger.error(`Error loading config (will overwrite file):`, error); + this.logger.error(error, `Error loading config (will overwrite file)`); } const data = JSON.stringify(config, null, 2); this.logger.verbose(`Persisting config to ${this.configPath}: ${data}`); @@ -69,7 +69,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { this.logger.verbose(`Config persisted to ${this.configPath}`); return true; } catch (error) { - this.logger.error(`Error persisting config to '${this.configPath}':`, error); + this.logger.error(error, `Error persisting config to '${this.configPath}'`); return false; } } diff --git a/packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts b/packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts index a9ccf90fd..212fcf2b0 100644 --- a/packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts @@ -2,7 +2,7 @@ import { Injectable, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { EventEmitter2 } from '@nestjs/event-emitter'; -import type { SchemaBasedCondition } from '@jsonforms/core'; +import type { JsonSchema7, SchemaBasedCondition } from '@jsonforms/core'; import type { DataSlice, SettingSlice, UIElement } from '@unraid/shared/jsonforms/settings.js'; import { RuleEffect } from '@jsonforms/core'; import { createLabeledControl } from '@unraid/shared/jsonforms/control.js'; @@ -26,6 +26,7 @@ import { ConfigType, MyServersConfig } from '../model/connect-config.model.js'; import { DynamicRemoteAccessType, WAN_ACCESS_TYPE, WAN_FORWARD_TYPE } from '../model/connect.model.js'; import { ConnectApiKeyService } from './connect-api-key.service.js'; import { DynamicRemoteAccessService } from './dynamic-remote-access.service.js'; +import { NetworkService } from './network.service.js'; declare module '@unraid/shared/services/user-settings.js' { interface UserSettings { @@ -40,7 +41,8 @@ export class ConnectSettingsService { private readonly remoteAccess: DynamicRemoteAccessService, private readonly apiKeyService: ConnectApiKeyService, private readonly eventEmitter: EventEmitter2, - private readonly userSettings: UserSettingsService + private readonly userSettings: UserSettingsService, + private readonly networkService: NetworkService ) { this.userSettings.register('remote-access', { buildSlice: async () => this.buildRemoteAccessSlice(), @@ -215,7 +217,7 @@ export class ConnectSettingsService { forwardType?: WAN_FORWARD_TYPE | undefined | null ): DynamicRemoteAccessType { // If access is disabled or always, DRA is disabled - if (accessType === WAN_ACCESS_TYPE.DISABLED || accessType === WAN_ACCESS_TYPE.ALWAYS) { + if (accessType === WAN_ACCESS_TYPE.DISABLED) { return DynamicRemoteAccessType.DISABLED; } // if access is enabled and forward type is UPNP, DRA is UPNP, otherwise it is static @@ -231,10 +233,10 @@ export class ConnectSettingsService { ); this.configService.set('connect.config.wanaccess', input.accessType === WAN_ACCESS_TYPE.ALWAYS); - this.configService.set( - 'connect.config.wanport', - input.forwardType === WAN_FORWARD_TYPE.STATIC ? input.port : null - ); + if (input.forwardType === WAN_FORWARD_TYPE.STATIC) { + this.configService.set('connect.config.wanport', input.port); + // when forwarding with upnp, the upnp service will clear & set the wanport as necessary + } this.configService.set( 'connect.config.upnpEnabled', input.forwardType === WAN_FORWARD_TYPE.UPNP @@ -251,17 +253,15 @@ export class ConnectSettingsService { }, }); + await this.networkService.reloadNetworkStack(); + return true; } public async dynamicRemoteAccessSettings(): Promise { const config = this.configService.getOrThrow('connect.config'); return { - accessType: config.wanaccess - ? config.dynamicRemoteAccessType !== DynamicRemoteAccessType.DISABLED - ? WAN_ACCESS_TYPE.DYNAMIC - : WAN_ACCESS_TYPE.ALWAYS - : WAN_ACCESS_TYPE.DISABLED, + accessType: config.wanaccess ? WAN_ACCESS_TYPE.ALWAYS : WAN_ACCESS_TYPE.DISABLED, forwardType: config.upnpEnabled ? WAN_FORWARD_TYPE.UPNP : WAN_FORWARD_TYPE.STATIC, port: config.wanport ? Number(config.wanport) : null, }; @@ -272,9 +272,48 @@ export class ConnectSettingsService { *------------------------------------------------------------------------**/ async buildRemoteAccessSlice(): Promise { - return mergeSettingSlices([await this.remoteAccessSlice()], { - as: 'remote-access', - }); + const slice = await this.remoteAccessSlice(); + /**------------------------------------------------------------------------ + * UX: Only validate 'port' when relevant + * + * 'port' will be null when remote access is disabled, and it's irrelevant + * when using upnp (because it becomes read-only for the end-user). + * + * In these cases, we should omit type and range validation for 'port' + * to avoid confusing end-users. + * + * But, when using static port forwarding, 'port' is required, so we validate it. + *------------------------------------------------------------------------**/ + return { + properties: { + 'remote-access': { + type: 'object', + properties: slice.properties as JsonSchema7['properties'], + allOf: [ + { + if: { + properties: { + forwardType: { const: WAN_FORWARD_TYPE.STATIC }, + accessType: { const: WAN_ACCESS_TYPE.ALWAYS }, + }, + required: ['forwardType', 'accessType'], + }, + then: { + required: ['port'], + properties: { + port: { + type: 'number', + minimum: 1, + maximum: 65535, + }, + }, + }, + }, + ], + }, + }, + elements: slice.elements, + }; } buildFlashBackupSlice(): SettingSlice { @@ -289,7 +328,8 @@ export class ConnectSettingsService { async remoteAccessSlice(): Promise { const isSignedIn = await this.isSignedIn(); const isSSLCertProvisioned = await this.isSSLCertProvisioned(); - const precondition = isSignedIn && isSSLCertProvisioned; + const { sslEnabled } = this.configService.getOrThrow('store.emhttp.nginx'); + const precondition = isSignedIn && isSSLCertProvisioned && sslEnabled; /** shown when preconditions are not met */ const requirements: UIElement[] = [ @@ -315,6 +355,10 @@ export class ConnectSettingsService { text: 'You have provisioned a valid SSL certificate', status: isSSLCertProvisioned, }, + { + text: 'SSL is enabled', + status: sslEnabled, + }, ], }, }, @@ -353,19 +397,30 @@ export class ConnectSettingsService { }, }, rule: { - effect: RuleEffect.SHOW, + effect: RuleEffect.DISABLE, condition: { + scope: '#/properties/remote-access', schema: { - properties: { - forwardType: { - enum: [WAN_FORWARD_TYPE.STATIC], + anyOf: [ + { + properties: { + accessType: { + const: WAN_ACCESS_TYPE.DISABLED, + }, + }, + required: ['accessType'], }, - accessType: { - enum: [WAN_ACCESS_TYPE.DYNAMIC, WAN_ACCESS_TYPE.ALWAYS], + { + properties: { + forwardType: { + const: WAN_FORWARD_TYPE.UPNP, + }, + }, + required: ['forwardType'], }, - }, + ], }, - } as Omit, + }, }, }), ]; @@ -374,22 +429,22 @@ export class ConnectSettingsService { const properties: DataSlice = { accessType: { type: 'string', - enum: Object.values(WAN_ACCESS_TYPE), + enum: [WAN_ACCESS_TYPE.DISABLED, WAN_ACCESS_TYPE.ALWAYS], title: 'Allow Remote Access', - default: 'DISABLED', + default: WAN_ACCESS_TYPE.DISABLED, }, forwardType: { type: 'string', enum: Object.values(WAN_FORWARD_TYPE), title: 'Forward Type', - default: 'STATIC', + default: WAN_FORWARD_TYPE.STATIC, }, port: { - type: 'number', + // 'port' is null when remote access is disabled. + type: ['number', 'null'], title: 'WAN Port', minimum: 0, maximum: 65535, - default: 0, }, }; diff --git a/packages/unraid-api-plugin-connect/src/service/dynamic-remote-access.service.ts b/packages/unraid-api-plugin-connect/src/service/dynamic-remote-access.service.ts index 043b2b63d..7d829da21 100644 --- a/packages/unraid-api-plugin-connect/src/service/dynamic-remote-access.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/dynamic-remote-access.service.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable, Logger, OnApplicationBootstrap } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { URL_TYPE } from '@unraid/shared/network.model.js'; @@ -15,15 +15,19 @@ import { StaticRemoteAccessService } from './static-remote-access.service.js'; import { UpnpRemoteAccessService } from './upnp-remote-access.service.js'; @Injectable() -export class DynamicRemoteAccessService { +export class DynamicRemoteAccessService implements OnApplicationBootstrap { private readonly logger = new Logger(DynamicRemoteAccessService.name); constructor( - private readonly configService: ConfigService, + private readonly configService: ConfigService, private readonly staticRemoteAccessService: StaticRemoteAccessService, private readonly upnpRemoteAccessService: UpnpRemoteAccessService ) {} + async onApplicationBootstrap() { + await this.initRemoteAccess(); + } + /** * Get the current state of dynamic remote access */ @@ -60,6 +64,7 @@ export class DynamicRemoteAccessService { type: url.type ?? URL_TYPE.WAN, }; this.configService.set('connect.dynamicRemoteAccess.allowedUrl', newAllowed); + this.logger.verbose(`setAllowedUrl: ${JSON.stringify(newAllowed, null, 2)}`); } private setErrorMessage(error: string) { @@ -75,6 +80,7 @@ export class DynamicRemoteAccessService { type: DynamicRemoteAccessType; }) { try { + this.logger.verbose(`enableDynamicRemoteAccess ${JSON.stringify(input, null, 2)}`); await this.stopRemoteAccess(); if (input.allowedUrl) { this.setAllowedUrl({ @@ -98,6 +104,7 @@ export class DynamicRemoteAccessService { * @param type The new dynamic remote access type to set */ private async setType(type: DynamicRemoteAccessType): Promise { + this.logger.verbose(`setType: ${type}`); // Update the config first this.configService.set('connect.config.dynamicRemoteAccessType', type); @@ -138,4 +145,22 @@ export class DynamicRemoteAccessService { this.clearPing(); this.clearError(); } + + private async initRemoteAccess() { + this.logger.verbose('Initializing Remote Access'); + const { wanaccess, upnpEnabled } = this.configService.get('connect.config', { infer: true }); + if (wanaccess && upnpEnabled) { + await this.enableDynamicRemoteAccess({ + type: DynamicRemoteAccessType.UPNP, + allowedUrl: { + ipv4: null, + ipv6: null, + type: URL_TYPE.WAN, + name: 'WAN', + }, + }); + } + // if wanaccess is true and upnpEnabled is false, static remote access is already "enabled". + // if wanaccess is false, remote access is already "disabled". + } } diff --git a/packages/unraid-api-plugin-connect/src/service/static-remote-access.service.ts b/packages/unraid-api-plugin-connect/src/service/static-remote-access.service.ts index dce2aba96..bae350c52 100644 --- a/packages/unraid-api-plugin-connect/src/service/static-remote-access.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/static-remote-access.service.ts @@ -21,13 +21,9 @@ export class StaticRemoteAccessService { } async beginRemoteAccess(): Promise { - const { dynamicRemoteAccessType } = - this.configService.getOrThrow('connect.config'); - if (dynamicRemoteAccessType !== DynamicRemoteAccessType.STATIC) { - this.logger.error('Invalid Dynamic Remote Access Type: %s', dynamicRemoteAccessType); - return null; - } - this.logger.log('Enabling Static Remote Access'); + this.logger.log('Begin Static Remote Access'); + // enabling/disabling static remote access is a config-only change. + // the actual forwarding must be configured on the router, outside of the API. this.eventEmitter.emit(EVENTS.ENABLE_WAN_ACCESS); return this.urlResolverService.getRemoteAccessUrl(); } diff --git a/packages/unraid-api-plugin-connect/src/service/upnp-remote-access.service.ts b/packages/unraid-api-plugin-connect/src/service/upnp-remote-access.service.ts index a3c48c753..ea38757a0 100644 --- a/packages/unraid-api-plugin-connect/src/service/upnp-remote-access.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/upnp-remote-access.service.ts @@ -24,22 +24,19 @@ export class UpnpRemoteAccessService { } async begin() { - const sslPort = this.configService.get('store.emhttp.var.portssl'); - if (!sslPort || isNaN(Number(sslPort))) { - throw new Error(`Invalid SSL port configuration: ${sslPort}`); + this.logger.log('Begin UPNP Remote Access'); + const { httpsPort, httpPort, sslMode } = this.configService.getOrThrow('store.emhttp.nginx'); + const localPort = sslMode === 'no' ? Number(httpPort) : Number(httpsPort); + if (isNaN(localPort)) { + throw new Error(`Invalid local port configuration: ${localPort}`); } try { - await this.upnpService.createOrRenewUpnpLease({ - sslPort: Number(sslPort), - }); + const mapping = await this.upnpService.createOrRenewUpnpLease({ localPort }); + this.configService.set('connect.config.wanport', mapping.publicPort); this.eventEmitter.emit(EVENTS.ENABLE_WAN_ACCESS); return this.urlResolverService.getRemoteAccessUrl(); } catch (error) { - this.logger.error( - 'Failed to begin UPNP Remote Access using port %s: %O', - String(sslPort), - error - ); + this.logger.error(error, 'Failed to begin UPNP Remote Access'); await this.stop(); } } diff --git a/packages/unraid-api-plugin-connect/src/service/upnp.service.ts b/packages/unraid-api-plugin-connect/src/service/upnp.service.ts index 0cc775140..798808716 100644 --- a/packages/unraid-api-plugin-connect/src/service/upnp.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/upnp.service.ts @@ -1,4 +1,4 @@ -import { Inject, Injectable, Logger } from '@nestjs/common'; +import { Inject, Injectable, Logger, OnModuleDestroy } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { Cron, SchedulerRegistry } from '@nestjs/schedule'; @@ -11,7 +11,7 @@ import { UPNP_RENEWAL_JOB_TOKEN } from '../helper/nest-tokens.js'; import { ConfigType } from '../model/connect-config.model.js'; @Injectable() -export class UpnpService { +export class UpnpService implements OnModuleDestroy { private readonly logger = new Logger(UpnpService.name); #enabled = false; #wanPort: number | undefined; @@ -39,6 +39,10 @@ export class UpnpService { return this.scheduleRegistry.getCronJob(UPNP_RENEWAL_JOB_TOKEN); } + async onModuleDestroy() { + await this.disableUpnp(); + } + private async removeUpnpMapping() { if (isDefined(this.#wanPort) && isDefined(this.#localPort)) { const portMap = { @@ -143,20 +147,18 @@ export class UpnpService { return newWanPort; } - async createOrRenewUpnpLease(args?: { sslPort?: number; wanPort?: number }) { - const { sslPort, wanPort } = args ?? {}; - if (wanPort !== this.#wanPort || this.#localPort !== sslPort) { + async createOrRenewUpnpLease(args?: { localPort?: number; wanPort?: number }) { + const { localPort, wanPort } = args ?? {}; + const newWanOrLocalPort = wanPort !== this.#wanPort || localPort !== this.#localPort; + const upnpWasInitialized = this.#wanPort && this.#localPort; + // remove old mapping when new ports are requested + if (upnpWasInitialized && newWanOrLocalPort) { await this.removeUpnpMapping(); } + // get new ports to use const wanPortToUse = await this.getWanPortToUse(args); - const localPortToUse = sslPort ?? this.#localPort; - if (wanPortToUse && localPortToUse) { - this.#wanPort = wanPortToUse; - await this.createUpnpMapping({ - publicPort: wanPortToUse, - privatePort: localPortToUse, - }); - } else { + const localPortToUse = localPort ?? this.#localPort; + if (!wanPortToUse || !localPortToUse) { await this.disableUpnp(); this.logger.error('No WAN port found %o. Disabled UPNP.', { wanPort: wanPortToUse, @@ -164,6 +166,17 @@ export class UpnpService { }); throw new Error('No WAN port found. Disabled UPNP.'); } + // create new mapping + const mapping = { + publicPort: wanPortToUse, + privatePort: localPortToUse, + }; + const success = await this.createUpnpMapping(mapping); + if (success) { + return mapping; + } else { + throw new Error('Failed to create UPNP mapping'); + } } async disableUpnp() { diff --git a/packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts b/packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts index bf6e9365e..81324f80b 100644 --- a/packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts @@ -2,6 +2,7 @@ import { Injectable, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { URL_TYPE } from '@unraid/shared/network.model.js'; +import { makeSafeRunner } from '@unraid/shared/util/processing.js'; import { ConfigType } from '../model/connect-config.model.js'; @@ -115,7 +116,7 @@ export interface AccessUrl { export class UrlResolverService { private readonly logger = new Logger(UrlResolverService.name); - constructor(private readonly configService: ConfigService) {} + constructor(private readonly configService: ConfigService) {} /** * Constructs a URL from the given field parameters. @@ -259,11 +260,7 @@ export class UrlResolverService { } const { nginx } = store.emhttp; - const { - config: { - remote: { wanport }, - }, - } = store; + const wanport = this.configService.getOrThrow('connect.config.wanport', { infer: true }); if (!nginx || Object.keys(nginx).length === 0) { return { urls: [], errors: [new Error('Nginx Not Loaded')] }; @@ -272,8 +269,15 @@ export class UrlResolverService { const errors: Error[] = []; const urls: AccessUrl[] = []; - try { - // Default URL + const doSafely = makeSafeRunner((error) => { + if (error instanceof Error) { + errors.push(error); + } else { + this.logger.warn(error, 'Uncaught error in network resolver'); + } + }); + + doSafely(() => { const defaultUrl = new URL(nginx.defaultUrl); urls.push({ name: 'Default', @@ -281,15 +285,9 @@ export class UrlResolverService { ipv4: defaultUrl, ipv6: defaultUrl, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); - try { + doSafely(() => { // Lan IP URL const lanIp4Url = this.getUrlForServer(nginx, 'lanIp'); urls.push({ @@ -297,15 +295,9 @@ export class UrlResolverService { type: URL_TYPE.LAN, ipv4: lanIp4Url, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); - try { + doSafely(() => { // Lan IP6 URL const lanIp6Url = this.getUrlForServer(nginx, 'lanIp6'); urls.push({ @@ -313,15 +305,9 @@ export class UrlResolverService { type: URL_TYPE.LAN, ipv6: lanIp6Url, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); - try { + doSafely(() => { // Lan Name URL const lanNameUrl = this.getUrlForServer(nginx, 'lanName'); urls.push({ @@ -329,15 +315,9 @@ export class UrlResolverService { type: URL_TYPE.MDNS, ipv4: lanNameUrl, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); - try { + doSafely(() => { // Lan MDNS URL const lanMdnsUrl = this.getUrlForServer(nginx, 'lanMdns'); urls.push({ @@ -345,35 +325,23 @@ export class UrlResolverService { type: URL_TYPE.MDNS, ipv4: lanMdnsUrl, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); // Now Process the FQDN Urls nginx.fqdnUrls.forEach((fqdnUrl: FqdnEntry) => { - try { + doSafely(() => { const urlType = this.getUrlTypeFromFqdn(fqdnUrl.interface); const fqdnUrlToUse = this.getUrlForField({ url: fqdnUrl.fqdn, - portSsl: urlType === URL_TYPE.WAN ? Number(wanport) : nginx.httpsPort, + portSsl: Number(wanport || nginx.httpsPort), }); urls.push({ name: `FQDN ${fqdnUrl.interface}${fqdnUrl.id !== null ? ` ${fqdnUrl.id}` : ''}`, - type: this.getUrlTypeFromFqdn(fqdnUrl.interface), + type: urlType, ipv4: fqdnUrlToUse, }); - } catch (error: unknown) { - if (error instanceof Error) { - errors.push(error); - } else { - this.logger.warn('Uncaught error in network resolver', error); - } - } + }); }); return { urls, errors }; diff --git a/packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts b/packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts index b38255ac4..c0e4644ef 100644 --- a/packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts +++ b/packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts @@ -14,12 +14,12 @@ interface PortTestParams { describe('UrlResolverService', () => { let service: UrlResolverService; - let mockConfigService: ConfigService; + let mockConfigService: ConfigService; beforeEach(() => { mockConfigService = { get: vi.fn(), - } as unknown as ConfigService; + } as unknown as ConfigService; service = new UrlResolverService(mockConfigService); }); diff --git a/packages/unraid-shared/src/util/processing.ts b/packages/unraid-shared/src/util/processing.ts new file mode 100644 index 000000000..37b6e6991 --- /dev/null +++ b/packages/unraid-shared/src/util/processing.ts @@ -0,0 +1,33 @@ +// Utils related to processing operations +// e.g. parallel processing, safe processing, etc. + +/** + * Creates a function that runs a given function and catches any errors. + * If an error is caught, it is passed to the `onError` function. + * + * @param onError - The function to call if an error is caught. + * @returns A function that runs the given function and catches any errors. + * @example + * const errors: Error[] = []; + * const doSafely = makeSafeRunner((error) => { + * if (error instanceof Error) { + * errors.push(error); + * } else { + * this.logger.warn(error, 'Uncaught error in network resolver'); + * } + * }); + * + * doSafely(() => { + * JSON.parse(aFile); + * throw new Error('test'); + * }); + */ +export function makeSafeRunner(onError: (error: unknown) => void) { + return (fn: () => T) => { + try { + return fn(); + } catch (error) { + onError(error); + } + }; +}