Skip to content

Commit

Permalink
feat(core): Remove all floating promises. Enforce `@typescript-eslint…
Browse files Browse the repository at this point in the history
…/no-floating-promises` (#6281)
  • Loading branch information
netroy committed May 24, 2023
1 parent 5d2f474 commit e046f65
Show file tree
Hide file tree
Showing 33 changed files with 94 additions and 120 deletions.
1 change: 0 additions & 1 deletion packages/@n8n_io/eslint-config/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ const config = (module.exports = {
'@typescript-eslint/naming-convention': 'off',
'@typescript-eslint/no-duplicate-imports': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/no-floating-promises': 'off',
'@typescript-eslint/no-loop-func': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-shadow': 'off',
Expand Down
14 changes: 6 additions & 8 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
/* eslint-disable no-param-reassign */
/* eslint-disable no-await-in-loop */
/* eslint-disable no-restricted-syntax */
/* eslint-disable @typescript-eslint/no-floating-promises */
/* eslint-disable @typescript-eslint/no-shadow */
/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
Expand Down Expand Up @@ -306,8 +305,7 @@ export class ActiveWorkflowRunner {

return new Promise((resolve, reject) => {
const executionMode = 'webhook';
// @ts-ignore
WebhookHelpers.executeWebhook(
void WebhookHelpers.executeWebhook(
workflow,
webhookData,
workflowData,
Expand Down Expand Up @@ -627,7 +625,7 @@ export class ActiveWorkflowRunner {
): void => {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
Logger.debug(`Received event to trigger execution for workflow "${workflow.name}"`);
WorkflowHelpers.saveStaticData(workflow);
void WorkflowHelpers.saveStaticData(workflow);
const executePromise = this.runWorkflow(
workflowData,
node,
Expand All @@ -638,14 +636,14 @@ export class ActiveWorkflowRunner {
);

if (donePromise) {
executePromise.then((executionId) => {
void executePromise.then((executionId) => {
this.activeExecutions
.getPostExecutePromise(executionId)
.then(donePromise.resolve)
.catch(donePromise.reject);
});
} else {
executePromise.catch(Logger.error);
void executePromise.catch(Logger.error);
}
};

Expand Down Expand Up @@ -684,7 +682,7 @@ export class ActiveWorkflowRunner {
): void => {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
Logger.debug(`Received trigger for workflow "${workflow.name}"`);
WorkflowHelpers.saveStaticData(workflow);
void WorkflowHelpers.saveStaticData(workflow);
// eslint-disable-next-line id-denylist
const executePromise = this.runWorkflow(
workflowData,
Expand All @@ -696,7 +694,7 @@ export class ActiveWorkflowRunner {
);

if (donePromise) {
executePromise.then((executionId) => {
void executePromise.then((executionId) => {
this.activeExecutions
.getPostExecutePromise(executionId)
.then(donePromise.resolve)
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -552,12 +552,12 @@ export class Server extends AbstractServer {

// Check for basic auth credentials if activated
if (config.getEnv('security.basicAuth.active')) {
await setupBasicAuth(this.app, config, authIgnoreRegex);
setupBasicAuth(this.app, config, authIgnoreRegex);
}

// Check for and validate JWT if configured
if (config.getEnv('security.jwtAuth.active')) {
await setupExternalJWTAuth(this.app, config, authIgnoreRegex);
setupExternalJWTAuth(this.app, config, authIgnoreRegex);
}

// ----------------------------------------
Expand Down
3 changes: 1 addition & 2 deletions packages/cli/src/TestWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,7 @@ export class TestWebhooks {

if (!foundWebhook) {
// As it removes all webhooks of the workflow execute only once
// eslint-disable-next-line @typescript-eslint/no-floating-promises
activeWebhooks.removeWorkflow(workflow);
void activeWebhooks.removeWorkflow(workflow);
}

foundWebhook = true;
Expand Down
6 changes: 2 additions & 4 deletions packages/cli/src/WaitTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
/* eslint-disable @typescript-eslint/naming-convention */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/restrict-template-expressions */
/* eslint-disable @typescript-eslint/no-floating-promises */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import {
ErrorReporterProxy as ErrorReporter,
LoggerProxy as Logger,
Expand Down Expand Up @@ -40,10 +38,10 @@ export class WaitTracker {
constructor() {
// Poll every 60 seconds a list of upcoming executions
this.mainTimer = setInterval(() => {
this.getWaitingExecutions();
void this.getWaitingExecutions();
}, 60000);

this.getWaitingExecutions();
void this.getWaitingExecutions();
}

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
Expand Down
3 changes: 1 addition & 2 deletions packages/cli/src/WaitingWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ export class WaitingWebhooks {

return new Promise((resolve, reject) => {
const executionMode = 'webhook';
// eslint-disable-next-line @typescript-eslint/no-floating-promises
WebhookHelpers.executeWebhook(
void WebhookHelpers.executeWebhook(
workflow,
webhookData,
workflowData as IWorkflowDb,
Expand Down
27 changes: 11 additions & 16 deletions packages/cli/src/WorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/restrict-template-expressions */
/* eslint-disable @typescript-eslint/no-shadow */
/* eslint-disable @typescript-eslint/no-floating-promises */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/prefer-optional-chain */
/* eslint-disable no-param-reassign */
Expand Down Expand Up @@ -75,8 +74,7 @@ export class WorkflowRunner {
* The process did send a hook message so execute the appropriate hook
*/
processHookMessage(workflowHooks: WorkflowHooks, hookData: IProcessMessageDataHook) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
workflowHooks.executeHookFunctions(hookData.hook, hookData.parameters);
void workflowHooks.executeHookFunctions(hookData.hook, hookData.parameters);
}

/**
Expand Down Expand Up @@ -305,11 +303,8 @@ export class WorkflowRunner {
error,
error.node,
);
additionalData.hooks
.executeHookFunctions('workflowExecuteAfter', [failedExecution])
.then(() => {
this.activeExecutions.remove(executionId, failedExecution);
});
await additionalData.hooks.executeHookFunctions('workflowExecuteAfter', [failedExecution]);
this.activeExecutions.remove(executionId, failedExecution);
return executionId;
}

Expand Down Expand Up @@ -382,7 +377,7 @@ export class WorkflowRunner {
if (workflowTimeout > 0) {
const timeout = Math.min(workflowTimeout, config.getEnv('executions.maxTimeout')) * 1000; // as seconds
executionTimeout = setTimeout(() => {
this.activeExecutions.stopExecution(executionId, 'timeout');
void this.activeExecutions.stopExecution(executionId, 'timeout');
}, timeout);
}

Expand All @@ -395,15 +390,15 @@ export class WorkflowRunner {
fullRunData.status = this.activeExecutions.getStatus(executionId);
this.activeExecutions.remove(executionId, fullRunData);
})
.catch((error) => {
.catch(async (error) =>
this.processError(
error,
new Date(),
data.executionMode,
executionId,
additionalData.hooks,
);
});
),
);
} catch (error) {
await this.processError(
error,
Expand Down Expand Up @@ -467,7 +462,7 @@ export class WorkflowRunner {

// Normally also workflow should be supplied here but as it only used for sending
// data to editor-UI is not needed.
hooks.executeHookFunctions('workflowExecuteBefore', []);
await hooks.executeHookFunctions('workflowExecuteBefore', []);
} catch (error) {
// We use "getWorkflowHooksWorkerExecuter" as "getWorkflowHooksWorkerMain" does not contain the
// "workflowExecuteAfter" which we require.
Expand Down Expand Up @@ -585,7 +580,7 @@ export class WorkflowRunner {
this.activeExecutions.remove(executionId, runData);
// Normally also static data should be supplied here but as it only used for sending
// data to editor-UI is not needed.
hooks.executeHookFunctions('workflowExecuteAfter', [runData]);
await hooks.executeHookFunctions('workflowExecuteAfter', [runData]);
try {
// Check if this execution data has to be removed from database
// based on workflow settings.
Expand Down Expand Up @@ -668,7 +663,7 @@ export class WorkflowRunner {
let workflowTimeout = workflowSettings.executionTimeout ?? config.getEnv('executions.timeout'); // initialize with default

const processTimeoutFunction = (timeout: number) => {
this.activeExecutions.stopExecution(executionId, 'timeout');
void this.activeExecutions.stopExecution(executionId, 'timeout');
executionTimeout = setTimeout(() => subprocess.kill(), Math.max(timeout * 0.2, 5000)); // minimum 5 seconds
};

Expand Down Expand Up @@ -732,7 +727,7 @@ export class WorkflowRunner {
const timeoutError = new WorkflowOperationError('Workflow execution timed out!');

// No need to add hook here as the subprocess takes care of calling the hooks
this.processError(timeoutError, startedAt, data.executionMode, executionId);
await this.processError(timeoutError, startedAt, data.executionMode, executionId);
} else if (message.type === 'startExecution') {
const executionId = await this.activeExecutions.add(message.data.runData);
childExecutionIds.push(executionId);
Expand Down
3 changes: 1 addition & 2 deletions packages/cli/src/WorkflowRunnerProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,7 @@ process.on('message', async (message: IProcessMessage) => {
status: 'canceled',
};

// eslint-disable-next-line @typescript-eslint/no-floating-promises
workflowRunner.sendHookToParentProcess('workflowExecuteAfter', [runData]);
await workflowRunner.sendHookToParentProcess('workflowExecuteAfter', [runData]);
}

await sendToParentProcess(message.type === 'timeout' ? message.type : 'end', {
Expand Down
4 changes: 1 addition & 3 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ export class Start extends BaseCommand {
if (dbType === 'sqlite') {
const shouldRunVacuum = config.getEnv('database.sqlite.executeVacuumOnStartup');
if (shouldRunVacuum) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
await Db.collections.Execution.query('VACUUM;');
}
}
Expand Down Expand Up @@ -360,8 +359,7 @@ export class Start extends BaseCommand {
this.openBrowser();
} else if (key.charCodeAt(0) === 3) {
// Ctrl + c got pressed
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.stopProcess();
void this.stopProcess();
} else {
// When anything else got pressed, record it and send it on enter into the child process
// eslint-disable-next-line no-lonely-if
Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ export class Worker extends BaseCommand {
LoggerProxy.info('Stopping n8n...');

// Stop accepting new jobs
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Worker.jobQueue.pause(true);
await Worker.jobQueue.pause(true);

try {
await this.externalHooks.run('n8n.stop', []);
Expand Down Expand Up @@ -239,8 +238,9 @@ export class Worker extends BaseCommand {
const queue = Container.get(Queue);
await queue.init();
Worker.jobQueue = queue.getBullObjectInstance();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Worker.jobQueue.process(flags.concurrency, async (job) => this.runJob(job, this.nodeTypes));
void Worker.jobQueue.process(flags.concurrency, async (job) =>
this.runJob(job, this.nodeTypes),
);

this.logger.info('\nn8n worker is now ready');
this.logger.info(` * Version: ${N8N_VERSION}`);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/middlewares/basicAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { compare } from 'bcryptjs';
import type { Config } from '@/config';
import { basicAuthAuthorizationError } from '@/ResponseHelper';

export const setupBasicAuth = async (app: Application, config: Config, authIgnoreRegex: RegExp) => {
export const setupBasicAuth = (app: Application, config: Config, authIgnoreRegex: RegExp) => {
const basicAuthUser = config.getEnv('security.basicAuth.user');
if (basicAuthUser === '') {
throw new Error('Basic auth is activated but no user got defined. Please set one!');
Expand Down
6 changes: 1 addition & 5 deletions packages/cli/src/middlewares/externalJWTAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import jwks from 'jwks-rsa';
import type { Config } from '@/config';
import { jwtAuthAuthorizationError } from '@/ResponseHelper';

export const setupExternalJWTAuth = async (
app: Application,
config: Config,
authIgnoreRegex: RegExp,
) => {
export const setupExternalJWTAuth = (app: Application, config: Config, authIgnoreRegex: RegExp) => {
const jwtAuthHeader = config.getEnv('security.jwtAuth.jwtHeader');
if (jwtAuthHeader === '') {
throw new Error('JWT auth is activated but no request header was defined. Please set one!');
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/credentials.ee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ let sharingSpy: jest.SpyInstance<boolean>;
beforeAll(async () => {
const app = await utils.initTestServer({ endpointGroups: ['credentials'] });

utils.initConfigFile();
await utils.initConfigFile();

const globalOwnerRole = await testDb.getGlobalOwnerRole();
globalMemberRole = await testDb.getGlobalMemberRole();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ let authAgent: AuthAgent;
beforeAll(async () => {
app = await utils.initTestServer({ endpointGroups: ['credentials'] });

utils.initConfigFile();
await utils.initConfigFile();

globalOwnerRole = await testDb.getGlobalOwnerRole();
globalMemberRole = await testDb.getGlobalMemberRole();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/eventbus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ beforeAll(async () => {

mockedSyslog.createClient.mockImplementation(() => new syslog.Client());

utils.initConfigFile();
await utils.initConfigFile();
config.set('eventBus.logWriter.logBaseName', 'n8n-test-logwriter');
config.set('eventBus.logWriter.keepLogCount', 1);
config.set('userManagement.disabled', false);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/ldap/ldap.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ beforeAll(async () => {
defaultLdapConfig.bindingAdminPassword,
);

utils.initConfigFile();
await utils.initConfigFile();

await setCurrentAuthenticationMethod('email');
});
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/nodes.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ beforeAll(async () => {
ownerShell = await testDb.createUserShell(globalOwnerRole);
authOwnerShellAgent = utils.createAuthAgent(app)(ownerShell);

utils.initConfigFile();
await utils.initConfigFile();
});

beforeEach(async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/test/integration/publicApi/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ beforeAll(async () => {
enablePublicAPI: true,
});

utils.initConfigFile();
await utils.initConfigFile();

const [globalOwnerRole, fetchedGlobalMemberRole, _, fetchedCredentialOwnerRole] =
await testDb.getAllRoles();
Expand All @@ -51,7 +51,7 @@ beforeAll(async () => {

saveCredential = testDb.affixRoleToSaveCredential(credentialOwnerRole);

utils.initCredentialsTypes();
await utils.initCredentialsTypes();
});

beforeEach(async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/integration/publicApi/executions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ afterAll(async () => {

const testWithAPIKey =
(method: 'get' | 'post' | 'put' | 'delete', url: string, apiKey: string | null) => async () => {
authOwnerAgent.set({ 'X-N8N-API-KEY': apiKey });
void authOwnerAgent.set({ 'X-N8N-API-KEY': apiKey });
const response = await authOwnerAgent[method](url);
expect(response.statusCode).toBe(401);
};
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/test/integration/publicApi/workflows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ beforeAll(async () => {
apiKey: randomApiKey(),
});

utils.initConfigFile();
await utils.initConfigFile();
await utils.initNodeTypes();
workflowRunner = await utils.initActiveWorkflowRunner();
});
Expand Down Expand Up @@ -76,7 +76,7 @@ afterAll(async () => {

const testWithAPIKey =
(method: 'get' | 'post' | 'put' | 'delete', url: string, apiKey: string | null) => async () => {
authOwnerAgent.set({ 'X-N8N-API-KEY': apiKey });
void authOwnerAgent.set({ 'X-N8N-API-KEY': apiKey });
const response = await authOwnerAgent[method](url);
expect(response.statusCode).toBe(401);
};
Expand Down
Loading

0 comments on commit e046f65

Please sign in to comment.