Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Use logger for packages/cli messages (no-changelog) #9302

Merged
merged 3 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/cli/src/AbstractServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export abstract class AbstractServer {

this.server.on('error', (error: Error & { code: string }) => {
if (error.code === 'EADDRINUSE') {
console.log(
this.logger.info(
`n8n's port ${PORT} is already in use. Do you have another instance of n8n running already?`,
);
process.exit(1);
Expand All @@ -167,7 +167,7 @@ export abstract class AbstractServer {

await this.setupHealthCheck();

console.log(`n8n ready on ${ADDRESS}, port ${PORT}`);
this.logger.info(`n8n ready on ${ADDRESS}, port ${PORT}`);
}

async start(): Promise<void> {
Expand Down Expand Up @@ -236,11 +236,11 @@ export abstract class AbstractServer {
await this.configure();

if (!inTest) {
console.log(`Version: ${N8N_VERSION}`);
this.logger.info(`Version: ${N8N_VERSION}`);

const defaultLocale = config.getEnv('defaultLocale');
if (defaultLocale !== 'en') {
console.log(`Locale: ${defaultLocale}`);
this.logger.info(`Locale: ${defaultLocale}`);
}

await this.externalHooks.run('n8n.ready', [this, config]);
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/ResponseHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Readable } from 'node:stream';

import { inDevelopment } from '@/constants';
import { ResponseError } from './errors/response-errors/abstract/response.error';
import Container from 'typedi';
import { Logger } from './Logger';

export function sendSuccessResponse(
res: Response,
Expand Down Expand Up @@ -83,7 +85,7 @@ export function sendErrorResponse(res: Response, error: Error) {

if (isResponseError(error)) {
if (inDevelopment) {
console.error(picocolors.red(error.httpStatusCode), error.message);
Container.get(Logger).error(picocolors.red([error.httpStatusCode, error.message].join(' ')));
}

//render custom 404 page for form triggers
Expand Down Expand Up @@ -112,7 +114,7 @@ export function sendErrorResponse(res: Response, error: Error) {

if (error instanceof NodeApiError) {
if (inDevelopment) {
console.error(picocolors.red(error.name), error.message);
Container.get(Logger).error([picocolors.red(error.name), error.message].join(' '));
}

Object.assign(response, error);
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/src/WorkflowExecuteAdditionalData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,10 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks {
]);
} catch (error) {
ErrorReporter.error(error);
console.error('There was a problem running hook "workflow.postExecute"', error);
Container.get(Logger).error(
'There was a problem running hook "workflow.postExecute"',
error,
);
}
}
},
Expand Down
9 changes: 6 additions & 3 deletions packages/cli/src/WorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,16 @@ export class WorkflowRunner {
]);
} catch (error) {
ErrorReporter.error(error);
console.error('There was a problem running hook "workflow.postExecute"', error);
this.logger.error('There was a problem running hook "workflow.postExecute"', error);
}
}
})
.catch((error) => {
ErrorReporter.error(error);
console.error('There was a problem running internal hook "onWorkflowPostExecute"', error);
this.logger.error(
'There was a problem running internal hook "onWorkflowPostExecute"',
error,
);
});
}

Expand Down Expand Up @@ -411,7 +414,7 @@ export class WorkflowRunner {
try {
job = await this.jobQueue.add(jobData, jobOptions);

console.log(`Started with job ID: ${job.id.toString()} (Execution ID: ${executionId})`);
this.logger.info(`Started with job ID: ${job.id.toString()} (Execution ID: ${executionId})`);

hooks = WorkflowExecuteAdditionalData.getWorkflowHooksWorkerMain(
data.executionMode,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export abstract class BaseCommand extends Command {
const forceShutdownTimer = setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after timeout no matter what
console.log(`process exited after ${this.gracefulShutdownTimeoutInS}s`);
this.logger.info(`process exited after ${this.gracefulShutdownTimeoutInS}s`);
const errorMsg = `Shutdown timed out after ${this.gracefulShutdownTimeoutInS} seconds`;
await this.exitWithCrash(errorMsg, new Error(errorMsg));
}, this.gracefulShutdownTimeoutInS * 1000);
Expand Down
34 changes: 17 additions & 17 deletions packages/cli/src/commands/executeBatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,11 @@ export class ExecuteBatch extends BaseCommand {
if (flags.snapshot !== undefined) {
if (fs.existsSync(flags.snapshot)) {
if (!fs.lstatSync(flags.snapshot).isDirectory()) {
console.log('The parameter --snapshot must be an existing directory');
this.logger.error('The parameter --snapshot must be an existing directory');
return;
}
} else {
console.log('The parameter --snapshot must be an existing directory');
this.logger.error('The parameter --snapshot must be an existing directory');
return;
}

Expand All @@ -191,11 +191,11 @@ export class ExecuteBatch extends BaseCommand {
if (flags.compare !== undefined) {
if (fs.existsSync(flags.compare)) {
if (!fs.lstatSync(flags.compare).isDirectory()) {
console.log('The parameter --compare must be an existing directory');
this.logger.error('The parameter --compare must be an existing directory');
return;
}
} else {
console.log('The parameter --compare must be an existing directory');
this.logger.error('The parameter --compare must be an existing directory');
return;
}

Expand All @@ -205,7 +205,7 @@ export class ExecuteBatch extends BaseCommand {
if (flags.output !== undefined) {
if (fs.existsSync(flags.output)) {
if (fs.lstatSync(flags.output).isDirectory()) {
console.log('The parameter --output must be a writable file');
this.logger.error('The parameter --output must be a writable file');
return;
}
}
Expand All @@ -225,7 +225,7 @@ export class ExecuteBatch extends BaseCommand {
const matchedIds = paramIds.filter((id) => re.exec(id));

if (matchedIds.length === 0) {
console.log(
this.logger.error(
'The parameter --ids must be a list of numeric IDs separated by a comma or a file with this content.',
);
return;
Expand All @@ -245,7 +245,7 @@ export class ExecuteBatch extends BaseCommand {
.filter((id) => re.exec(id)),
);
} else {
console.log('Skip list file not found. Exiting.');
this.logger.error('Skip list file not found. Exiting.');
return;
}
}
Expand Down Expand Up @@ -302,18 +302,18 @@ export class ExecuteBatch extends BaseCommand {

if (flags.output !== undefined) {
fs.writeFileSync(flags.output, this.formatJsonOutput(results));
console.log('\nExecution finished.');
console.log('Summary:');
console.log(`\tSuccess: ${results.summary.successfulExecutions}`);
console.log(`\tFailures: ${results.summary.failedExecutions}`);
console.log(`\tWarnings: ${results.summary.warningExecutions}`);
console.log('\nNodes successfully tested:');
this.logger.info('\nExecution finished.');
this.logger.info('Summary:');
this.logger.info(`\tSuccess: ${results.summary.successfulExecutions}`);
this.logger.info(`\tFailures: ${results.summary.failedExecutions}`);
this.logger.info(`\tWarnings: ${results.summary.warningExecutions}`);
this.logger.info('\nNodes successfully tested:');
Object.entries(results.coveredNodes).forEach(([nodeName, nodeCount]) => {
console.log(`\t${nodeName}: ${nodeCount}`);
this.logger.info(`\t${nodeName}: ${nodeCount}`);
});
console.log('\nCheck the JSON file for more details.');
this.logger.info('\nCheck the JSON file for more details.');
} else if (flags.shortOutput) {
console.log(
this.logger.info(
this.formatJsonOutput({
...results,
executions: results.executions.filter(
Expand All @@ -322,7 +322,7 @@ export class ExecuteBatch extends BaseCommand {
}),
);
} else {
console.log(this.formatJsonOutput(results));
this.logger.info(this.formatJsonOutput(results));
}

await this.stopProcess(true);
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class Start extends BaseCommand {
const editorUrl = Container.get(UrlService).baseUrl;

open(editorUrl, { wait: true }).catch(() => {
console.log(
this.logger.info(
`\nWas not able to open URL in browser. Please open manually by visiting:\n${editorUrl}\n`,
);
});
Expand Down Expand Up @@ -339,7 +339,7 @@ export class Start extends BaseCommand {
}

async catch(error: Error) {
console.log(error.stack);
if (error.stack) this.logger.error(error.stack);
await this.exitWithCrash('Exiting due to an error.', error);
}
}
8 changes: 4 additions & 4 deletions packages/cli/src/commands/update/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,24 @@ export class UpdateWorkflowCommand extends BaseCommand {
const { flags } = await this.parse(UpdateWorkflowCommand);

if (!flags.all && !flags.id) {
console.info('Either option "--all" or "--id" have to be set!');
this.logger.error('Either option "--all" or "--id" have to be set!');
return;
}

if (flags.all && flags.id) {
console.info(
this.logger.error(
'Either something else on top should be "--all" or "--id" can be set never both!',
);
return;
}

if (flags.active === undefined) {
console.info('No update flag like "--active=true" has been set!');
this.logger.error('No update flag like "--active=true" has been set!');
return;
}

if (!['false', 'true'].includes(flags.active)) {
console.info('Valid values for flag "--active" are only "false" or "true"!');
this.logger.error('Valid values for flag "--active" are only "false" or "true"!');
return;
}

Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/controllers/e2e.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import { MfaService } from '@/Mfa/mfa.service';
import { Push } from '@/push';
import { CacheService } from '@/services/cache/cache.service';
import { PasswordUtility } from '@/services/password.utility';
import Container from 'typedi';
import { Logger } from '@/Logger';

if (!inE2ETests) {
console.error('E2E endpoints only allowed during E2E tests');
Container.get(Logger).error('E2E endpoints only allowed during E2E tests');
process.exit(1);
}

Expand Down Expand Up @@ -149,7 +151,9 @@ export class E2EController {
`DELETE FROM ${table}; DELETE FROM sqlite_sequence WHERE name=${table};`,
);
} catch (error) {
console.warn('Dropping Table for E2E Reset error: ', error);
Container.get(Logger).warn('Dropping Table for E2E Reset error', {
error: error as Error,
});
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { MessageEventBusDestination } from './MessageEventBusDestination.ee';
import { isLogStreamingEnabled } from '../MessageEventBus/MessageEventBusHelper';
import { eventMessageGenericDestinationTestEvent } from '../EventMessageClasses/EventMessageGeneric';
import type { MessageEventBus, MessageWithCallback } from '../MessageEventBus/MessageEventBus';
import Container from 'typedi';
import { Logger } from '@/Logger';
export const isMessageEventBusDestinationSyslogOptions = (
candidate: unknown,
): candidate is MessageEventBusDestinationSyslogOptions => {
Expand Down Expand Up @@ -63,7 +65,7 @@ export class MessageEventBusDestinationSyslog
});
this.logger.debug(`MessageEventBusDestinationSyslog with id ${this.getId()} initialized`);
this.client.on('error', function (error) {
console.error(error);
Container.get(Logger).error(`${error.message}`);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class MessageEventBusDestinationWebhook
try {
JSON.parse(this.jsonQuery);
} catch {
console.log('JSON parameter need to be an valid JSON');
this.logger.error('JSON parameter need to be an valid JSON');
}
this.axiosRequestOptions.params = jsonParse(this.jsonQuery);
}
Expand All @@ -198,7 +198,7 @@ export class MessageEventBusDestinationWebhook
try {
JSON.parse(this.jsonHeaders);
} catch {
console.log('JSON parameter need to be an valid JSON');
this.logger.error('JSON parameter need to be an valid JSON');
}
this.axiosRequestOptions.headers = jsonParse(this.jsonHeaders);
}
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/help.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { Help } from '@oclif/core';
import Container from 'typedi';
import { Logger } from 'winston';

// oclif expects a default export
// eslint-disable-next-line import/no-default-export
export default class CustomHelp extends Help {
async showRootHelp() {
console.log(
Container.get(Logger).info(
'You can find up to date information about the CLI here:\nhttps://docs.n8n.io/hosting/cli-commands/',
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@ import { getN8nPackageJson, inDevelopment } from '@/constants';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { RiskReporter, Risk, n8n } from '@/security-audit/types';
import { isApiEnabled } from '@/PublicApi';
import { Logger } from '@/Logger';

@Service()
export class InstanceRiskReporter implements RiskReporter {
constructor(private readonly instanceSettings: InstanceSettings) {}
constructor(
private readonly instanceSettings: InstanceSettings,
private readonly logger: Logger,
) {}

async report(workflows: WorkflowEntity[]) {
const unprotectedWebhooks = this.getUnprotectedWebhookNodes(workflows);
Expand Down Expand Up @@ -174,7 +178,7 @@ export class InstanceRiskReporter implements RiskReporter {
versions = await this.getNextVersions(localVersion).then((v) => this.removeIconData(v));
} catch (error) {
if (inDevelopment) {
console.error('Failed to fetch n8n versions. Skipping outdated instance report...');
this.logger.error('Failed to fetch n8n versions. Skipping outdated instance report...');
}
return null;
}
Expand Down
3 changes: 0 additions & 3 deletions packages/cli/test/integration/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,6 @@ describe('PATCH /credentials/:id', () => {
.patch(`/credentials/${savedCredential.id}`)
.send(invalidPayload);

if (response.statusCode === 500) {
console.log(response.statusCode, response.body);
}
expect(response.statusCode).toBe(400);
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/unit/services/redis.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('RedisService', () => {

const mockHandler = jest.fn();
mockHandler.mockImplementation((stream: string, id: string, message: string[]) => {
console.log('Received message', stream, id, message);
Container.get(Logger).info('Received message', { stream, id, message });
});
consumer.addMessageHandler('some handler', mockHandler);

Expand Down
Loading