From b425f209a8579c9e5baece637e7862e29ba45a5b Mon Sep 17 00:00:00 2001 From: AlexisMora Date: Fri, 24 Oct 2025 17:21:59 +0200 Subject: [PATCH] Fix: print human readable errors on terminal --- src/commands/add-cert.ts | 1 - src/commands/config.ts | 1 - src/commands/create-folder.ts | 1 - src/commands/delete-permanently-file.ts | 1 - src/commands/delete-permanently-folder.ts | 1 - src/commands/download-file.ts | 1 - src/commands/list.ts | 1 - src/commands/login.ts | 1 - src/commands/logout.ts | 1 - src/commands/logs.ts | 1 - src/commands/move-file.ts | 1 - src/commands/move-folder.ts | 1 - src/commands/rename-file.ts | 1 - src/commands/rename-folder.ts | 1 - src/commands/trash-clear.ts | 1 - src/commands/trash-file.ts | 1 - src/commands/trash-folder.ts | 1 - src/commands/trash-list.ts | 1 - src/commands/trash-restore-file.ts | 1 - src/commands/trash-restore-folder.ts | 1 - src/commands/upload-file.ts | 3 +-- src/commands/webdav-config.ts | 1 - src/commands/webdav.ts | 1 - src/commands/whoami.ts | 1 - src/hooks/prerun/auth_check.ts | 1 - src/utils/cli.utils.ts | 4 +--- src/utils/errors.utils.ts | 15 ++++++++---- src/utils/logger.utils.ts | 16 ------------- src/webdav/middewares/auth.middleware.ts | 13 +++++++---- src/webdav/middewares/errors.middleware.ts | 11 +++++++-- test/commands/login.test.ts | 15 +++++++++--- test/utils/errors.utils.test.ts | 27 +++++++++++++--------- 32 files changed, 58 insertions(+), 70 deletions(-) diff --git a/src/commands/add-cert.ts b/src/commands/add-cert.ts index 23c6dc73..da02f2d9 100644 --- a/src/commands/add-cert.ts +++ b/src/commands/add-cert.ts @@ -41,7 +41,6 @@ export default class AddCert extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/config.ts b/src/commands/config.ts index 07fd59e1..e026a409 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -45,7 +45,6 @@ export default class Config extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/create-folder.ts b/src/commands/create-folder.ts index 348fe709..df539e8f 100644 --- a/src/commands/create-folder.ts +++ b/src/commands/create-folder.ts @@ -70,7 +70,6 @@ export default class CreateFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/delete-permanently-file.ts b/src/commands/delete-permanently-file.ts index 6905be7e..2f12e2d3 100644 --- a/src/commands/delete-permanently-file.ts +++ b/src/commands/delete-permanently-file.ts @@ -47,7 +47,6 @@ export default class DeletePermanentlyFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/delete-permanently-folder.ts b/src/commands/delete-permanently-folder.ts index 8af70069..1cbd7fff 100644 --- a/src/commands/delete-permanently-folder.ts +++ b/src/commands/delete-permanently-folder.ts @@ -47,7 +47,6 @@ export default class DeletePermanentlyFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/download-file.ts b/src/commands/download-file.ts index 6f5099d1..2b58330c 100644 --- a/src/commands/download-file.ts +++ b/src/commands/download-file.ts @@ -118,7 +118,6 @@ export default class DownloadFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/list.ts b/src/commands/list.ts index 4a6c46a3..c2c40d63 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -82,7 +82,6 @@ export default class List extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/login.ts b/src/commands/login.ts index 1f4c3562..f528f0b9 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -92,7 +92,6 @@ export default class Login extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/logout.ts b/src/commands/logout.ts index 56b56174..5ab305e2 100644 --- a/src/commands/logout.ts +++ b/src/commands/logout.ts @@ -32,7 +32,6 @@ export default class Logout extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/logs.ts b/src/commands/logs.ts index 902f55dc..fd7b0205 100644 --- a/src/commands/logs.ts +++ b/src/commands/logs.ts @@ -22,7 +22,6 @@ export default class Logs extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/move-file.ts b/src/commands/move-file.ts index 19968197..71eff208 100644 --- a/src/commands/move-file.ts +++ b/src/commands/move-file.ts @@ -52,7 +52,6 @@ export default class MoveFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/move-folder.ts b/src/commands/move-folder.ts index 9e697258..3021acc1 100644 --- a/src/commands/move-folder.ts +++ b/src/commands/move-folder.ts @@ -54,7 +54,6 @@ export default class MoveFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/rename-file.ts b/src/commands/rename-file.ts index e1b88016..1e26365e 100644 --- a/src/commands/rename-file.ts +++ b/src/commands/rename-file.ts @@ -52,7 +52,6 @@ export default class RenameFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/rename-folder.ts b/src/commands/rename-folder.ts index b15714b6..92caf358 100644 --- a/src/commands/rename-folder.ts +++ b/src/commands/rename-folder.ts @@ -47,7 +47,6 @@ export default class RenameFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-clear.ts b/src/commands/trash-clear.ts index 39bf22e8..2ecb1301 100644 --- a/src/commands/trash-clear.ts +++ b/src/commands/trash-clear.ts @@ -54,7 +54,6 @@ export default class TrashClear extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-file.ts b/src/commands/trash-file.ts index 1e2ea6e8..d2bca1f3 100644 --- a/src/commands/trash-file.ts +++ b/src/commands/trash-file.ts @@ -42,7 +42,6 @@ export default class TrashFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-folder.ts b/src/commands/trash-folder.ts index 3ce6abd7..635d78f6 100644 --- a/src/commands/trash-folder.ts +++ b/src/commands/trash-folder.ts @@ -42,7 +42,6 @@ export default class TrashFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-list.ts b/src/commands/trash-list.ts index 1d62af0f..4b1c99d1 100644 --- a/src/commands/trash-list.ts +++ b/src/commands/trash-list.ts @@ -67,7 +67,6 @@ export default class TrashList extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-restore-file.ts b/src/commands/trash-restore-file.ts index 43299062..b18ceb4d 100644 --- a/src/commands/trash-restore-file.ts +++ b/src/commands/trash-restore-file.ts @@ -54,7 +54,6 @@ export default class TrashRestoreFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/trash-restore-folder.ts b/src/commands/trash-restore-folder.ts index 31b68dd8..f20b060f 100644 --- a/src/commands/trash-restore-folder.ts +++ b/src/commands/trash-restore-folder.ts @@ -56,7 +56,6 @@ export default class TrashRestoreFolder extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/upload-file.ts b/src/commands/upload-file.ts index 43a155bf..ac703d98 100644 --- a/src/commands/upload-file.ts +++ b/src/commands/upload-file.ts @@ -157,7 +157,7 @@ export default class UploadFile extends Command { } } } catch (error) { - ErrorUtils.report(this.error.bind(this), error, { command: this.id }); + ErrorUtils.report(error, { command: this.id }); } progressBar?.update(100); @@ -184,7 +184,6 @@ export default class UploadFile extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/webdav-config.ts b/src/commands/webdav-config.ts index 93e14f4b..a75b9d60 100644 --- a/src/commands/webdav-config.ts +++ b/src/commands/webdav-config.ts @@ -93,7 +93,6 @@ export default class WebDAVConfig extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/webdav.ts b/src/commands/webdav.ts index 220feecd..69e9ee43 100644 --- a/src/commands/webdav.ts +++ b/src/commands/webdav.ts @@ -68,7 +68,6 @@ export default class Webdav extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/commands/whoami.ts b/src/commands/whoami.ts index 4e8afc51..d6a32ef9 100644 --- a/src/commands/whoami.ts +++ b/src/commands/whoami.ts @@ -47,7 +47,6 @@ export default class Whoami extends Command { error, command: this.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag: flags['json'], }); this.exit(1); diff --git a/src/hooks/prerun/auth_check.ts b/src/hooks/prerun/auth_check.ts index 46431f2e..0cb2af67 100644 --- a/src/hooks/prerun/auth_check.ts +++ b/src/hooks/prerun/auth_check.ts @@ -27,7 +27,6 @@ const hook: Hook<'prerun'> = async function (opts) { error: err, command: Command.id, logReporter: this.log.bind(this), - errorReporter: this.error.bind(this), jsonFlag, }); opts.context.exit(1); diff --git a/src/utils/cli.utils.ts b/src/utils/cli.utils.ts index 0a810b1f..e3c1cfb8 100644 --- a/src/utils/cli.utils.ts +++ b/src/utils/cli.utils.ts @@ -175,14 +175,12 @@ export class CLIUtils { static readonly catchError = ({ error, logReporter, - errorReporter, command, jsonFlag, }: { error: Error; command?: string; logReporter: (message: string) => void; - errorReporter: (message: string) => void; jsonFlag?: boolean; }) => { let message; @@ -196,7 +194,7 @@ export class CLIUtils { if (jsonFlag) { CLIUtils.consoleLog(JSON.stringify({ success: false, message })); } else { - ErrorUtils.report(errorReporter, error, { command }); + ErrorUtils.report(error, { command }); CLIUtils.error(logReporter, message); } }; diff --git a/src/utils/errors.utils.ts b/src/utils/errors.utils.ts index 15e2226f..be84edfd 100644 --- a/src/utils/errors.utils.ts +++ b/src/utils/errors.utils.ts @@ -1,11 +1,18 @@ +import { logger } from './logger.utils'; +import { types } from 'node:util'; + +export function isError(error: unknown): error is Error { + return types.isNativeError(error); +} + export class ErrorUtils { - static report(reporter: (error: string) => void, error: unknown, props: Record = {}) { - if (error instanceof Error) { - reporter( + static report(error: unknown, props: Record = {}) { + if (isError(error)) { + logger.error( `[REPORTED_ERROR]: ${error.message}\nProperties => ${JSON.stringify(props, null, 2)}\nStack => ${error.stack}`, ); } else { - reporter(`[REPORTED_ERROR]: ${JSON.stringify(error)}\nProperties => ${JSON.stringify(props, null, 2)}\n`); + logger.error(`[REPORTED_ERROR]: ${JSON.stringify(error)}\nProperties => ${JSON.stringify(props, null, 2)}\n`); } } } diff --git a/src/utils/logger.utils.ts b/src/utils/logger.utils.ts index 78f249a7..fae60614 100644 --- a/src/utils/logger.utils.ts +++ b/src/utils/logger.utils.ts @@ -49,19 +49,3 @@ export const webdavLogger = winston.createLogger({ }), ], }); - -if (process.env.NODE_ENV !== 'production') { - webdavLogger.add( - new winston.transports.Console({ - format: winston.format.simple(), - }), - ); -} - -if (process.env.NODE_ENV !== 'production') { - logger.add( - new winston.transports.Console({ - format: winston.format.simple(), - }), - ); -} diff --git a/src/webdav/middewares/auth.middleware.ts b/src/webdav/middewares/auth.middleware.ts index 1c4179db..f00c45d1 100644 --- a/src/webdav/middewares/auth.middleware.ts +++ b/src/webdav/middewares/auth.middleware.ts @@ -3,6 +3,7 @@ import { SdkManager } from '../../services/sdk-manager.service'; import { AuthService } from '../../services/auth.service'; import { webdavLogger } from '../../utils/logger.utils'; import { XMLUtils } from '../../utils/xml.utils'; +import { isError } from '../../utils/errors.utils'; export const AuthMiddleware = (authService: AuthService): RequestHandler => { return (_, res, next) => { @@ -13,12 +14,14 @@ export const AuthMiddleware = (authService: AuthService): RequestHandler => { next(); } catch (error) { let message = 'Authentication required to access this resource.'; - if ('message' in (error as Error) && (error as Error).message.trim().length > 0) { - message = (error as Error).message; + if (isError(error)) { + message = error.message; + if (error.stack) { + webdavLogger.error(`Error from AuthMiddleware: ${message}\nStack: ${error.stack}`); + } else { + webdavLogger.error(`Error from AuthMiddleware: ${message}`); + } } - - webdavLogger.error('Error from AuthMiddleware: ' + message); - const errorBodyXML = XMLUtils.toWebDavXML( { [XMLUtils.addDefaultNamespace('responsedescription')]: message, diff --git a/src/webdav/middewares/errors.middleware.ts b/src/webdav/middewares/errors.middleware.ts index 98f80d50..6a4f7b28 100644 --- a/src/webdav/middewares/errors.middleware.ts +++ b/src/webdav/middewares/errors.middleware.ts @@ -1,14 +1,21 @@ import { ErrorRequestHandler } from 'express'; import { webdavLogger } from '../../utils/logger.utils'; import { XMLUtils } from '../../utils/xml.utils'; +import { isError } from '../../utils/errors.utils'; // eslint-disable-next-line @typescript-eslint/no-unused-vars export const ErrorHandlingMiddleware: ErrorRequestHandler = (err, req, res, _) => { - webdavLogger.error(`[ERROR MIDDLEWARE] [${req.method.toUpperCase()} - ${req.url}]`, err); + const message = isError(err) ? err.message : 'Something went wrong'; + + if (isError(err) && err.stack) { + webdavLogger.error(`[ERROR MIDDLEWARE] [${req.method.toUpperCase()} - ${req.url}] ${message}\nStack: ${err.stack}`); + } else { + webdavLogger.error(`[ERROR MIDDLEWARE] [${req.method.toUpperCase()} - ${req.url}] ${message}`); + } const errorBodyXML = XMLUtils.toWebDavXML( { - [XMLUtils.addDefaultNamespace('responsedescription')]: 'message' in err ? err.message : 'Something went wrong', + [XMLUtils.addDefaultNamespace('responsedescription')]: message, }, {}, 'error', diff --git a/test/commands/login.test.ts b/test/commands/login.test.ts index 06ca9115..8d7aa2f5 100644 --- a/test/commands/login.test.ts +++ b/test/commands/login.test.ts @@ -6,6 +6,12 @@ import Login from '../../src/commands/login'; import { AuthService } from '../../src/services/auth.service'; import { CLIUtils, NoFlagProvidedError } from '../../src/utils/cli.utils'; +vi.mock('../../src/utils/logger.utils', () => ({ + logger: { + error: vi.fn(), + }, +})); + describe('Login Command', () => { beforeEach(() => { vi.restoreAllMocks(); @@ -26,7 +32,8 @@ describe('Login Command', () => { await Login.run(['--non-interactive', `--password="${UserLoginFixture.password}"`]); fail('Expected function to throw an error, but it did not.'); } catch (error) { - expect((error as Error).message).to.contain(new NoFlagProvidedError('email').message); + expect((error as Error).message).to.contain('EEXIT: 1'); + expect((error as Error & { oclif?: { exit: number } }).oclif?.exit).to.equal(1); } expect(getValueFromFlagsSpy).toHaveBeenCalledOnce(); @@ -50,7 +57,8 @@ describe('Login Command', () => { await Login.run(['--non-interactive', `--email="${UserLoginFixture.email}"`]); fail('Expected function to throw an error, but it did not.'); } catch (error) { - expect((error as Error).message).to.contain(new NoFlagProvidedError('password').message); + expect((error as Error).message).to.contain('EEXIT: 1'); + expect((error as Error & { oclif?: { exit: number } }).oclif?.exit).to.equal(1); } expect(getValueFromFlagsSpy).toHaveBeenCalledTimes(2); @@ -78,7 +86,8 @@ describe('Login Command', () => { ]); fail('Expected function to throw an error, but it did not.'); } catch (error) { - expect((error as Error).message).to.contain(new NoFlagProvidedError('twofactor').message); + expect((error as Error).message).to.contain('EEXIT: 1'); + expect((error as Error & { oclif?: { exit: number } }).oclif?.exit).to.equal(1); } expect(getValueFromFlagsSpy).toHaveBeenCalledTimes(3); diff --git a/test/utils/errors.utils.test.ts b/test/utils/errors.utils.test.ts index ea0c13dc..4ad245d2 100644 --- a/test/utils/errors.utils.test.ts +++ b/test/utils/errors.utils.test.ts @@ -1,33 +1,38 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ErrorUtils } from '../../src/utils/errors.utils'; +import { logger } from '../../src/utils/logger.utils'; -describe('Errors Utils', () => { - const reporter = vi.fn(); +vi.mock('../../src/utils/logger.utils', () => ({ + logger: { + error: vi.fn(), + }, +})); +describe('Errors Utils', () => { beforeEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); }); - it('When reporting an error, should call reporter with the expected message and properties', () => { + it('When reporting an error, should log with the expected message and properties', () => { const error = new Error('Test Error'); const props = { key: 'value' }; - ErrorUtils.report(reporter, error, props); + ErrorUtils.report(error, props); - expect(reporter).toHaveBeenCalledOnce(); - expect(reporter).toHaveBeenCalledWith( + expect(logger.error).toHaveBeenCalledOnce(); + expect(logger.error).toHaveBeenCalledWith( `[REPORTED_ERROR]: ${error.message}\nProperties => ${JSON.stringify(props, null, 2)}\nStack => ${error.stack}`, ); }); - it('When reporting an object, should call reporter with the expected message and properties', () => { + it('When reporting an object, should log with the expected message and properties', () => { const error = { data: 'error data' }; const props = { key: 'value' }; - ErrorUtils.report(reporter, error, props); + ErrorUtils.report(error, props); - expect(reporter).toHaveBeenCalledOnce(); - expect(reporter).toHaveBeenCalledWith( + expect(logger.error).toHaveBeenCalledOnce(); + expect(logger.error).toHaveBeenCalledWith( `[REPORTED_ERROR]: ${JSON.stringify(error)}\nProperties => ${JSON.stringify(props, null, 2)}\n`, ); });