refactor: Assign properties directly to error.

This commit is contained in:
Ruben Verborgh 2021-07-12 17:24:08 +01:00
parent 8a62938c18
commit 649f7a9a78
4 changed files with 33 additions and 11 deletions

View File

@ -14,7 +14,7 @@ import { TypedRepresentationConverter } from './TypedRepresentationConverter';
* Serializes an Error by filling in the provided template. * Serializes an Error by filling in the provided template.
* Content-type is based on the constructor parameter. * Content-type is based on the constructor parameter.
* *
* In case the input Error has an `options.errorCode` value, * In case the input Error has an `errorCode` value,
* the converter will look in the `descriptions` for a file * the converter will look in the `descriptions` for a file
* with the exact same name as that error code + `extension`. * with the exact same name as that error code + `extension`.
* The templating engine will then be applied to that file. * The templating engine will then be applied to that file.
@ -56,8 +56,8 @@ export class ErrorToTemplateConverter extends TypedRepresentationConverter {
} }
private async getErrorCodeMessage(error: Error): Promise<string | undefined> { private async getErrorCodeMessage(error: Error): Promise<string | undefined> {
if (HttpError.isInstance(error) && error.options.errorCode) { if (HttpError.isInstance(error) && error.errorCode) {
const filePath = joinFilePath(this.descriptions, `${error.options.errorCode}${this.extension}`); const filePath = joinFilePath(this.descriptions, `${error.errorCode}${this.extension}`);
let template: string; let template: string;
try { try {
template = await fsPromises.readFile(filePath, 'utf8'); template = await fsPromises.readFile(filePath, 'utf8');
@ -66,7 +66,7 @@ export class ErrorToTemplateConverter extends TypedRepresentationConverter {
return; return;
} }
return this.engine.apply(template, (error.options.details ?? {}) as NodeJS.Dict<string>); return this.engine.apply(template, (error.details ?? {}) as NodeJS.Dict<string>);
} }
} }
} }

View File

@ -10,10 +10,12 @@ export interface HttpErrorOptions {
* A class for all errors that could be thrown by Solid. * A class for all errors that could be thrown by Solid.
* All errors inheriting from this should fix the status code thereby hiding the HTTP internals from other components. * All errors inheriting from this should fix the status code thereby hiding the HTTP internals from other components.
*/ */
export class HttpError extends Error { export class HttpError extends Error implements HttpErrorOptions {
protected static readonly statusCode: number; protected static readonly statusCode: number;
public readonly statusCode: number; public readonly statusCode: number;
public readonly options: HttpErrorOptions; public readonly cause?: unknown;
public readonly errorCode?: string;
public readonly details?: NodeJS.Dict<unknown>;
/** /**
* Creates a new HTTP error. Subclasses should call this with their fixed status code. * Creates a new HTTP error. Subclasses should call this with their fixed status code.
@ -26,7 +28,9 @@ export class HttpError extends Error {
super(message); super(message);
this.statusCode = statusCode; this.statusCode = statusCode;
this.name = name; this.name = name;
this.options = options; this.cause = options.cause;
this.errorCode = options.errorCode;
this.details = options.details;
} }
public static isInstance(error: any): error is HttpError { public static isInstance(error: any): error is HttpError {

View File

@ -1,6 +1,7 @@
import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError';
import { ConflictHttpError } from '../../../../src/util/errors/ConflictHttpError'; import { ConflictHttpError } from '../../../../src/util/errors/ConflictHttpError';
import { ForbiddenHttpError } from '../../../../src/util/errors/ForbiddenHttpError'; import { ForbiddenHttpError } from '../../../../src/util/errors/ForbiddenHttpError';
import type { HttpErrorOptions } from '../../../../src/util/errors/HttpError';
import { HttpError } from '../../../../src/util/errors/HttpError'; import { HttpError } from '../../../../src/util/errors/HttpError';
import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; import { InternalServerError } from '../../../../src/util/errors/InternalServerError';
import { MethodNotAllowedHttpError } from '../../../../src/util/errors/MethodNotAllowedHttpError'; import { MethodNotAllowedHttpError } from '../../../../src/util/errors/MethodNotAllowedHttpError';
@ -11,8 +12,8 @@ import { UnsupportedMediaTypeHttpError } from '../../../../src/util/errors/Unsup
// Only used to make typings easier in the tests // Only used to make typings easier in the tests
class FixedHttpError extends HttpError { class FixedHttpError extends HttpError {
public constructor(message?: string) { public constructor(message?: string, options?: HttpErrorOptions) {
super(0, '', message); super(0, '', message, options);
} }
} }
@ -31,7 +32,12 @@ describe('HttpError', (): void => {
]; ];
describe.each(errors)('%s', (name, statusCode, constructor): void => { describe.each(errors)('%s', (name, statusCode, constructor): void => {
const instance = new constructor('my message'); const options = {
cause: new Error('cause'),
errorCode: 'E1234',
details: {},
};
const instance = new constructor('my message', options);
it(`is an instance of ${name}.`, (): void => { it(`is an instance of ${name}.`, (): void => {
expect(constructor.isInstance(instance)).toBeTruthy(); expect(constructor.isInstance(instance)).toBeTruthy();
@ -48,5 +54,17 @@ describe('HttpError', (): void => {
it('sets the message.', (): void => { it('sets the message.', (): void => {
expect(instance.message).toBe('my message'); expect(instance.message).toBe('my message');
}); });
it('sets the cause.', (): void => {
expect(instance.cause).toBe(options.cause);
});
it('sets the error code.', (): void => {
expect(instance.errorCode).toBe(options.errorCode);
});
it('sets the details.', (): void => {
expect(instance.details).toBe(options.details);
});
}); });
}); });

View File

@ -23,7 +23,7 @@ describe('A BaseIdentifierStrategy', (): void => {
expect((): any => strategy.getParentContainer({ path: '/unsupported' })) expect((): any => strategy.getParentContainer({ path: '/unsupported' }))
.toThrow('The identifier /unsupported is outside the configured identifier space.'); .toThrow('The identifier /unsupported is outside the configured identifier space.');
expect((): any => strategy.getParentContainer({ path: '/unsupported' })) expect((): any => strategy.getParentContainer({ path: '/unsupported' }))
.toThrow(expect.objectContaining({ options: { errorCode: 'E0001', details: { path: '/unsupported' }}})); .toThrow(expect.objectContaining({ errorCode: 'E0001', details: { path: '/unsupported' }}));
}); });
it('errors when attempting to get the parent of a root container.', async(): Promise<void> => { it('errors when attempting to get the parent of a root container.', async(): Promise<void> => {