Skip to content

Commit

Permalink
refactor(core): Reorganize error hierarchy in cli package (no-chang…
Browse files Browse the repository at this point in the history
…elog) (n8n-io#7839)

Ensure all errors in `cli` inherit from `ApplicationError` to continue
normalizing all the errors we report to Sentry

Follow-up to: n8n-io#7820
  • Loading branch information
ivov authored and rishikeshjoshi committed Nov 28, 2023
1 parent 23082f9 commit c73cfd3
Show file tree
Hide file tree
Showing 81 changed files with 346 additions and 297 deletions.
3 changes: 2 additions & 1 deletion packages/cli/src/AbstractServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
import * as Db from '@/Db';
import type { N8nInstanceType, IExternalHooksClass } from '@/Interfaces';
import { ExternalHooks } from '@/ExternalHooks';
import { send, sendErrorResponse, ServiceUnavailableError } from '@/ResponseHelper';
import { send, sendErrorResponse } from '@/ResponseHelper';
import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares';
import { TestWebhooks } from '@/TestWebhooks';
import { WaitingWebhooks } from '@/WaitingWebhooks';
import { webhookRequestHandler } from '@/WebhookHelpers';
import { generateHostInstanceId } from './databases/utils/generators';
import { Logger } from '@/Logger';
import { ServiceUnavailableError } from './errors/response-errors/service-unavailable.error';

export abstract class AbstractServer {
protected logger: Logger;
Expand Down
10 changes: 4 additions & 6 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import type {
WebhookAccessControlOptions,
WebhookRequest,
} from '@/Interfaces';
import * as ResponseHelper from '@/ResponseHelper';
import * as WebhookHelpers from '@/WebhookHelpers';
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';

Expand All @@ -68,6 +67,7 @@ import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.reposi
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee';
import { ActivationErrorsService } from '@/ActivationErrors.service';
import { NotFoundError } from './errors/response-errors/not-found.error';

const WEBHOOK_PROD_UNREGISTERED_HINT =
"The workflow must be active for a production URL to run successfully. You can activate the workflow using the toggle in the top-right of the editor. Note that unlike test URL calls, production URL calls aren't shown on the canvas (only in the executions list)";
Expand Down Expand Up @@ -165,9 +165,7 @@ export class ActiveWorkflowRunner implements IWebhookManager {
});

if (workflowData === null) {
throw new ResponseHelper.NotFoundError(
`Could not find workflow with id "${webhook.workflowId}"`,
);
throw new NotFoundError(`Could not find workflow with id "${webhook.workflowId}"`);
}

const workflow = new Workflow({
Expand Down Expand Up @@ -196,7 +194,7 @@ export class ActiveWorkflowRunner implements IWebhookManager {
const workflowStartNode = workflow.getNode(webhookData.node);

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

return new Promise((resolve, reject) => {
Expand Down Expand Up @@ -252,7 +250,7 @@ export class ActiveWorkflowRunner implements IWebhookManager {

const webhook = await this.webhookService.findWebhook(httpMethod, path);
if (webhook === null) {
throw new ResponseHelper.NotFoundError(
throw new NotFoundError(
webhookNotFoundErrorMessage(path, httpMethod),
WEBHOOK_PROD_UNREGISTERED_HINT,
);
Expand Down
11 changes: 1 addition & 10 deletions packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import type {
INodeTypes,
IWorkflowExecuteAdditionalData,
ICredentialTestFunctions,
Severity,
} from 'n8n-workflow';
import {
ICredentialsHelper,
Expand All @@ -55,6 +54,7 @@ import { isObjectLiteral } from './utils';
import { Logger } from '@/Logger';
import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
import { CredentialNotFoundError } from './errors/credential-not-found.error';

const { OAUTH2_CREDENTIAL_TEST_SUCCEEDED, OAUTH2_CREDENTIAL_TEST_FAILED } = RESPONSE_ERROR_MESSAGES;

Expand Down Expand Up @@ -87,15 +87,6 @@ const mockNodeTypes: INodeTypes = {
},
};

class CredentialNotFoundError extends Error {
severity: Severity;

constructor(credentialId: string, credentialType: string) {
super(`Credential with ID "${credentialId}" does not exist for type "${credentialType}".`);
this.severity = 'warning';
}
}

@Service()
export class CredentialsHelper extends ICredentialsHelper {
constructor(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Authorized, Get, Post, RestController } from '@/decorators';
import { ExternalSecretsRequest } from '@/requests';
import { NotFoundError } from '@/ResponseHelper';
import { Response } from 'express';
import { Service } from 'typedi';
import { ProviderNotFoundError, ExternalSecretsService } from './ExternalSecrets.service.ee';
import { ExternalSecretsService } from './ExternalSecrets.service.ee';
import { ExternalSecretsProviderNotFoundError } from '@/errors/external-secrets-provider-not-found.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';

@Service()
@Authorized(['global', 'owner'])
Expand All @@ -22,7 +23,7 @@ export class ExternalSecretsController {
try {
return this.secretsService.getProvider(providerName);
} catch (e) {
if (e instanceof ProviderNotFoundError) {
if (e instanceof ExternalSecretsProviderNotFoundError) {
throw new NotFoundError(`Could not find provider "${e.providerName}"`);
}
throw e;
Expand All @@ -41,7 +42,7 @@ export class ExternalSecretsController {
}
return result;
} catch (e) {
if (e instanceof ProviderNotFoundError) {
if (e instanceof ExternalSecretsProviderNotFoundError) {
throw new NotFoundError(`Could not find provider "${e.providerName}"`);
}
throw e;
Expand All @@ -54,7 +55,7 @@ export class ExternalSecretsController {
try {
await this.secretsService.saveProviderSettings(providerName, req.body, req.user.id);
} catch (e) {
if (e instanceof ProviderNotFoundError) {
if (e instanceof ExternalSecretsProviderNotFoundError) {
throw new NotFoundError(`Could not find provider "${e.providerName}"`);
}
throw e;
Expand All @@ -68,7 +69,7 @@ export class ExternalSecretsController {
try {
await this.secretsService.saveProviderConnected(providerName, req.body.connected);
} catch (e) {
if (e instanceof ProviderNotFoundError) {
if (e instanceof ExternalSecretsProviderNotFoundError) {
throw new NotFoundError(`Could not find provider "${e.providerName}"`);
}
throw e;
Expand All @@ -88,7 +89,7 @@ export class ExternalSecretsController {
}
return { updated: resp };
} catch (e) {
if (e instanceof ProviderNotFoundError) {
if (e instanceof ExternalSecretsProviderNotFoundError) {
throw new NotFoundError(`Could not find provider "${e.providerName}"`);
}
throw e;
Expand Down
17 changes: 6 additions & 11 deletions packages/cli/src/ExternalSecrets/ExternalSecrets.service.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,15 @@ import type { IDataObject } from 'n8n-workflow';
import { deepCopy } from 'n8n-workflow';
import Container, { Service } from 'typedi';
import { ExternalSecretsManager } from './ExternalSecretsManager.ee';

export class ProviderNotFoundError extends Error {
constructor(public providerName: string) {
super(undefined);
}
}
import { ExternalSecretsProviderNotFoundError } from '@/errors/external-secrets-provider-not-found.error';

@Service()
export class ExternalSecretsService {
getProvider(providerName: string): ExternalSecretsRequest.GetProviderResponse | null {
const providerAndSettings =
Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
if (!providerAndSettings) {
throw new ProviderNotFoundError(providerName);
throw new ExternalSecretsProviderNotFoundError(providerName);
}
const { provider, settings } = providerAndSettings;
return {
Expand Down Expand Up @@ -110,7 +105,7 @@ export class ExternalSecretsService {
const providerAndSettings =
Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
if (!providerAndSettings) {
throw new ProviderNotFoundError(providerName);
throw new ExternalSecretsProviderNotFoundError(providerName);
}
const { settings } = providerAndSettings;
const newData = this.unredact(data, settings.settings);
Expand All @@ -121,7 +116,7 @@ export class ExternalSecretsService {
const providerAndSettings =
Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
if (!providerAndSettings) {
throw new ProviderNotFoundError(providerName);
throw new ExternalSecretsProviderNotFoundError(providerName);
}
await Container.get(ExternalSecretsManager).setProviderConnected(providerName, connected);
return this.getProvider(providerName);
Expand All @@ -135,7 +130,7 @@ export class ExternalSecretsService {
const providerAndSettings =
Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
if (!providerAndSettings) {
throw new ProviderNotFoundError(providerName);
throw new ExternalSecretsProviderNotFoundError(providerName);
}
const { settings } = providerAndSettings;
const newData = this.unredact(data, settings.settings);
Expand All @@ -146,7 +141,7 @@ export class ExternalSecretsService {
const providerAndSettings =
Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
if (!providerAndSettings) {
throw new ProviderNotFoundError(providerName);
throw new ExternalSecretsProviderNotFoundError(providerName);
}
return Container.get(ExternalSecretsManager).updateProvider(providerName);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/GenericHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { Container } from 'typedi';
import { Like } from 'typeorm';
import config from '@/config';
import type { ExecutionPayload, ICredentialsDb, IWorkflowDb } from '@/Interfaces';
import * as ResponseHelper from '@/ResponseHelper';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import type { TagEntity } from '@db/entities/TagEntity';
Expand All @@ -20,6 +19,7 @@ import type { UserUpdatePayload } from '@/requests';
import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { ExecutionRepository } from '@db/repositories/execution.repository';
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { BadRequestError } from './errors/response-errors/bad-request.error';

/**
* Returns the base URL n8n is reachable from
Expand Down Expand Up @@ -109,7 +109,7 @@ export async function validateEntity(
.join(' | ');

if (errorMessages) {
throw new ResponseHelper.BadRequestError(errorMessages);
throw new BadRequestError(errorMessages);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/Ldap/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ import {
isLdapCurrentAuthenticationMethod,
setCurrentAuthenticationMethod,
} from '@/sso/ssoHelpers';
import { BadRequestError, InternalServerError } from '../ResponseHelper';
import { RoleService } from '@/services/role.service';
import { Logger } from '@/Logger';
import { UserRepository } from '@db/repositories/user.repository';
import { SettingsRepository } from '@db/repositories/settings.repository';
import { AuthProviderSyncHistoryRepository } from '@db/repositories/authProviderSyncHistory.repository';
import { AuthIdentityRepository } from '@db/repositories/authIdentity.repository';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';

/**
* Check whether the LDAP feature is disabled in the instance
Expand Down
8 changes: 0 additions & 8 deletions packages/cli/src/License.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@ type FeatureReturnType = Partial<
} & { [K in NumericLicenseFeature]: number } & { [K in BooleanLicenseFeature]: boolean }
>;

export class FeatureNotLicensedError extends Error {
constructor(feature: (typeof LICENSE_FEATURES)[keyof typeof LICENSE_FEATURES]) {
super(
`Your license does not allow for ${feature}. To enable ${feature}, please upgrade to a license that supports this feature.`,
);
}
}

@Service()
export class License {
private manager: LicenseManager | undefined;
Expand Down
11 changes: 2 additions & 9 deletions packages/cli/src/NodeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,7 @@ import { LoadNodesAndCredentials } from './LoadNodesAndCredentials';
import { join, dirname } from 'path';
import { readdir } from 'fs/promises';
import type { Dirent } from 'fs';

class UnrecognizedNodeError extends Error {
severity = 'warning';

constructor(nodeType: string) {
super(`Unrecognized node type: ${nodeType}".`);
}
}
import { UnrecognizedNodeTypeError } from './errors/unrecognized-node-type.error';

@Service()
export class NodeTypes implements INodeTypes {
Expand Down Expand Up @@ -75,7 +68,7 @@ export class NodeTypes implements INodeTypes {
return loadedNodes[type];
}

throw new UnrecognizedNodeError(type);
throw new UnrecognizedNodeTypeError(type);
}

async getNodeTranslationPath({
Expand Down
71 changes: 1 addition & 70 deletions packages/cli/src/ResponseHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,76 +12,7 @@ import type {
IWorkflowDb,
} from '@/Interfaces';
import { inDevelopment } from '@/constants';

/**
* Special Error which allows to return also an error code and http status code
*/
abstract class ResponseError extends Error {
/**
* Creates an instance of ResponseError.
* Must be used inside a block with `ResponseHelper.send()`.
*/
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';
}
}

export class BadRequestError extends ResponseError {
constructor(message: string, errorCode?: number) {
super(message, 400, errorCode);
}
}

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

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 UnprocessableRequestError extends ResponseError {
constructor(message: string, hint: string | undefined = undefined) {
super(message, 422, 422, hint);
}
}

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

export class ServiceUnavailableError extends ResponseError {
constructor(message: string, errorCode = 503) {
super(message, 503, errorCode);
}
}
import { ResponseError } from './errors/response-errors/abstract/response.error';

export function sendSuccessResponse(
res: Response,
Expand Down
Loading

0 comments on commit c73cfd3

Please sign in to comment.