Skip to content

Commit

Permalink
Merge branch 'master' into ADO-2190-ai-assistant-ui
Browse files Browse the repository at this point in the history
* master:
  fix(core): Prevent occassional 429s on license init in multi-main setup (#9284)
  fix(Gmail Node): Remove duplicate options when creating drafts (#9299)
  refactor(core): Use logger for `packages/cli` messages (no-changelog) (#9302)
  fix(editor): Show MFA section to instance owner, even when external auth is enabled (#9301)
  fix(editor): Resolve `$vars` and `$secrets` in expressions in credentials fields (#9289)
  fix(n8n Form Trigger Node): Fix missing options when using respond to webhook (#9282)
  refactor(editor): Migrate `pushConnection` mixin to composable and remove collaboration store side effects (no-changelog) (#9249)
  • Loading branch information
MiloradFilipovic committed May 6, 2024
2 parents 2d2c899 + 22b6f90 commit daf7cdd
Show file tree
Hide file tree
Showing 39 changed files with 1,175 additions and 855 deletions.
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
34 changes: 30 additions & 4 deletions packages/cli/src/License.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ export class License {
private readonly usageMetricsService: UsageMetricsService,
) {}

/**
* Whether this instance should renew the license - on init and periodically.
*/
private renewalEnabled(instanceType: N8nInstanceType) {
if (instanceType !== 'main') return false;

const autoRenewEnabled = config.getEnv('license.autoRenewEnabled');

/**
* In multi-main setup, all mains start off with `unset` status and so renewal disabled.
* On becoming leader or follower, each will enable or disable renewal, respectively.
* This ensures the mains do not cause a 429 (too many requests) on license init.
*/
if (config.getEnv('multiMainSetup.enabled')) {
return autoRenewEnabled && config.getEnv('multiMainSetup.instanceType') === 'leader';
}

return autoRenewEnabled;
}

async init(instanceType: N8nInstanceType = 'main') {
if (this.manager) {
this.logger.warn('License manager already initialized or shutting down');
Expand All @@ -53,7 +73,6 @@ export class License {

const isMainInstance = instanceType === 'main';
const server = config.getEnv('license.serverUrl');
const autoRenewEnabled = isMainInstance && config.getEnv('license.autoRenewEnabled');
const offlineMode = !isMainInstance;
const autoRenewOffset = config.getEnv('license.autoRenewOffset');
const saveCertStr = isMainInstance
Expand All @@ -66,13 +85,15 @@ export class License {
? async () => await this.usageMetricsService.collectUsageMetrics()
: async () => [];

const renewalEnabled = this.renewalEnabled(instanceType);

try {
this.manager = new LicenseManager({
server,
tenantId: config.getEnv('license.tenantId'),
productIdentifier: `n8n-${N8N_VERSION}`,
autoRenewEnabled,
renewOnInit: autoRenewEnabled,
autoRenewEnabled: renewalEnabled,
renewOnInit: renewalEnabled,
autoRenewOffset,
offlineMode,
logger: this.logger,
Expand Down Expand Up @@ -126,7 +147,7 @@ export class License {

if (this.orchestrationService.isMultiMainSetupEnabled && !isMultiMainLicensed) {
this.logger.debug(
'[Multi-main setup] License changed with no support for multi-main setup - no new followers will be allowed to init. To restore multi-main setup, please upgrade to a license that supporst this feature.',
'[Multi-main setup] License changed with no support for multi-main setup - no new followers will be allowed to init. To restore multi-main setup, please upgrade to a license that supports this feature.',
);
}
}
Expand Down Expand Up @@ -335,4 +356,9 @@ export class License {
isWithinUsersLimit() {
return this.getUsersLimit() === UNLIMITED_LICENSE_QUOTA;
}

async reinit() {
this.manager?.reset();
await this.init();
}
}
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
12 changes: 7 additions & 5 deletions packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export abstract class BaseCommand extends Command {

protected shutdownService: ShutdownService = Container.get(ShutdownService);

protected license: License;

/**
* How long to wait for graceful shutdown before force killing the process.
*/
Expand Down Expand Up @@ -269,21 +271,21 @@ export abstract class BaseCommand extends Command {
}

async initLicense(): Promise<void> {
const license = Container.get(License);
await license.init(this.instanceType ?? 'main');
this.license = Container.get(License);
await this.license.init(this.instanceType ?? 'main');

const activationKey = config.getEnv('license.activationKey');

if (activationKey) {
const hasCert = (await license.loadCertStr()).length > 0;
const hasCert = (await this.license.loadCertStr()).length > 0;

if (hasCert) {
return this.logger.debug('Skipping license activation');
}

try {
this.logger.debug('Attempting license activation');
await license.activate(activationKey);
await this.license.activate(activationKey);
this.logger.debug('License init complete');
} catch (e) {
this.logger.error('Could not activate license', e as Error);
Expand Down Expand Up @@ -320,7 +322,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
6 changes: 4 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 @@ -211,9 +211,11 @@ export class Start extends BaseCommand {

orchestrationService.multiMainSetup
.on('leader-stepdown', async () => {
await this.license.reinit(); // to disable renewal
await this.activeWorkflowRunner.removeAllTriggerAndPollerBasedWorkflows();
})
.on('leader-takeover', async () => {
await this.license.reinit(); // to enable renewal
await this.activeWorkflowRunner.addAllTriggerAndPollerBasedWorkflows();
});
}
Expand Down Expand Up @@ -339,7 +341,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
Loading

0 comments on commit daf7cdd

Please sign in to comment.