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(core): Reorganize error hierarchy in cli package (no-changelog) #7839

Merged
merged 1 commit into from
Nov 28, 2023
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
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
Loading