Skip to content

Commit

Permalink
Merge branch 'master' into pay-1142-rbac-telemetry
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov committed Dec 18, 2023
2 parents 7d3f388 + 614f488 commit bc43fa2
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 55 deletions.
18 changes: 17 additions & 1 deletion packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { WorkflowHistoryManager } from '@/workflows/workflowHistory/workflowHist
export abstract class BaseCommand extends Command {
protected logger = Container.get(Logger);

protected externalHooks: IExternalHooksClass;
protected externalHooks?: IExternalHooksClass;

protected nodeTypes: NodeTypes;

Expand All @@ -40,6 +40,11 @@ export abstract class BaseCommand extends Command {

protected isShuttingDown = false;

/**
* How long to wait for graceful shutdown before force killing the process.
*/
protected gracefulShutdownTimeoutInS: number = config.getEnv('generic.gracefulShutdownTimeout');

async init(): Promise<void> {
await initErrorHandling();
initExpressionEvaluator();
Expand Down Expand Up @@ -309,9 +314,20 @@ export abstract class BaseCommand extends Command {
return;
}

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`);
const errorMsg = `Shutdown timed out after ${this.gracefulShutdownTimeoutInS} seconds`;
await this.exitWithCrash(errorMsg, new Error(errorMsg));
}, this.gracefulShutdownTimeoutInS * 1000);

this.logger.info(`Received ${signal}. Shutting down...`);
this.isShuttingDown = true;

await this.stopProcess();

clearTimeout(forceShutdownTimer);
};
}
}
9 changes: 1 addition & 8 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,7 @@ export class Start extends BaseCommand {
// Stop with trying to activate workflows that could not be activated
this.activeWorkflowRunner.removeAllQueuedWorkflowActivations();

await this.externalHooks.run('n8n.stop', []);

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
console.log('process exited after 30s');
await this.exitSuccessFully();
}, 30000);
await this.externalHooks?.run('n8n.stop', []);

// Shut down License manager to unclaim any floating entitlements
// Note: While this saves a new license cert to DB, the previous entitlements are still kept in memory so that the shutdown process can complete
Expand Down
8 changes: 1 addition & 7 deletions packages/cli/src/commands/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,7 @@ export class Webhook extends BaseCommand {
this.logger.info('\nStopping n8n...');

try {
await this.externalHooks.run('n8n.stop', []);

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
await this.exitSuccessFully();
}, 30000);
await this.externalHooks?.run('n8n.stop', []);

// Wait for active workflow executions to finish
const activeExecutionsInstance = Container.get(ActiveExecutions);
Expand Down
23 changes: 11 additions & 12 deletions packages/cli/src/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,15 @@ export class Worker extends BaseCommand {
await Worker.jobQueue.pause(true);

try {
await this.externalHooks.run('n8n.stop', []);
await this.externalHooks?.run('n8n.stop', []);

const maxStopTime = config.getEnv('queue.bull.gracefulShutdownTimeout') * 1000;

const stopTime = new Date().getTime() + maxStopTime;

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
await this.exitSuccessFully();
}, maxStopTime);
const hardStopTime = Date.now() + this.gracefulShutdownTimeoutInS;

// Wait for active workflow executions to finish
let count = 0;
while (Object.keys(Worker.runningJobs).length !== 0) {
if (count++ % 4 === 0) {
const waitLeft = Math.ceil((stopTime - new Date().getTime()) / 1000);
const waitLeft = Math.ceil((hardStopTime - Date.now()) / 1000);
this.logger.info(
`Waiting for ${
Object.keys(Worker.runningJobs).length
Expand Down Expand Up @@ -272,6 +264,13 @@ export class Worker extends BaseCommand {
}

async init() {
const configuredShutdownTimeout = config.getEnv('queue.bull.gracefulShutdownTimeout');
if (configuredShutdownTimeout) {
this.gracefulShutdownTimeoutInS = configuredShutdownTimeout;
this.logger.warn(
'QUEUE_WORKER_TIMEOUT has been deprecated. Rename it to N8N_GRACEFUL_SHUTDOWN_TIMEOUT.',
);
}
await this.initCrashJournal();

this.logger.debug('Starting n8n worker...');
Expand Down Expand Up @@ -483,7 +482,7 @@ export class Worker extends BaseCommand {
});

await new Promise<void>((resolve) => server.listen(port, () => resolve()));
await this.externalHooks.run('worker.ready');
await this.externalHooks?.run('worker.ready');
this.logger.info(`\nn8n worker health check via, port ${port}`);
}

Expand Down
9 changes: 8 additions & 1 deletion packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ export const schema = {
env: 'QUEUE_RECOVERY_INTERVAL',
},
gracefulShutdownTimeout: {
doc: 'How long should n8n wait for running executions before exiting worker process',
doc: '[DEPRECATED] (Use N8N_GRACEFUL_SHUTDOWN_TIMEOUT instead) How long should n8n wait for running executions before exiting worker process (seconds)',
format: Number,
default: 30,
env: 'QUEUE_WORKER_TIMEOUT',
Expand Down Expand Up @@ -497,6 +497,13 @@ export const schema = {
default: 'dev',
env: 'N8N_RELEASE_TYPE',
},

gracefulShutdownTimeout: {
doc: 'How long should n8n process wait for components to shut down before exiting the process (seconds)',
format: Number,
default: 30,
env: 'N8N_GRACEFUL_SHUTDOWN_TIMEOUT',
},
},

// How n8n can be reached (Editor & REST-API)
Expand Down
6 changes: 4 additions & 2 deletions packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1781,7 +1781,8 @@ export type CloudUpdateLinkSourceType =
| 'settings-users'
| 'variables'
| 'community-nodes'
| 'workflow-history';
| 'workflow-history'
| 'worker-view';

export type UTMCampaign =
| 'upgrade-custom-data-filter'
Expand All @@ -1799,7 +1800,8 @@ export type UTMCampaign =
| 'upgrade-variables'
| 'upgrade-community-nodes'
| 'upgrade-workflow-history'
| 'upgrade-advanced-permissions';
| 'upgrade-advanced-permissions'
| 'upgrade-worker-view';

export type N8nBanners = {
[key in BannerName]: {
Expand Down
6 changes: 2 additions & 4 deletions packages/editor-ui/src/components/ExecutionsList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -364,16 +364,14 @@ export default defineComponent({
workflows: [] as IWorkflowShortResponse[],
};
},
created() {
this.autoRefresh = this.autoRefreshEnabled;
},
mounted() {
setPageTitle(`n8n - ${this.pageTitle}`);
void this.handleAutoRefreshToggle();
document.addEventListener('visibilitychange', this.onDocumentVisibilityChange);
},
async created() {
this.autoRefresh = this.autoRefreshEnabled;
await this.loadWorkflows();
void this.externalHooks.run('executionsList.openDialog');
Expand Down Expand Up @@ -945,7 +943,7 @@ export default defineComponent({
}
},
async startAutoRefreshInterval() {
if (this.autoRefresh && this.route.name === VIEWS.WORKFLOW_EXECUTIONS) {
if (this.autoRefresh && this.route.name === VIEWS.EXECUTIONS) {
await this.loadAutoRefresh();
this.autoRefreshTimeout = setTimeout(() => {
void this.startAutoRefreshInterval();
Expand Down
4 changes: 4 additions & 0 deletions packages/editor-ui/src/components/WorkerList.ee.vue
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ export default defineComponent({
mounted() {
setPageTitle(`n8n - ${this.pageTitle}`);
this.isMounting = false;
this.$telemetry.track('User viewed worker view', {
instance_id: this.rootStore.instanceId,
});
},
beforeMount() {
if (window.Cypress !== undefined) {
Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/plugins/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@
"credentialEdit.oAuthButton.signInWithGoogle": "Sign in with Google",
"credentialEdit.credentialSharing.info.owner": "Sharing a credential allows people to use it in their workflows. They cannot access credential details.",
"credentialEdit.credentialSharing.info.reader": "You can view this credential because you have permission to read and share (and rename or delete it too).{notShared}",
"credentialEdit.credentialSharing.info.notShared": "To use it in a workflow you need to share it with yourself.",
"credentialEdit.credentialSharing.info.notShared": "You can share it with yourself or other users to use it in a workflow.",
"credentialEdit.credentialSharing.info.sharee": "Only {credentialOwnerName} can change who this credential is shared with",
"credentialEdit.credentialSharing.info.sharee.fallback": "the owner",
"credentialEdit.credentialSharing.select.placeholder": "Add users...",
Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/views/WorkerView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const settingsStore = useSettingsStore();
const uiStore = useUIStore();
const goToUpgrade = () => {
void uiStore.goToUpgrade('source-control', 'upgrade-source-control');
void uiStore.goToUpgrade('worker-view', 'upgrade-worker-view');
};
</script>

Expand Down
18 changes: 6 additions & 12 deletions patches/[email protected]
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
diff --git a/cjs/container-instance.class.js b/cjs/container-instance.class.js
index e473b1e652aa0b6e7462f7ba93fcef2812483b20..1e406113d68c401ee170c997afb53e5f71edeee2 100644
index e473b1e652aa0b6e7462f7ba93fcef2812483b20..9e57857e5584373b88a9fad3fbb37bbcc56b554a 100644
--- a/cjs/container-instance.class.js
+++ b/cjs/container-instance.class.js
@@ -209,6 +209,7 @@ class ContainerInstance {
// this allows us to support javascript where we don't have decorators and emitted metadata about dependencies
// need to be injected, and user can use provided container to get instances he needs
params.push(this);
+ if (process.env.NODE_ENV === 'production') Object.freeze(constructableTargetType.prototype);
value = new constructableTargetType(...params);
// TODO: Calling this here, leads to infinite loop, because @Inject decorator registerds a handler
// TODO: which calls Container.get, which will check if the requested type has a value set and if not
@@ -234,6 +235,7 @@ class ContainerInstance {
@@ -234,6 +234,9 @@ class ContainerInstance {
*/
initializeParams(target, paramTypes) {
return paramTypes.map((paramType, index) => {
+ if (paramType === undefined) throw new ReferenceError('Cannot inject an `undefined` dependency. Possibly a circular dependency detected');
+ if (paramType === undefined) {
+ throw new ReferenceError(`Circular dependency: Target${target.name}. Index: ${index} `);
+ }
const paramHandler = container_class_1.Container.handlers.find(handler => {
/**
* @Inject()-ed values are stored as parameter handlers and they reference their target
* @Inject()-ed values are stored as parameter handlers and they reference their target
12 changes: 6 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit bc43fa2

Please sign in to comment.