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: Improve error logging/reporting for cli #4691

Merged
merged 2 commits into from
Nov 22, 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
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
"passport": "^0.6.0",
"passport-cookie": "^1.0.9",
"passport-jwt": "^4.0.0",
"picocolors": "^1.0.0",
"pg": "^8.3.0",
"posthog-node": "^1.3.0",
"prom-client": "^13.1.0",
Expand Down
18 changes: 5 additions & 13 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,8 @@ export class ActiveWorkflowRunner {
): Promise<IResponseCallbackData> {
Logger.debug(`Received webhook "${httpMethod}" for path "${path}"`);
if (this.activeWorkflows === null) {
throw new ResponseHelper.ResponseError(
throw new ResponseHelper.NotFoundError(
'The "activeWorkflows" instance did not get initialized yet.',
404,
404,
);
}

Expand Down Expand Up @@ -224,10 +222,8 @@ export class ActiveWorkflowRunner {
});
if (dynamicWebhooks === undefined || dynamicWebhooks.length === 0) {
// The requested webhook is not registered
throw new ResponseHelper.ResponseError(
throw new ResponseHelper.NotFoundError(
`The requested webhook "${httpMethod} ${path}" is not registered.`,
404,
404,
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}
Expand All @@ -252,10 +248,8 @@ export class ActiveWorkflowRunner {
}
});
if (webhook === undefined) {
throw new ResponseHelper.ResponseError(
throw new ResponseHelper.NotFoundError(
`The requested webhook "${httpMethod} ${path}" is not registered.`,
404,
404,
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}
Expand All @@ -277,10 +271,8 @@ export class ActiveWorkflowRunner {
relations: ['shared', 'shared.user', 'shared.user.globalRole'],
});
if (workflowData === undefined) {
throw new ResponseHelper.ResponseError(
throw new ResponseHelper.NotFoundError(
`Could not find workflow with id "${webhook.workflowId}"`,
404,
404,
);
}

Expand Down Expand Up @@ -313,7 +305,7 @@ export class ActiveWorkflowRunner {
const workflowStartNode = workflow.getNode(webhookData.node);

if (workflowStartNode === null) {
throw new ResponseHelper.ResponseError('Could not find node to process webhook.', 404, 404);
throw new ResponseHelper.NotFoundError('Could not find node to process webhook.');
}

return new Promise((resolve, reject) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/GenericHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export async function validateEntity(
.join(' | ');

if (errorMessages) {
throw new ResponseHelper.ResponseError(errorMessages, undefined, 400);
throw new ResponseHelper.BadRequestError(errorMessages);
}
}

Expand Down
143 changes: 90 additions & 53 deletions packages/cli/src/ResponseHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
import { Request, Response } from 'express';
import { parse, stringify } from 'flatted';
import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
import picocolors from 'picocolors';
import { ErrorReporterProxy as ErrorReporter, NodeApiError } from 'n8n-workflow';

import type {
IExecutionDb,
Expand All @@ -15,44 +16,69 @@ import type {
IWorkflowDb,
} from './Interfaces';

const inDevelopment = !process.env.NODE_ENV || process.env.NODE_ENV === 'development';

/**
* Special Error which allows to return also an error code and http status code
*
* @class ResponseError
* @extends {Error}
*/
export class ResponseError extends Error {
// The HTTP status code of response
httpStatusCode?: number;

// The error code in the response
errorCode?: number;

// The error hint the response
hint?: string;

abstract class ResponseError extends Error {
/**
* Creates an instance of ResponseError.
* Must be used inside a block with `ResponseHelper.send()`.
*
* @param {string} message The error message
* @param {number} [errorCode] The error code which can be used by frontend to identify the actual error
* @param {number} [httpStatusCode] The HTTP status code the response should have
* @param {string} [hint] The error hint to provide a context (webhook related)
*/
constructor(message: string, errorCode?: number, httpStatusCode?: number, hint?: string) {
constructor(
message: string,
// The HTTP status code of response
readonly httpStatusCode: number,
// The error code in the response
readonly errorCode: number = httpStatusCode,
// The error hint the response
readonly hint: string | undefined = undefined,
) {
super(message);
this.name = 'ResponseError';
}
}

if (errorCode) {
this.errorCode = errorCode;
}
if (httpStatusCode) {
this.httpStatusCode = httpStatusCode;
}
if (hint) {
this.hint = hint;
}
export class BadRequestError extends ResponseError {
constructor(message: string) {
super(message, 400);
}
}

export class AuthError extends ResponseError {
constructor(message: string) {
super(message, 401);
}
}

export class UnauthorizedError extends ResponseError {
constructor(message: string, hint: string | undefined = undefined) {
super(message, 403, 403, hint);
}
}

export class NotFoundError extends ResponseError {
constructor(message: string, hint: string | undefined = undefined) {
super(message, 404, 404, hint);
}
}

export class ConflictError extends ResponseError {
constructor(message: string, hint: string | undefined = undefined) {
super(message, 409, 409, hint);
}
}

export class InternalServerError extends ResponseError {
constructor(message: string, errorCode = 500) {
super(message, 500, errorCode);
netroy marked this conversation as resolved.
Show resolved Hide resolved
}
}

export class ServiceUnavailableError extends ResponseError {
constructor(message: string, errorCode = 503) {
super(message, 503, errorCode);
}
}

Expand Down Expand Up @@ -95,40 +121,49 @@ export function sendSuccessResponse(
}
}

export function sendErrorResponse(res: Response, error: ResponseError) {
let httpStatusCode = 500;
if (error.httpStatusCode) {
httpStatusCode = error.httpStatusCode;
}
interface ErrorResponse {
code: number;
message: string;
hint?: string;
stacktrace?: string;
}

if (!process.env.NODE_ENV || process.env.NODE_ENV === 'development') {
console.error('ERROR RESPONSE');
console.error(error);
}
export function sendErrorResponse(res: Response, error: Error) {
let httpStatusCode = 500;

const response = {
const response: ErrorResponse = {
code: 0,
message: 'Unknown error',
hint: '',
};

if (error.name === 'NodeApiError') {
Object.assign(response, error);
}
if (error instanceof ResponseError) {
if (inDevelopment) {
console.error(picocolors.red(error.httpStatusCode), error.message);
}

if (error.errorCode) {
response.code = error.errorCode;
}
if (error.message) {
response.message = error.message;
httpStatusCode = error.httpStatusCode;

if (error.errorCode) {
response.code = error.errorCode;
}
if (error.hint) {
response.hint = error.hint;
}
}
if (error.hint) {
response.hint = error.hint;

if (error instanceof NodeApiError) {
if (inDevelopment) {
console.error(picocolors.red(error.name), error.message);
}

Object.assign(response, error);
}
if (error.stack && process.env.NODE_ENV !== 'production') {
// @ts-ignore
response.stack = error.stack;

if (error.stack && inDevelopment) {
response.stacktrace = error.stack;
}

res.status(httpStatusCode).json(response);
}

Expand All @@ -154,7 +189,9 @@ export function send<T, R extends Request, S extends Response>(
sendSuccessResponse(res, data, raw);
} catch (error) {
if (error instanceof Error) {
ErrorReporter.error(error);
if (!(error instanceof ResponseError) || error.httpStatusCode > 404) {
ErrorReporter.error(error);
}

if (isUniqueConstraintError(error)) {
error.message = 'There is already an entry with this name';
Expand Down
Loading