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

feat(API): Report unhandled app crashes to Sentry #4548

Merged
merged 4 commits into from
Nov 8, 2022
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
11 changes: 4 additions & 7 deletions packages/cli/commands/executeBatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import { Command, flags } from '@oclif/command';

import { BinaryDataManager, UserSettings } from 'n8n-core';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { INode, ITaskData, LoggerProxy } from 'n8n-workflow';
import { ITaskData, LoggerProxy, sleep } from 'n8n-workflow';

import { sep } from 'path';

Expand Down Expand Up @@ -147,9 +146,7 @@ export class ExecuteBatch extends Command {
});
}
// eslint-disable-next-line no-await-in-loop
await new Promise((resolve) => {
setTimeout(resolve, 500);
});
await sleep(500);
executingWorkflows = activeExecutionsInstance.getActiveExecutions();
}
// We may receive true but when called from `process.on`
Expand Down Expand Up @@ -192,8 +189,8 @@ export class ExecuteBatch extends Command {

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
async run() {
process.on('SIGTERM', ExecuteBatch.stopProcess);
process.on('SIGINT', ExecuteBatch.stopProcess);
process.once('SIGTERM', ExecuteBatch.stopProcess);
process.once('SIGINT', ExecuteBatch.stopProcess);

const logger = getLogger();
LoggerProxy.init(logger);
Expand Down
35 changes: 22 additions & 13 deletions packages/cli/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Command, flags } from '@oclif/command';
// eslint-disable-next-line import/no-extraneous-dependencies
import Redis from 'ioredis';

import { IDataObject, LoggerProxy } from 'n8n-workflow';
import { IDataObject, LoggerProxy, sleep } from 'n8n-workflow';
import { createHash } from 'crypto';
import config from '../config';
import {
Expand All @@ -34,6 +34,8 @@ import {

import { getLogger } from '../src/Logger';
import { getAllInstalledPackages } from '../src/CommunityNodes/packageModel';
import { initErrorHandling } from '../src/ErrorReporting';
import * as CrashJournal from '../src/CrashJournal';

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-var-requires
const open = require('open');
Expand Down Expand Up @@ -83,14 +85,20 @@ export class Start extends Command {
}

/**
* Stoppes the n8n in a graceful way.
* Stop n8n in a graceful way.
* Make for example sure that all the webhooks from third party services
* get removed.
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
static async stopProcess() {
getLogger().info('\nStopping n8n...');

const exit = () => {
CrashJournal.cleanup().finally(() => {
process.exit(processExitCode);
});
};

try {
// Stop with trying to activate workflows that could not be activated
activeWorkflowRunner?.removeAllQueuedWorkflowActivations();
Expand All @@ -102,7 +110,7 @@ export class Start extends Command {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
console.log(`process exited after 30s`);
process.exit(processExitCode);
exit();
}, 30000);

await InternalHooksManager.getInstance().onN8nStop();
Expand Down Expand Up @@ -136,33 +144,34 @@ export class Start extends Command {
});
}
// eslint-disable-next-line no-await-in-loop
await new Promise((resolve) => {
setTimeout(resolve, 500);
});
await sleep(500);
executingWorkflows = activeExecutionsInstance.getActiveExecutions();
}
} catch (error) {
console.error('There was an error shutting down n8n.', error);
}

process.exit(processExitCode);
exit();
}

async run() {
// Make sure that n8n shuts down gracefully if possible
process.on('SIGTERM', Start.stopProcess);
process.on('SIGINT', Start.stopProcess);
process.once('SIGTERM', Start.stopProcess);
process.once('SIGINT', Start.stopProcess);

const logger = getLogger();
LoggerProxy.init(logger);
logger.info('Initializing n8n process');

initErrorHandling();
await CrashJournal.init();

// eslint-disable-next-line @typescript-eslint/no-shadow
const { flags } = this.parse(Start);

// Wrap that the process does not close but we can still use async
await (async () => {
try {
const logger = getLogger();
LoggerProxy.init(logger);
logger.info('Initializing n8n process');

// Start directly with the init of the database to improve startup time
const startDbInitPromise = Db.init().catch((error: Error) => {
logger.error(`There was an error initializing DB: "${error.message}"`);
Expand Down
33 changes: 21 additions & 12 deletions packages/cli/commands/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Command, flags } from '@oclif/command';
// eslint-disable-next-line import/no-extraneous-dependencies
import Redis from 'ioredis';

import { IDataObject, LoggerProxy } from 'n8n-workflow';
import { IDataObject, LoggerProxy, sleep } from 'n8n-workflow';
import config from '../config';
import {
ActiveExecutions,
Expand All @@ -26,9 +26,11 @@ import {
} from '../src';

import { getLogger } from '../src/Logger';
import { initErrorHandling } from '../src/ErrorReporting';
import * as CrashJournal from '../src/CrashJournal';

let activeWorkflowRunner: ActiveWorkflowRunner.ActiveWorkflowRunner | undefined;
let processExistCode = 0;
let processExitCode = 0;

export class Webhook extends Command {
static description = 'Starts n8n webhook process. Intercepts only production URLs.';
Expand All @@ -40,22 +42,28 @@ export class Webhook extends Command {
};

/**
* Stops the n8n in a graceful way.
* Stops n8n in a graceful way.
* Make for example sure that all the webhooks from third party services
* get removed.
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
static async stopProcess() {
LoggerProxy.info(`\nStopping n8n...`);

const exit = () => {
CrashJournal.cleanup().finally(() => {
process.exit(processExitCode);
});
};

try {
const externalHooks = ExternalHooks();
await externalHooks.run('n8n.stop', []);

setTimeout(() => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
process.exit(processExistCode);
exit();
}, 30000);

// Wait for active workflow executions to finish
Expand All @@ -70,16 +78,14 @@ export class Webhook extends Command {
);
}
// eslint-disable-next-line no-await-in-loop
await new Promise((resolve) => {
setTimeout(resolve, 500);
});
await sleep(500);
executingWorkflows = activeExecutionsInstance.getActiveExecutions();
}
} catch (error) {
LoggerProxy.error('There was an error shutting down n8n.', error);
}

process.exit(processExistCode);
exit();
}

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
Expand All @@ -88,8 +94,11 @@ export class Webhook extends Command {
LoggerProxy.init(logger);

// Make sure that n8n shuts down gracefully if possible
process.on('SIGTERM', Webhook.stopProcess);
process.on('SIGINT', Webhook.stopProcess);
process.once('SIGTERM', Webhook.stopProcess);
process.once('SIGINT', Webhook.stopProcess);

initErrorHandling();
await CrashJournal.init();

// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-shadow
const { flags } = this.parse(Webhook);
Expand Down Expand Up @@ -118,7 +127,7 @@ export class Webhook extends Command {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions, @typescript-eslint/no-unsafe-member-access
logger.error(`There was an error initializing DB: "${error.message}"`);

processExistCode = 1;
processExitCode = 1;
// @ts-ignore
process.emit('SIGINT');
process.exit(1);
Expand Down Expand Up @@ -230,7 +239,7 @@ export class Webhook extends Command {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
logger.error(`Webhook process cannot continue. "${error.message}"`);

processExistCode = 1;
processExitCode = 1;
// @ts-ignore
process.emit('SIGINT');
process.exit(1);
Expand Down
29 changes: 17 additions & 12 deletions packages/cli/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ import PCancelable from 'p-cancelable';
import { Command, flags } from '@oclif/command';
import { BinaryDataManager, UserSettings, WorkflowExecute } from 'n8n-core';

import { IExecuteResponsePromiseData, INodeTypes, IRun, Workflow, LoggerProxy } from 'n8n-workflow';
import {
IExecuteResponsePromiseData,
INodeTypes,
IRun,
Workflow,
LoggerProxy,
sleep,
} from 'n8n-workflow';

import { FindOneOptions, getConnectionManager } from 'typeorm';

Expand Down Expand Up @@ -62,11 +69,11 @@ export class Worker extends Command {

static jobQueue: Queue.JobQueue;

static processExistCode = 0;
static processExitCode = 0;
// static activeExecutions = ActiveExecutions.getInstance();

/**
* Stoppes the n8n in a graceful way.
* Stop n8n in a graceful way.
* Make for example sure that all the webhooks from third party services
* get removed.
*/
Expand All @@ -88,7 +95,7 @@ export class Worker extends Command {
setTimeout(() => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
process.exit(Worker.processExistCode);
process.exit(Worker.processExitCode);
}, maxStopTime);

// Wait for active workflow executions to finish
Expand All @@ -103,15 +110,13 @@ export class Worker extends Command {
);
}
// eslint-disable-next-line no-await-in-loop
await new Promise((resolve) => {
setTimeout(resolve, 500);
});
await sleep(500);
}
} catch (error) {
LoggerProxy.error('There was an error shutting down n8n.', error);
}

process.exit(Worker.processExistCode);
process.exit(Worker.processExitCode);
}

async runJob(job: Queue.Job, nodeTypes: INodeTypes): Promise<Queue.JobResponse> {
Expand Down Expand Up @@ -258,8 +263,8 @@ export class Worker extends Command {
console.info('Starting n8n worker...');

// Make sure that n8n shuts down gracefully if possible
process.on('SIGTERM', Worker.stopProcess);
process.on('SIGINT', Worker.stopProcess);
process.once('SIGTERM', Worker.stopProcess);
process.once('SIGINT', Worker.stopProcess);

// Wrap that the process does not close but we can still use async
await (async () => {
Expand All @@ -270,7 +275,7 @@ export class Worker extends Command {
const startDbInitPromise = Db.init().catch((error) => {
logger.error(`There was an error initializing DB: "${error.message}"`);

Worker.processExistCode = 1;
Worker.processExitCode = 1;
// @ts-ignore
process.emit('SIGINT');
process.exit(1);
Expand Down Expand Up @@ -441,7 +446,7 @@ export class Worker extends Command {
} catch (error) {
logger.error(`Worker process cannot continue. "${error.message}"`);

Worker.processExistCode = 1;
Worker.processExitCode = 1;
// @ts-ignore
process.emit('SIGINT');
process.exit(1);
Expand Down
33 changes: 33 additions & 0 deletions packages/cli/src/CrashJournal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { existsSync } from 'fs';
import { mkdir, utimes, open, rm } from 'fs/promises';
import { join, dirname } from 'path';
import { UserSettings } from 'n8n-core';
import { ErrorReporterProxy as ErrorReporter, LoggerProxy, sleep } from 'n8n-workflow';

export const touchFile = async (filePath: string): Promise<void> => {
await mkdir(dirname(filePath), { recursive: true });
const time = new Date();
try {
await utimes(filePath, time, time);
} catch {
const fd = await open(filePath, 'w');
await fd.close();
}
};

const journalFile = join(UserSettings.getUserN8nFolderPath(), 'crash.journal');

export const init = async () => {
if (existsSync(journalFile)) {
// Crash detected
ErrorReporter.warn('Last session crashed');
LoggerProxy.error('Last session crashed');
// add a 10 seconds pause to slow down crash-looping
await sleep(10_000);
}
await touchFile(journalFile);
};

export const cleanup = async () => {
await rm(journalFile, { force: true });
};
14 changes: 7 additions & 7 deletions packages/cli/src/ErrorReporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ErrorReporterProxy } from 'n8n-workflow';

let initialized = false;

export const initErrorHandling = (app?: Application) => {
export const initErrorHandling = () => {
if (initialized) return;

if (!config.getEnv('diagnostics.enabled')) {
Expand All @@ -27,15 +27,15 @@ export const initErrorHandling = (app?: Application) => {
},
});

if (app) {
const { requestHandler, errorHandler } = Sentry.Handlers;
app.use(requestHandler());
app.use(errorHandler());
}

ErrorReporterProxy.init({
report: (error, options) => Sentry.captureException(error, options),
});

initialized = true;
};

export const setupErrorMiddleware = (app: Application) => {
const { requestHandler, errorHandler } = Sentry.Handlers;
app.use(requestHandler());
app.use(errorHandler());
};
4 changes: 2 additions & 2 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ import glob from 'fast-glob';
import { ResponseError } from './ResponseHelper';

import { toHttpNodeParameters } from './CurlConverterHelper';
import { initErrorHandling } from './ErrorReporting';
import { setupErrorMiddleware } from './ErrorReporting';

require('body-parser-xml')(bodyParser);

Expand Down Expand Up @@ -259,7 +259,7 @@ class App {
this.presetCredentialsLoaded = false;
this.endpointPresetCredentials = config.getEnv('credentials.overwrite.endpoint');

initErrorHandling(this.app);
setupErrorMiddleware(this.app);

const urlBaseWebhook = WebhookHelpers.getWebhookBaseUrl();
const telemetrySettings: ITelemetrySettings = {
Expand Down
Loading