Skip to content

Commit

Permalink
Return details arrays on all errors. (#3630)
Browse files Browse the repository at this point in the history
We used to use the `details` property to return a list of errors on a
lot of our errors, but the new format doesn't do this anymore. However,
some of the admin UI still expects this to be present, even when the
data could go into `message`. So for now, the solution is to duplicate
the data and put it both in `message` and in the first element of the
`details` list. If the error has its own details lust (such as openapi
errors etc), then they will overwrite this default `details` property.
  • Loading branch information
thomasheartman committed Apr 26, 2023
1 parent 33dce40 commit a7213bf
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 31 deletions.
38 changes: 38 additions & 0 deletions src/lib/error/api-error.test.ts
@@ -1,9 +1,47 @@
import { ErrorObject } from 'ajv';
import {
ApiErrorSchema,
fromLegacyError,
fromOpenApiValidationError,
fromOpenApiValidationErrors,
UnleashApiErrorNameWithoutExtraData,
UnleashApiErrorTypes,
UnleashError,
} from './api-error';
import BadDataError from './bad-data-error';

describe('v5 deprecation: backwards compatibility', () => {
it.each(UnleashApiErrorTypes)(
'Adds details to error type: "%s"',
(name: UnleashApiErrorNameWithoutExtraData) => {
const message = `Error type: ${name}`;
const error = new UnleashError({ name, message }).toJSON();

expect(error.message).toBe(message);
expect(error.details).toStrictEqual([
{
message,
description: message,
},
]);
},
);
});

describe('Standard/legacy error conversion', () => {
it('Moves message to the details list for baddataerror', () => {
const message = `: message!`;
const result = fromLegacyError(new BadDataError(message)).toJSON();

expect(result.message.includes('`details`'));
expect(result.details).toStrictEqual([
{
message,
description: message,
},
]);
});
});

describe('OpenAPI error conversion', () => {
it('Gives useful error messages for missing properties', () => {
Expand Down
49 changes: 23 additions & 26 deletions src/lib/error/api-error.ts
Expand Up @@ -2,32 +2,32 @@ import { v4 as uuidV4 } from 'uuid';
import { FromSchema } from 'json-schema-to-ts';
import { ErrorObject } from 'ajv';

const UnleashApiErrorTypes = [
'OwaspValidationError',
'PasswordUndefinedError',
'NoAccessError',
'UsedTokenError',
'InvalidOperationError',
export const UnleashApiErrorTypes = [
'ContentTypeError',
'DisabledError',
'FeatureHasTagError',
'IncompatibleProjectError',
'OperationDeniedError',
'NotFoundError',
'InvalidOperationError',
'MinimumOneEnvironmentError',
'NameExistsError',
'FeatureHasTagError',
'RoleInUseError',
'ProjectWithoutOwnerError',
'UnknownError',
'NoAccessError',
'NotFoundError',
'NotImplementedError',
'OperationDeniedError',
'OwaspValidationError',
'PasswordMismatch',
'PasswordMismatchError',
'DisabledError',
'ContentTypeError',
'NotImplementedError',
'PasswordUndefinedError',
'ProjectWithoutOwnerError',
'RoleInUseError',
'UnknownError',
'UsedTokenError',

// server errors; not the end user's fault
'InternalError',
] as const;

const UnleashApiErrorTypesWithExtraData = [
'MinimumOneEnvironmentError',
'BadDataError',
'BadRequestError',
'ValidationError',
Expand All @@ -42,10 +42,8 @@ const AllUnleashApiErrorTypes = [
] as const;

type UnleashApiErrorName = typeof AllUnleashApiErrorTypes[number];
type UnleashApiErrorNameWithoutExtraData = Exclude<
UnleashApiErrorName,
typeof UnleashApiErrorTypesWithExtraData[number]
>;
export type UnleashApiErrorNameWithoutExtraData =
typeof UnleashApiErrorTypes[number];

const statusCode = (errorName: UnleashApiErrorName): number => {
switch (errorName) {
Expand Down Expand Up @@ -174,6 +172,7 @@ export class UnleashError extends Error {
id: this.id,
name: this.name,
message: this.message,
details: [{ message: this.message, description: this.message }],
...this.additionalParameters,
};
}
Expand Down Expand Up @@ -228,22 +227,20 @@ export const fromLegacyError = (e: Error): UnleashError => {

if (
[
'ValidationError',
'BadRequestError',
'BadDataError',
'BadRequestError',
'InvalidTokenError',
'MinimumOneEnvironmentError',
'ValidationError',
].includes(name)
) {
return new UnleashError({
name: name as
| 'ValidationError'
| 'BadRequestError'
| 'BadDataError'
| 'InvalidTokenError'
| 'MinimumOneEnvironmentError',
| 'InvalidTokenError',
message:
'Your request body failed to validate. Refer to the `details` list to see what happened.',
'Request validation failed: your request body failed to validate. Refer to the `details` list to see what happened.',
details: [{ description: e.message, message: e.message }],
});
}
Expand Down
7 changes: 2 additions & 5 deletions src/lib/routes/util.ts
Expand Up @@ -26,14 +26,11 @@ export const handleErrors: (
logger: Logger,
error: Error,
) => void = (res, logger, error) => {
logger.warn(error.message);
// @ts-expect-error
// eslint-disable-next-line no-param-reassign
error.isJoi = true;

const finalError =
error instanceof UnleashError ? error : fromLegacyError(error);

logger.warn(finalError.id, finalError.message);

if (['InternalError', 'UnknownError'].includes(finalError.name)) {
logger.error('Server failed executing request', error);
}
Expand Down

0 comments on commit a7213bf

Please sign in to comment.