1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-31 00:16:47 +01:00

refactor: move status codes into classes (#4200)

Make each error class have to define its own status code. This makes
it easier for developers to see which code an error corresponds to and
means less jumping back and forth between files. In other words:
improved locality.

Unfortunately, the long switch needs to stay in case we get errors
thrown that aren't of the Unleash Error type, but we can move it to
the `fromLegacyError` file instead.

Tradeoff analysis by @kwasniew: 
+ I like the locality of error to code reasoning
- now HTTP leaks to the non-HTTP code that throws those errors e.g. application services

If we had other delivery mechanisms other than HTTP then it wouldn't make sense to couple error codes to one protocol (HTTP). But since we're mostly doing web it may not be a problem.

@thomasheartman's response:

This is a good point and something I hadn't considered. The same data was always available on those errors (by using the same property), I've just made the declaration local to each error instead of something that the parent class handles. The idea was to make it easier to create new error classes with their corresponding error codes. Because the errors are intended to be API errors (or at least, I've always considered them to be that), I think that makes sense.

Taking your comment into consideration, I still think it's the right thing to do, but I'm not bullish about it. We could always walk it back later if we find that it's not appropriate. The old code is still available and we could easily enough roll back this change if we find that we want to decouple it later.
This commit is contained in:
Thomas Heartman 2023-07-11 09:20:11 +02:00 committed by GitHub
parent a2b06e4222
commit f83350cb2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 130 additions and 76 deletions

View File

@ -9,6 +9,8 @@ type ValidationErrorDescription = {
path?: string;
};
class BadDataError extends UnleashError {
statusCode = 400;
details: ValidationErrorDescription[];
constructor(

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class ContentTypeError extends UnleashError {
statusCode = 415;
constructor(
acceptedContentTypes: [string, ...string[]],
providedContentType?: string,

View File

@ -1,5 +1,7 @@
import { UnleashError } from './unleash-error';
class DisabledError extends UnleashError {}
class DisabledError extends UnleashError {
statusCode = 422;
}
export default DisabledError;

View File

@ -1,5 +1,7 @@
import { UnleashError } from './unleash-error';
class FeatureHasTagError extends UnleashError {}
class FeatureHasTagError extends UnleashError {
statusCode = 409;
}
export default FeatureHasTagError;
module.exports = FeatureHasTagError;

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class ForbiddenError extends UnleashError {}
class ForbiddenError extends UnleashError {
statusCode = 403;
}
export default ForbiddenError;
module.exports = ForbiddenError;

View File

@ -7,6 +7,69 @@ import {
UnleashError,
} from './unleash-error';
const getStatusCode = (errorName: string): number => {
switch (errorName) {
case 'ContentTypeError':
return 415;
case 'ValidationError':
return 400;
case 'BadDataError':
return 400;
case 'OwaspValidationError':
return 400;
case 'PasswordUndefinedError':
return 400;
case 'MinimumOneEnvironmentError':
return 400;
case 'InvalidTokenError':
return 401;
case 'NoAccessError':
return 403;
case 'UsedTokenError':
return 403;
case 'InvalidOperationError':
return 403;
case 'IncompatibleProjectError':
return 403;
case 'OperationDeniedError':
return 403;
case 'NotFoundError':
return 404;
case 'NameExistsError':
return 409;
case 'FeatureHasTagError':
return 409;
case 'RoleInUseError':
return 400;
case 'ProjectWithoutOwnerError':
return 409;
case 'UnknownError':
return 500;
case 'InternalError':
return 500;
case 'PasswordMismatch':
return 401;
case 'UnauthorizedError':
return 401;
case 'DisabledError':
return 422;
case 'NotImplementedError':
return 405;
case 'NoAccessError':
return 403;
case 'AuthenticationRequired':
return 401;
case 'ForbiddenError':
return 403;
case 'PermissionError':
return 403;
case 'BadRequestError': //thrown by express; do not remove
return 400;
default:
return 500;
}
};
export const fromLegacyError = (e: Error): UnleashError => {
if (e instanceof UnleashError) {
return e;
@ -15,10 +78,13 @@ export const fromLegacyError = (e: Error): UnleashError => {
? (e.name as UnleashApiErrorName)
: 'UnknownError';
const statusCode = getStatusCode(name);
if (name === 'NoAccessError') {
return new GenericUnleashError({
name: 'NoAccessError',
message: e.message,
statusCode,
});
}
@ -30,6 +96,7 @@ export const fromLegacyError = (e: Error): UnleashError => {
return new GenericUnleashError({
name: 'BadDataError',
message: e.message,
statusCode,
});
}
@ -37,6 +104,7 @@ export const fromLegacyError = (e: Error): UnleashError => {
return new GenericUnleashError({
name: 'OwaspValidationError',
message: e.message,
statusCode,
});
}
@ -44,11 +112,13 @@ export const fromLegacyError = (e: Error): UnleashError => {
return new GenericUnleashError({
name: 'AuthenticationRequired',
message: `You must be authenticated to view this content. Please log in.`,
statusCode,
});
}
return new GenericUnleashError({
name: name as UnleashApiErrorName,
message: e.message,
statusCode,
});
};

View File

@ -1,6 +1,8 @@
import { ApiErrorSchema, UnleashError } from './unleash-error';
export default class IncompatibleProjectError extends UnleashError {
statusCode = 403;
constructor(targetProject: string) {
super(`${targetProject} is not a compatible target`);
}

View File

@ -1,5 +1,7 @@
import { UnleashError } from './unleash-error';
class InvalidOperationError extends UnleashError {}
class InvalidOperationError extends UnleashError {
statusCode = 403;
}
export default InvalidOperationError;
module.exports = InvalidOperationError;

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class InvalidTokenError extends UnleashError {
statusCode = 401;
constructor() {
super('Token was not valid');
}

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class MinimumOneEnvironmentError extends UnleashError {}
class MinimumOneEnvironmentError extends UnleashError {
statusCode = 400;
}
export default MinimumOneEnvironmentError;
module.exports = MinimumOneEnvironmentError;

View File

@ -1,5 +1,7 @@
import { UnleashError } from './unleash-error';
class NameExistsError extends UnleashError {}
class NameExistsError extends UnleashError {
statusCode = 409;
}
export default NameExistsError;
module.exports = NameExistsError;

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class NotImplementedError extends UnleashError {}
class NotImplementedError extends UnleashError {
statusCode = 405;
}
export default NotImplementedError;
module.exports = NotImplementedError;

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class NotFoundError extends UnleashError {
statusCode = 404;
constructor(message: string = 'The requested resource could not be found') {
super(message);
}

View File

@ -1,3 +1,5 @@
import { UnleashError } from './unleash-error';
export class OperationDeniedError extends UnleashError {}
export class OperationDeniedError extends UnleashError {
statusCode = 403;
}

View File

@ -7,6 +7,8 @@ type ValidationError = {
};
class OwaspValidationError extends UnleashError {
statusCode = 400;
private details: [ValidationError];
constructor(testResult: TestResult) {

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class PasswordMismatch extends UnleashError {
statusCode = 401;
constructor(message: string = 'Wrong password, try again.') {
super(message);
}

View File

@ -1,6 +1,8 @@
import { ApiErrorSchema, UnleashError } from './unleash-error';
export default class PasswordUndefinedError extends UnleashError {
statusCode = 400;
constructor() {
super('Password cannot be empty or undefined');
}

View File

@ -3,6 +3,8 @@ import { ApiErrorSchema, UnleashError } from './unleash-error';
type Permission = string | string[];
class PermissionError extends UnleashError {
statusCode = 403;
permissions: Permission;
constructor(permission: Permission = [], environment?: string) {

View File

@ -1,6 +1,8 @@
import { ApiErrorSchema, UnleashError } from './unleash-error';
export default class ProjectWithoutOwnerError extends UnleashError {
statusCode = 409;
constructor() {
super('A project must have at least one owner');
}

View File

@ -1,5 +1,7 @@
import { UnleashError } from './unleash-error';
class RoleInUseError extends UnleashError {}
class RoleInUseError extends UnleashError {
statusCode = 400;
}
export default RoleInUseError;

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class UnauthorizedError extends UnleashError {}
class UnauthorizedError extends UnleashError {
statusCode = 401;
}
export default UnauthorizedError;
module.exports = UnauthorizedError;

View File

@ -35,75 +35,12 @@ export const UnleashApiErrorTypes = [
export type UnleashApiErrorName = typeof UnleashApiErrorTypes[number];
const statusCode = (errorName: string): number => {
switch (errorName) {
case 'ContentTypeError':
return 415;
case 'ValidationError':
return 400;
case 'BadDataError':
return 400;
case 'OwaspValidationError':
return 400;
case 'PasswordUndefinedError':
return 400;
case 'MinimumOneEnvironmentError':
return 400;
case 'InvalidTokenError':
return 401;
case 'NoAccessError':
return 403;
case 'UsedTokenError':
return 403;
case 'InvalidOperationError':
return 403;
case 'IncompatibleProjectError':
return 403;
case 'OperationDeniedError':
return 403;
case 'NotFoundError':
return 404;
case 'NameExistsError':
return 409;
case 'FeatureHasTagError':
return 409;
case 'RoleInUseError':
return 400;
case 'ProjectWithoutOwnerError':
return 409;
case 'UnknownError':
return 500;
case 'InternalError':
return 500;
case 'PasswordMismatch':
return 401;
case 'UnauthorizedError':
return 401;
case 'DisabledError':
return 422;
case 'NotImplementedError':
return 405;
case 'NoAccessError':
return 403;
case 'AuthenticationRequired':
return 401;
case 'ForbiddenError':
return 403;
case 'PermissionError':
return 403;
case 'BadRequestError': //thrown by express; do not remove
return 400;
default:
return 500;
}
};
export abstract class UnleashError extends Error {
id: string;
name: string;
statusCode: number;
abstract statusCode: number;
additionalParameters: object;
@ -112,8 +49,6 @@ export abstract class UnleashError extends Error {
this.id = uuidV4();
this.name = name || this.constructor.name;
super.message = message;
this.statusCode = statusCode(this.name);
}
help(): string {
@ -135,14 +70,19 @@ export abstract class UnleashError extends Error {
}
export class GenericUnleashError extends UnleashError {
statusCode: number;
constructor({
name,
message,
statusCode,
}: {
name: UnleashApiErrorName;
message: string;
statusCode: number;
}) {
super(message, name);
this.statusCode = statusCode;
}
}

View File

@ -1,6 +1,8 @@
import { UnleashError } from './unleash-error';
class UsedTokenError extends UnleashError {
statusCode = 403;
constructor(usedAt: Date) {
super(`Token was already used at ${usedAt}`);
}

View File

@ -12,6 +12,8 @@ interface IOptions extends IBaseOptions {
}
class AuthenticationRequired extends UnleashError {
statusCode = 401;
private type: string;
private path: string;