Skip to content

Commit 23ef760

Browse files
elibosleypujitm
elibosley
and
pujitm
authored
fix: enhance plugin management with interactive removal prompts (#1549)
- Add RemovePluginQuestionSet for interactive plugin removal - Update plugin commands to use PluginManagementService - Improve plugin installation error handling and warnings - Clean up test fixtures and update plugin command tests - Reset dev config to clean state (v4.11.0, no plugins) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved plugin management in the CLI with interactive prompts for plugin removal and enhanced error handling. * CLI plugin commands now provide clearer user feedback and warnings for missing plugins. * Added log suppression capability and dedicated plugin log file support. * **Refactor** * Plugin CLI commands now use dedicated management services and interactive prompts instead of direct GraphQL operations, streamlining workflows and improving reliability. * Simplified CLI imports and logging for more straightforward error handling. * Deferred plugin module logging to application bootstrap for improved lifecycle management. * Updated logging service to respect global log suppression and added unconditional logging method. * **Tests** * Refactored plugin CLI command tests for better isolation and coverage, using service mocks and enhanced prompt simulations. * Updated report command tests to reflect new logging behavior. * **Chores** * Updated API configuration settings and removed outdated test fixture files and timestamp data. * Simplified test file logic by removing remote file download and cache functionality. * Adjusted build configuration to conditionally set CLI shebang based on environment. * Enhanced configuration file handler to optionally accept external logging. * Updated CLI command script to set environment variable for testing. * Added environment variables for log file paths and log suppression. * Updated logging setup to conditionally suppress logs and write plugin logs to file. * Improved error and output logging consistency across CLI commands. * Added placeholder file to ensure log directory version control tracking. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pujit Mehrotra <[email protected]>
1 parent 6ea94f0 commit 23ef760

31 files changed

+404
-366
lines changed

‎api/.env.development‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ PATHS_ACTIVATION_BASE=./dev/activation
1515
PATHS_PASSWD=./dev/passwd
1616
PATHS_RCLONE_SOCKET=./dev/rclone-socket
1717
PATHS_LOG_BASE=./dev/log # Where we store logs
18+
PATHS_LOGS_FILE=./dev/log/graphql-api.log
1819
PATHS_CONNECT_STATUS_FILE_PATH=./dev/connectStatus.json # Connect plugin status file
1920
ENVIRONMENT="development"
2021
NODE_ENV="development"

‎api/.env.test‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@ PATHS_PARITY_CHECKS=./dev/states/parity-checks.log
1313
PATHS_CONFIG_MODULES=./dev/configs
1414
PATHS_ACTIVATION_BASE=./dev/activation
1515
PATHS_PASSWD=./dev/passwd
16+
PATHS_LOGS_FILE=./dev/log/graphql-api.log
1617
PORT=5000
1718
NODE_ENV="test"

‎api/dev/configs/api.json‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
{
2-
"version": "4.10.0",
3-
"extraOrigins": [
4-
"https://google.com",
5-
"https://test.com"
6-
],
7-
"sandbox": true,
2+
"version": "4.11.0",
3+
"extraOrigins": [],
4+
"sandbox": false,
85
"ssoSubIds": [],
9-
"plugins": [
10-
"unraid-api-plugin-connect"
11-
]
6+
"plugins": []
127
}

‎api/dev/log/.gitkeep‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# custom log directory for tests & development

‎api/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"start": "node dist/main.js",
1818
"dev": "vite",
1919
"dev:debug": "NODE_OPTIONS='--inspect-brk=9229 --enable-source-maps' vite",
20-
"command": "pnpm run build && clear && ./dist/cli.js",
20+
"command": "COMMAND_TESTER=true pnpm run build > /dev/null 2>&1 && NODE_ENV=development ./dist/cli.js",
2121
"command:raw": "./dist/cli.js",
2222
"// Build and Deploy": "",
2323
"build": "vite build --mode=production",

‎api/src/cli.ts‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,37 @@
11
import '@app/dotenv.js';
22

3-
import { execa } from 'execa';
3+
import { Logger } from '@nestjs/common';
4+
45
import { CommandFactory } from 'nest-commander';
56

6-
import { internalLogger, logger } from '@app/core/log.js';
7-
import { LOG_LEVEL } from '@app/environment.js';
8-
import { CliModule } from '@app/unraid-api/cli/cli.module.js';
7+
import { LOG_LEVEL, SUPPRESS_LOGS } from '@app/environment.js';
98
import { LogService } from '@app/unraid-api/cli/log.service.js';
109

1110
const getUnraidApiLocation = async () => {
11+
const { execa } = await import('execa');
1212
try {
1313
const shellToUse = await execa('which unraid-api');
1414
return shellToUse.stdout.trim();
1515
} catch (err) {
16-
logger.debug('Could not find unraid-api in PATH, using default location');
17-
1816
return '/usr/bin/unraid-api';
1917
}
2018
};
2119

20+
const getLogger = () => {
21+
if (LOG_LEVEL === 'TRACE' && !SUPPRESS_LOGS) {
22+
return new LogService();
23+
}
24+
return false;
25+
};
26+
27+
const logger = getLogger();
2228
try {
2329
await import('json-bigint-patch');
30+
const { CliModule } = await import('@app/unraid-api/cli/cli.module.js');
31+
2432
await CommandFactory.run(CliModule, {
2533
cliName: 'unraid-api',
26-
logger: LOG_LEVEL === 'TRACE' ? new LogService() : false, // - enable this to see nest initialization issues
34+
logger: logger, // - enable this to see nest initialization issues
2735
completion: {
2836
fig: false,
2937
cmd: 'completion-script',
@@ -32,10 +40,8 @@ try {
3240
});
3341
process.exit(0);
3442
} catch (error) {
35-
logger.error('ERROR:', error);
36-
internalLogger.error({
37-
message: 'Failed to start unraid-api',
38-
error,
39-
});
43+
if (logger) {
44+
logger.error('ERROR:', error);
45+
}
4046
process.exit(1);
4147
}

‎api/src/core/log.ts‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,38 @@
11
import { pino } from 'pino';
22
import pretty from 'pino-pretty';
33

4-
import { API_VERSION, LOG_LEVEL, LOG_TYPE } from '@app/environment.js';
4+
import { API_VERSION, LOG_LEVEL, LOG_TYPE, PATHS_LOGS_FILE, SUPPRESS_LOGS } from '@app/environment.js';
55

66
export const levels = ['trace', 'debug', 'info', 'warn', 'error', 'fatal'] as const;
77

88
export type LogLevel = (typeof levels)[number];
99

1010
const level = levels[levels.indexOf(LOG_LEVEL.toLowerCase() as LogLevel)] ?? 'info';
1111

12-
export const logDestination = pino.destination();
12+
const nullDestination = pino.destination({
13+
write() {
14+
// Suppress all logs
15+
},
16+
});
17+
18+
export const logDestination =
19+
process.env.SUPPRESS_LOGS === 'true' ? nullDestination : pino.destination();
20+
const localFileDestination = pino.destination({
21+
dest: PATHS_LOGS_FILE,
22+
sync: true,
23+
});
1324

14-
const stream =
15-
LOG_TYPE === 'pretty'
16-
? pretty({
17-
singleLine: true,
18-
hideObject: false,
19-
colorize: true,
20-
ignore: 'hostname,pid',
21-
destination: logDestination,
22-
})
23-
: logDestination;
25+
const stream = SUPPRESS_LOGS
26+
? nullDestination
27+
: LOG_TYPE === 'pretty'
28+
? pretty({
29+
singleLine: true,
30+
hideObject: false,
31+
colorize: true,
32+
ignore: 'hostname,pid',
33+
destination: logDestination,
34+
})
35+
: logDestination;
2436

2537
export const logger = pino(
2638
{
@@ -70,6 +82,7 @@ export const keyServerLogger = logger.child({ logger: 'key-server' });
7082
export const remoteAccessLogger = logger.child({ logger: 'remote-access' });
7183
export const remoteQueryLogger = logger.child({ logger: 'remote-query' });
7284
export const apiLogger = logger.child({ logger: 'api' });
85+
export const pluginLogger = logger.child({ logger: 'plugin', stream: localFileDestination });
7386

7487
export const loggers = [
7588
internalLogger,

‎api/src/environment.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export const LOG_LEVEL = process.env.LOG_LEVEL
9292
: process.env.ENVIRONMENT === 'production'
9393
? 'INFO'
9494
: 'DEBUG';
95+
export const SUPPRESS_LOGS = process.env.SUPPRESS_LOGS === 'true';
9596
export const MOTHERSHIP_GRAPHQL_LINK = process.env.MOTHERSHIP_GRAPHQL_LINK
9697
? process.env.MOTHERSHIP_GRAPHQL_LINK
9798
: ENVIRONMENT === 'staging'
@@ -101,7 +102,9 @@ export const MOTHERSHIP_GRAPHQL_LINK = process.env.MOTHERSHIP_GRAPHQL_LINK
101102
export const PM2_HOME = process.env.PM2_HOME ?? join(homedir(), '.pm2');
102103
export const PM2_PATH = join(import.meta.dirname, '../../', 'node_modules', 'pm2', 'bin', 'pm2');
103104
export const ECOSYSTEM_PATH = join(import.meta.dirname, '../../', 'ecosystem.config.json');
104-
export const LOGS_DIR = process.env.LOGS_DIR ?? '/var/log/unraid-api';
105+
export const PATHS_LOGS_DIR =
106+
process.env.PATHS_LOGS_DIR ?? process.env.LOGS_DIR ?? '/var/log/unraid-api';
107+
export const PATHS_LOGS_FILE = process.env.PATHS_LOGS_FILE ?? '/var/log/graphql-api.log';
105108

106109
export const PATHS_CONFIG_MODULES =
107110
process.env.PATHS_CONFIG_MODULES ?? '/boot/config/plugins/dynamix.my.servers/configs';

0 commit comments

Comments
 (0)