Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-05-08 01:15:49 +02:00
Code Issues Projects Releases Wiki Activity

feat: unify error responses (#3607)

Browse Source 
This PR implements the first version of a suggested unification (and
documentation) of the errors that we return from the API today.

The goal is for this to be the first step towards the error type defined
in this internal [linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type
'Define the new API error type').

## The state of things today

As things stand, we currently have no (or **very** little) documentation
of the errors that are returned from the API. We mention error codes,
but never what the errors may contain.

Second, there is no specified format for errors, so what they return is
arbitrary, and based on ... Who knows? As a result, we have multiple
different errors returned by the API depending on what operation you're
trying to do. What's more, with OpenAPI validation in the mix, it's
absolutely possible for you to get two completely different error
objects for operations to the same endpoint.

Third, the errors we do return are usually pretty vague and don't really
provide any real help to the user. "You don't have the right
permissions". Great. Well what permissions do I need? And how would I
know? "BadDataError". Sick. Why is it bad?

... You get it.

## What we want to achieve

The ultimate goal is for error messages to serve both humans and
machines. When the user provides bad data, we should tell them what
parts of the data are bad and what they can do to fix it. When they
don't have the right permissions, we should tell them what permissions
they need.

Additionally, it would be nice if we could provide an ID for each error
instance, so that you (or an admin) can look through the logs and locate
he incident.

## What's included in **this** PR?

This PR does not aim to implement everything above. It's not intended to
magically fix everything. Its goal is to implement the necessary
**breaking** changes, so that they can be included in v5. Changing error
messages is a slightly grayer area than changing APIs directly, but
changing the format is definitely something I'd consider breaking.

So this PR:

- defines a minimal version of the error type defined in the [API error
definition linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type).
- aims to catch all errors we return today and wrap them in the error
type
-   updates tests to match the new expectations.

An important point: because we are cutting v5 very soon and because work
for this wasn't started until last week, the code here isn't necessarily
very polished. But it doesn't need to be. The internals can be as messy
as we want, as long as the API surface is stable.

That said, I'm very open to feedback about design and code completeness,
etc, but this has intentionally been done quickly.

Please also see my inline comments on the changes for more specific
details.

### Proposed follow-ups

As mentioned, this is the first step to implementing the error type. The
public API error type only exposes `id`, `name`, and `message`. This is
barely any more than most of the previous messages, but they are now all
using the same format. Any additional properties, such as `suggestion`,
`help`, `documentationLink` etc can be added as features without
breaking the current format. This is an intentional limitation of this
PR.

Regarding additional properties: there are some error responses that
must contain extra properties. Some of these are documented in the types
of the new error constructor, but not all. This includes `path` and
`type` properties on 401 errors, `details` on validation errors, and
more.

Also, because it was put together quickly, I don't yet know exactly how
we (as developers) would **prefer** to use these new error messages
within the code, so the internal API (the new type, name, etc), is just
a suggestion. This can evolve naturally over time if (based on feedback
and experience) without changing the public API.

## Returning multiple errors

Most of the time when we return errors today, we only return a single
error (even if many things are wrong). AJV, the OpenAPI integration we
use does have a setting that allows it to return all errors in a request
instead of a single one. I suggest we turn that on, but that we do it in
a separate PR (because it updates a number of other snapshots).

When returning errors that point to `details`, the objects in the
`details` now contain a new `description` property. This "deprecates"
the `message` property. Due to our general deprecation policy, this
should be kept around for another full major and can be removed in v6.

```json
{
  "name": "BadDataError",
  "message": "Something went wrong. Check the `details` property for more information."
  "details": [{
    "message": "The .params property must be an object. You provided an array.",
    "description": "The .params property must be an object. You provided an array.",
  }]
}
```
This commit is contained in:
Thomas Heartman 2023-04-25 15:40:46 +02:00 committed by GitHub
parent 345f593b85
commit 2765ae2c70
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
29 changed files with 2693 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/lib/app.ts
View File

@ -28,6 +28,7 @@ import { Knex } from 'knex';
import maintenanceMiddleware from './middleware/maintenance-middleware';
import { unless } from './middleware/unless-middleware';
import { catchAllErrorHandler } from './middleware/catch-all-error-handler';
import { UnleashError } from './error/api-error';
export default async function getApp(
config: IUnleashConfig,
@ -183,12 +184,17 @@ export default async function getApp(
res.send(indexHTML);
});
app.get(`${baseUriPath}/*`, (req, res) => {
if (req.path.startsWith(`${baseUriPath}/api`)) {
res.status(404).send({ message: 'Not found' });
return;
}
// handle all API 404s
app.use(`${baseUriPath}/api`, (req, res) => {
const error = new UnleashError({
name: 'NotFoundError',
message: `The path you were looking for (${baseUriPath}/api${req.path}) is not available.`,
});
res.status(error.statusCode).send(error);
return;
});
app.get(`${baseUriPath}/*`, (req, res) => {
res.send(indexHTML);
});

219
src/lib/error/api-error.test.ts Normal file
View File

@ -0,0 +1,219 @@
import { ErrorObject } from 'ajv';
import {
ApiErrorSchema,
fromOpenApiValidationError,
fromOpenApiValidationErrors,
} from './api-error';
describe('OpenAPI error conversion', () => {
it('Gives useful error messages for missing properties', () => {
const error = {
keyword: 'required',
instancePath: '',
dataPath: '.body',
schemaPath: '#/components/schemas/addonCreateUpdateSchema/required',
params: {
missingProperty: 'enabled',
},
message: "should have required property 'enabled'",
};
const { description } = fromOpenApiValidationError({})(error);
// it tells the user that the property is required
expect(description.includes('required'));
// it tells the user the name of the missing property
expect(description.includes(error.params.missingProperty));
});
it('Gives useful error messages for type errors', () => {
const error = {
keyword: 'type',
instancePath: '',
dataPath: '.body.parameters',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/parameters/type',
params: {
type: 'object',
},
message: 'should be object',
};
const parameterValue = [];
const { description } = fromOpenApiValidationError({
parameters: parameterValue,
})(error);
// it provides the message
expect(description.includes(error.message));
// it tells the user what they provided
expect(description.includes(JSON.stringify(parameterValue)));
});
it('Gives useful pattern error messages', () => {
const error = {
instancePath: '',
keyword: 'pattern',
dataPath: '.body.description',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/description/pattern',
params: {
pattern: '^this is',
},
message: 'should match pattern "^this is"',
};
const requestDescription = 'A pattern that does not match.';
const { description } = fromOpenApiValidationError({
description: requestDescription,
})(error);
// it tells the user what the pattern it should match is
expect(description.includes(error.params.pattern));
// it tells the user which property it pertains to
expect(description.includes('description'));
// it tells the user what they provided
expect(description.includes(requestDescription));
});
it('Gives useful min/maxlength error messages', () => {
const error = {
instancePath: '',
keyword: 'maxLength',
dataPath: '.body.description',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/description/maxLength',
params: {
limit: 5,
},
message: 'should NOT be longer than 5 characters',
};
const requestDescription = 'Longer than the max length';
const { description } = fromOpenApiValidationError({
description: requestDescription,
})(error);
// it tells the user what the pattern it should match is
expect(description.includes(error.params.limit.toString()));
// it tells the user which property it pertains to
expect(description.includes('description'));
// it tells the user what they provided
expect(description.includes(requestDescription));
});
it('Handles numerical min/max errors', () => {
const error = {
keyword: 'maximum',
instancePath: '',
dataPath: '.body.newprop',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/newprop/maximum',
params: {
comparison: '<=',
limit: 5,
exclusive: false,
},
message: 'should be <= 5',
};
const propertyValue = 6;
const { description } = fromOpenApiValidationError({
newprop: propertyValue,
})(error);
// it tells the user what the limit is
expect(description.includes(error.params.limit.toString()));
// it tells the user what kind of comparison it performed
expect(description.includes(error.params.comparison));
// it tells the user which property it pertains to
expect(description.includes('newprop'));
// it tells the user what they provided
expect(description.includes(propertyValue.toString()));
});
it('Handles multiple errors', () => {
const errors: [ErrorObject, ...ErrorObject[]] = [
{
keyword: 'maximum',
instancePath: '',
// @ts-expect-error
dataPath: '.body.newprop',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/newprop/maximum',
params: {
comparison: '<=',
limit: 5,
exclusive: false,
},
message: 'should be <= 5',
},
{
keyword: 'required',
instancePath: '',
dataPath: '.body',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/required',
params: {
missingProperty: 'enabled',
},
message: "should have required property 'enabled'",
},
];
// create an error and serialize it as it would be shown to the end user.
const serializedUnleashError: ApiErrorSchema =
fromOpenApiValidationErrors({ newprop: 7 }, errors).toJSON();
expect(serializedUnleashError.name).toBe('ValidationError');
expect(serializedUnleashError.message).toContain('`details`');
expect(
serializedUnleashError.details!![0].description.includes('newprop'),
);
expect(
serializedUnleashError.details!![1].description.includes('enabled'),
);
});
it('Handles deeply nested properties gracefully', () => {
const error = {
keyword: 'type',
dataPath: '.body.nestedObject.a.b',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/a/properties/b/type',
params: { type: 'string' },
message: 'should be string',
instancePath: '',
};
const { description } = fromOpenApiValidationError({
nestedObject: { a: { b: [] } },
})(error);
// it should hold the full path to the error
expect(description.includes('nestedObject.a.b'));
// it should include the value that the user sent
expect(description.includes('[]'));
});
it('Handles deeply nested properties on referenced schemas', () => {
const error = {
keyword: 'type',
dataPath: '.body.nestedObject.a.b',
schemaPath: '#/components/schemas/parametersSchema/type',
params: { type: 'object' },
message: 'should be object',
instancePath: '',
};
const illegalValue = 'illegal string';
const { description } = fromOpenApiValidationError({
nestedObject: { a: { b: illegalValue } },
})(error);
// it should hold the full path to the error
expect(description.includes('nestedObject.a.b'));
// it should include the value that the user sent
expect(description.includes(illegalValue));
});
});

310
src/lib/error/api-error.ts Normal file
View File

@ -0,0 +1,310 @@
import { v4 as uuidV4 } from 'uuid';
import { FromSchema } from 'json-schema-to-ts';
import { ErrorObject } from 'ajv';
const UnleashApiErrorTypes = [
'OwaspValidationError',
'PasswordUndefinedError',
'NoAccessError',
'UsedTokenError',
'InvalidOperationError',
'IncompatibleProjectError',
'OperationDeniedError',
'NotFoundError',
'NameExistsError',
'FeatureHasTagError',
'RoleInUseError',
'ProjectWithoutOwnerError',
'UnknownError',
'PasswordMismatch',
'PasswordMismatchError',
'DisabledError',
'ContentTypeError',
'NotImplementedError',
// server errors; not the end user's fault
'InternalError',
] as const;
const UnleashApiErrorTypesWithExtraData = [
'MinimumOneEnvironmentError',
'BadDataError',
'BadRequestError',
'ValidationError',
'AuthenticationRequired',
'NoAccessError',
'InvalidTokenError',
] as const;
const AllUnleashApiErrorTypes = [
...UnleashApiErrorTypes,
...UnleashApiErrorTypesWithExtraData,
] as const;
type UnleashApiErrorName = typeof AllUnleashApiErrorTypes[number];
type UnleashApiErrorNameWithoutExtraData = Exclude<
UnleashApiErrorName,
typeof UnleashApiErrorTypesWithExtraData[number]
>;
const statusCode = (errorName: UnleashApiErrorName): number => {
switch (errorName) {
case 'ContentTypeError':
return 415;
case 'ValidationError':
return 400;
case 'BadDataError':
return 400;
case 'BadRequestError':
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':
case 'PasswordMismatchError':
return 401;
case 'DisabledError':
return 422;
case 'NotImplementedError':
return 405;
case 'NoAccessError':
return 403;
case 'AuthenticationRequired':
return 401;
}
};
type ValidationErrorDescription = {
description: string;
message: string;
path?: string;
};
type UnleashErrorData =
| {
message: string;
documentationLink?: string;
} & (
| {
name: UnleashApiErrorNameWithoutExtraData;
}
| {
name: 'NoAccessError';
permission: string;
}
| {
name: 'AuthenticationRequired';
path: string;
type: string;
}
| {
name:
| 'ValidationError'
| 'BadDataError'
| 'BadRequestError'
| 'MinimumOneEnvironmentError'
| 'InvalidTokenError';
details: [
ValidationErrorDescription,
...ValidationErrorDescription[],
];
}
);
export class UnleashError extends Error {
id: string;
name: UnleashApiErrorName;
statusCode: number;
additionalParameters: object;
constructor({
name,
message,
documentationLink,
...rest
}: UnleashErrorData) {
super();
this.id = uuidV4();
this.name = name;
super.message = message;
this.statusCode = statusCode(name);
this.additionalParameters = rest;
}
help(): string {
return `Get help for id ${this.id}`;
}
toJSON(): ApiErrorSchema {
return {
id: this.id,
name: this.name,
message: this.message,
...this.additionalParameters,
};
}
toString(): string {
return `${this.name}: ${this.message}.`;
}
}
export const apiErrorSchema = {
$id: '#/components/schemas/apiError',
type: 'object',
required: ['id', 'name', 'message'],
description:
'An Unleash API error. Contains information about what went wrong.',
properties: {
name: {
type: 'string',
enum: AllUnleashApiErrorTypes,
description:
'The kind of error that occurred. Meant for machine consumption.',
example: 'ValidationError',
},
id: {
type: 'string',
description:
'A unique identifier for this error instance. Can be used to search logs etc.',
example: '0b84c7fd-5278-4087-832d-0b502c7929b3',
},
message: {
type: 'string',
description: 'A human-readable explanation of what went wrong.',
example:
"We couldn't find an addon provider with the name that you are trying to add ('bogus-addon')",
},
},
components: {},
} as const;
export const fromLegacyError = (e: Error): UnleashError => {
const name = AllUnleashApiErrorTypes.includes(e.name as UnleashApiErrorName)
? (e.name as UnleashApiErrorName)
: 'UnknownError';
if (name === 'NoAccessError') {
return new UnleashError({
name,
message: e.message,
permission: 'unknown',
});
}
if (
[
'ValidationError',
'BadRequestError',
'BadDataError',
'InvalidTokenError',
'MinimumOneEnvironmentError',
].includes(name)
) {
return new UnleashError({
name: name as
| 'ValidationError'
| 'BadRequestError'
| 'BadDataError'
| 'InvalidTokenError'
| 'MinimumOneEnvironmentError',
message:
'Your request body failed to validate. Refer to the `details` list to see what happened.',
details: [{ description: e.message, message: e.message }],
});
}
if (name === 'AuthenticationRequired') {
return new UnleashError({
name,
message: `You must be authenticated to view this content. Please log in.`,
path: `/err/maybe/login?`,
type: 'password',
});
}
return new UnleashError({
name: name as UnleashApiErrorNameWithoutExtraData,
message: e.message,
});
};
export const fromOpenApiValidationError =
(requestBody: object) =>
(validationError: ErrorObject): ValidationErrorDescription => {
// @ts-expect-error Unsure why, but the `dataPath` isn't listed on the type definition for error objects. However, it's always there. Suspect this is a bug in the library.
const propertyName = validationError.dataPath.substring(
'.body.'.length,
);
if (validationError.keyword === 'required') {
const path =
propertyName + '.' + validationError.params.missingProperty;
const description = `The ${path} property is required. It was not present on the data you sent.`;
return {
path,
description,
message: description,
};
} else {
const youSent = JSON.stringify(requestBody[propertyName]);
const description = `The .${propertyName} property ${validationError.message}. You sent ${youSent}.`;
return {
description,
message: description,
path: propertyName,
};
}
};
export const fromOpenApiValidationErrors = (
requestBody: object,
validationErrors: [ErrorObject, ...ErrorObject[]],
): UnleashError => {
const details = validationErrors.map(
fromOpenApiValidationError(requestBody),
);
return new UnleashError({
name: 'ValidationError',
message:
"The request payload you provided doesn't conform to the schema. Check the `details` property for a list of errors that we found.",
// @ts-expect-error We know that the list is non-empty
details,
});
};
export type ApiErrorSchema = FromSchema<typeof apiErrorSchema>;

36
src/lib/error/no-access-error.ts
View File

@ -1,31 +1,17 @@
class NoAccessError extends Error {
permission: string;
name: string;
message: string;
environment?: string;
import { UnleashError } from './api-error';
class NoAccessError extends UnleashError {
constructor(permission: string, environment?: string) {
super();
const message =
`You don't have the required permissions to perform this operation. You need the "${permission}" permission to perform this action` +
(environment ? ` in the "${environment}" environment.` : `.`);
super({
name: 'NoAccessError',
message,
permission,
});
Error.captureStackTrace(this, this.constructor);
this.name = this.constructor.name;
this.permission = permission;
this.environment = environment;
if (environment) {
this.message = `You need permission=${permission} to perform this action on environment=${environment}`;
} else {
this.message = `You need permission=${permission} to perform this action`;
}
}
toJSON(): any {
return {
permission: this.permission,
message: this.message,
};
}
}

31
src/lib/features/export-import-toggles/export-import-service.ts
View File

@ -44,6 +44,7 @@ import { isValidField } from './import-context-validation';
import { IImportTogglesStore } from './import-toggles-store-type';
import { ImportPermissionsService } from './import-permissions-service';
import { ImportValidationMessages } from './import-validation-messages';
import { UnleashError } from '../../error/api-error';
export default class ExportImportService {
private logger: Logger;
@ -365,11 +366,16 @@ export default class ExportImportService {
Array.isArray(unsupportedContextFields) &&
unsupportedContextFields.length > 0
) {
throw new BadDataError(
`Context fields with errors: ${unsupportedContextFields
.map((field) => field.name)
.join(', ')}`,
);
throw new UnleashError({
name: 'BadDataError',
message:
'Some of the context fields you are trying to import are not supported.',
// @ts-ignore-error We know that the array contains at least one
// element here.
errors: unsupportedContextFields.map((field) => ({
description: `${field.name} is not supported.`,
})),
});
}
}
@ -441,11 +447,16 @@ export default class ExportImportService {
private async verifyStrategies(dto: ImportTogglesSchema) {
const unsupportedStrategies = await this.getUnsupportedStrategies(dto);
if (unsupportedStrategies.length > 0) {
throw new BadDataError(
`Unsupported strategies: ${unsupportedStrategies
.map((strategy) => strategy.name)
.join(', ')}`,
);
throw new UnleashError({
name: 'BadDataError',
message:
'Some of the strategies you are trying to import are not supported.',
// @ts-ignore-error We know that the array contains at least one
// element here.
errors: unsupportedStrategies.map((strategy) => ({
description: `${strategy.name} is not supported.`,
})),
});
}
}

24
src/lib/features/export-import-toggles/export-import.e2e.test.ts
View File

@ -671,13 +671,11 @@ test('reject import with unknown context fields', async () => {
400,
);
expect(body).toMatchObject({
details: [
{
message: 'Context fields with errors: ContextField1',
},
],
});
expect(
body.errors.includes((error) =>
error.description.includes('ContextField1'),
),
);
});
test('reject import with unsupported strategies', async () => {
@ -697,13 +695,11 @@ test('reject import with unsupported strategies', async () => {
400,
);
expect(body).toMatchObject({
details: [
{
message: 'Unsupported strategies: customStrategy',
},
],
});
expect(
body.errors.includes((error) =>
error.description.includes('customStrategy'),
),
);
});
test('validate import data', async () => {

29
src/lib/middleware/authorization-middleware.ts
View File

@ -1,7 +1,7 @@
import { IAuthRequest } from '../routes/unleash-types';
import { NextFunction, Response } from 'express';
import AuthenticationRequired from '../types/authentication-required';
import { LogProvider } from '../logger';
import { UnleashError } from '../error/api-error';
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
const authorizationMiddleware = (
@ -11,13 +11,6 @@ const authorizationMiddleware = (
const logger = getLogger('/middleware/authorization-middleware.ts');
logger.debug('Enabling Authorization middleware');
const generateAuthResponse = async () =>
new AuthenticationRequired({
type: 'password',
path: `${baseUriPath}/auth/simple/login`,
message: 'You must sign in order to use Unleash',
});
return async (req: IAuthRequest, res: Response, next: NextFunction) => {
if (req.session && req.session.user) {
req.user = req.session.user;
@ -28,11 +21,23 @@ const authorizationMiddleware = (
}
if (req.header('authorization')) {
// API clients should get 401 without body
return res.sendStatus(401);
return res.status(401).json(
new UnleashError({
name: 'PasswordMismatchError',
message: 'You must log in to use Unleash.',
}),
);
}
// Admin UI users should get auth-response
const authRequired = await generateAuthResponse();
return res.status(401).json(authRequired);
const path = `${baseUriPath}/auth/simple/login`;
const error = new UnleashError({
name: 'AuthenticationRequired',
message: `You must log in to use Unleash. Your request had no authorization header, so we could not authorize you. Try logging in at ${baseUriPath}/auth/simple/login.`,
type: 'password',
path,
});
return res.status(error.statusCode).json(error);
};
};

10
src/lib/middleware/content_type_checker.test.ts
View File

@ -16,7 +16,10 @@ const returns415: (t: jest.Mock) => Response = (t) => ({
status: (code) => {
expect(415).toBe(code);
return {
end: t,
json: () => ({
// @ts-ignore
end: t,
}),
};
},
});
@ -25,7 +28,10 @@ const expectNoCall: (t: jest.Mock) => Response = (t) => ({
// @ts-ignore
status: () => ({
// @ts-ignore
end: () => expect(t).toHaveBeenCalledTimes(0),
json: () => ({
// @ts-ignore
end: () => expect(t).toHaveBeenCalledTimes(0),
}),
}),
});

11
src/lib/middleware/content_type_checker.ts
View File

@ -1,4 +1,5 @@
import { RequestHandler } from 'express';
import { UnleashError } from '../error/api-error';
import { is } from 'type-is';
const DEFAULT_ACCEPTED_CONTENT_TYPE = 'application/json';
@ -21,7 +22,15 @@ export default function requireContentType(
if (is(contentType, acceptedContentTypes)) {
next();
} else {
res.status(415).end();
const error = new UnleashError({
name: 'ContentTypeError',
message: `We do not accept the content-type you provided (${
contentType || "you didn't provide one"
}). Try using one of the content-types we do accept instead (${acceptedContentTypes.join(
', ',
)}) and make sure the body is in the corresponding format.`,
});
res.status(error.statusCode).json(error).end();
}
};
}

175
src/lib/openapi/util/standard-responses.ts
View File

@ -5,33 +5,206 @@ export const emptyResponse = {
const unauthorizedResponse = {
description:
'Authorization information is missing or invalid. Provide a valid API token as the `authorization` header, e.g. `authorization:*.*.my-admin-token`.',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'AuthenticationRequired',
description: 'The name of the error kind',
},
message: {
type: 'string',
example:
'You must log in to use Unleash. Your request had no authorization header, so we could not authorize you. Try logging in at /auth/simple/login.',
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const forbiddenResponse = {
description:
'User credentials are valid but does not have enough privileges to execute this operation',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'NoAccessError',
description: 'The name of the error kind',
},
message: {
type: 'string',
example:
'You need the "UPDATE_ADDON" permission to perform this action in the "development" environment.',
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const badRequestResponse = {
description: 'The request data does not match what we expect.',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'ValidationError',
description: 'The name of the error kind',
},
message: {
type: 'string',
example: `The request payload you provided doesn't conform to the schema. The .parameters property should be object. You sent [].`,
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const notFoundResponse = {
description: 'The requested resource was not found.',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'NotFoundError',
description: 'The name of the error kind',
},
message: {
type: 'string',
example: `Could not find the addon with ID "12345".`,
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const conflictResponse = {
description:
'The provided resource can not be created or updated because it would conflict with the current state of the resource or with an already existing resource, respectively.',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'NameExistsError',
description: 'The name of the error kind',
},
message: {
type: 'string',
example:
'There is already a feature called "my-awesome-feature".',
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const contentTooLargeResponse = {
description:
'The body request body is larger than what we accept. By default we only accept bodies of 100kB or less',
'The request body is larger than what we accept. By default we only accept bodies of 100kB or less',
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'ContentTooLarge',
description: 'The name of the error kind',
},
message: {
type: 'string',
example:
'You provided more data than we can handle. Unleash accepts at most X MB.',
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const unsupportedMediaTypeResponse = {
description: `The operation does not support request payloads of the provided type. Please ensure that you're using one of the listed payload types and that you have specified the right content type in the "content-type" header.`,
content: {
'application/json': {
schema: {
type: 'object',
properties: {
id: {
type: 'string',
example: '9c40958a-daac-400e-98fb-3bb438567008',
description: 'The ID of the error instance',
},
name: {
type: 'string',
example: 'ContentTypeerror',
description: 'The name of the error kind',
},
message: {
type: 'string',
example:
'We do not accept the content-type you provided (application/xml). Try using one of the content-types we do accept instead (application/json) and make sure the body is in the corresponding format.',
description: 'A description of what went wrong.',
},
},
},
},
},
} as const;
const standardResponses = {

4
src/lib/routes/admin-api/addon.ts
View File

@ -121,7 +121,7 @@ Note: passing \`null\` as a value for the description property will set it to an
requestBody: createRequestSchema('addonCreateUpdateSchema'),
responses: {
200: createResponseSchema('addonSchema'),
...getStandardResponses(400, 401, 403, 413, 415),
...getStandardResponses(400, 401, 403, 404, 413, 415),
},
}),
],
@ -142,7 +142,7 @@ Note: passing \`null\` as a value for the description property will set it to an
operationId: 'deleteAddon',
responses: {
200: emptyResponse,
...getStandardResponses(401, 403),
...getStandardResponses(401, 403, 404),
},
}),
],

8
src/lib/routes/admin-api/strategy.test.ts
View File

@ -53,8 +53,10 @@ test('require a name when creating a new strategy', async () => {
.send({})
.expect(400)
.expect((res) => {
expect(res.body.validation[0].message).toEqual(
"should have required property 'name'",
expect(
['name', 'property', 'required'].every((word) =>
res.body.details[0].description.includes(word),
),
);
});
});
@ -66,7 +68,7 @@ test('require parameters array when creating a new strategy', async () => {
.send({ name: 'TestStrat' })
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toEqual(
expect(res.body.details[0].description).toEqual(
'"parameters" is required',
);
});

5
src/lib/routes/proxy-api/index.ts
View File

@ -20,6 +20,7 @@ import {
import { Context } from 'unleash-client';
import { enrichContextWithIp } from '../../proxy';
import { corsOriginMiddleware } from '../../middleware';
import { UnleashError } from '../../error/api-error';
interface ApiUserRequest<
PARAM = any,
@ -135,9 +136,11 @@ export default class ProxyController extends Controller {
req: ApiUserRequest,
res: Response,
) {
res.status(405).json({
const error = new UnleashError({
name: 'NotImplementedError',
message: 'The frontend API does not support this endpoint.',
});
res.status(error.statusCode).json(error);
}
private async getProxyFeatures(

51
src/lib/routes/util.ts
View File

@ -1,7 +1,7 @@
import joi from 'joi';
import { Response } from 'express';
import { Logger } from '../logger';
import BaseError from '../error/base-error';
import { fromLegacyError, UnleashError } from '../error/api-error';
export const customJoi = joi.extend((j) => ({
type: 'isUrlFriendly',
@ -31,49 +31,12 @@ export const handleErrors: (
// eslint-disable-next-line no-param-reassign
error.isJoi = true;
if (error instanceof BaseError) {
return res.status(error.statusCode).json(error).end();
const finalError =
error instanceof UnleashError ? error : fromLegacyError(error);
if (['InternalError', 'UnknownError'].includes(finalError.name)) {
logger.error('Server failed executing request', error);
}
switch (error.name) {
case 'ValidationError':
return res.status(400).json(error).end();
case 'BadDataError':
return res.status(400).json(error).end();
case 'BadRequestError':
return res.status(400).json(error).end();
case 'OwaspValidationError':
return res.status(400).json(error).end();
case 'PasswordUndefinedError':
return res.status(400).json(error).end();
case 'MinimumOneEnvironmentError':
return res.status(400).json(error).end();
case 'InvalidTokenError':
return res.status(401).json(error).end();
case 'NoAccessError':
return res.status(403).json(error).end();
case 'UsedTokenError':
return res.status(403).json(error).end();
case 'InvalidOperationError':
return res.status(403).json(error).end();
case 'IncompatibleProjectError':
return res.status(403).json(error).end();
case 'OperationDeniedError':
return res.status(403).json(error).end();
case 'NotFoundError':
return res.status(404).json(error).end();
case 'NameExistsError':
return res.status(409).json(error).end();
case 'FeatureHasTagError':
return res.status(409).json(error).end();
case 'RoleInUseError':
return res.status(400).json(error).end();
case 'ProjectWithoutOwnerError':
return res.status(409).json(error).end();
case 'TypeError':
return res.status(400).json(error).end();
default:
logger.error('Server failed executing request', error);
return res.status(500).end();
}
return res.status(finalError.statusCode).json(finalError).end();
};

3
src/lib/services/addon-service.ts
View File

@ -212,10 +212,11 @@ export default class AddonService {
data: IAddonDto,
userName: string,
): Promise<IAddon> {
const existingConfig = await this.addonStore.get(id); // because getting an early 404 here makes more sense
const addonConfig = await addonSchema.validateAsync(data);
await this.validateKnownProvider(addonConfig);
await this.validateRequiredParameters(addonConfig);
if (this.sensitiveParams[addonConfig.provider].length > 0) {
const existingConfig = await this.addonStore.get(id);
addonConfig.parameters = Object.keys(addonConfig.parameters).reduce(
(params, key) => {
const o = { ...params };

11
src/lib/services/openapi-service.ts
View File

@ -11,6 +11,7 @@ import { ApiOperation } from '../openapi/util/api-operation';
import { Logger } from '../logger';
import { validateSchema } from '../openapi/validate';
import { IFlagResolver } from '../types';
import { fromOpenApiValidationErrors } from '../error/api-error';
export class OpenApiService {
private readonly config: IUnleashConfig;
@ -58,10 +59,12 @@ export class OpenApiService {
useErrorHandler(app: Express): void {
app.use((err, req, res, next) => {
if (err && err.status && err.validationErrors) {
res.status(err.status).json({
error: err.message,
validation: err.validationErrors,
});
const apiError = fromOpenApiValidationErrors(
req.body,
err.validationErrors,
);
res.status(apiError.statusCode).json(apiError);
} else {
next(err);
}

15
src/lib/services/user-service.ts
View File

@ -24,10 +24,10 @@ import SettingService from './setting-service';
import { SimpleAuthSettings } from '../server-impl';
import { simpleAuthSettingsKey } from '../types/settings/simple-auth-settings';
import DisabledError from '../error/disabled-error';
import PasswordMismatch from '../error/password-mismatch';
import BadDataError from '../error/bad-data-error';
import { isDefined } from '../util/isDefined';
import { TokenUserSchema } from '../openapi/spec/token-user-schema';
import { UnleashError } from '../error/api-error';
const systemUser = new User({ id: -1, username: 'system' });
@ -78,12 +78,15 @@ class UserService {
private passwordResetTimeouts: { [key: string]: NodeJS.Timeout } = {};
private baseUriPath: string;
constructor(
stores: Pick<IUnleashStores, 'userStore' | 'eventStore'>,
{
server,
getLogger,
authentication,
}: Pick<IUnleashConfig, 'getLogger' | 'authentication'>,
}: Pick<IUnleashConfig, 'getLogger' | 'authentication' | 'server'>,
services: {
accessService: AccessService;
resetTokenService: ResetTokenService;
@ -103,6 +106,8 @@ class UserService {
if (authentication && authentication.createAdminUser) {
process.nextTick(() => this.initAdminUser());
}
this.baseUriPath = server.baseUriPath || '';
}
validatePassword(password: string): boolean {
@ -298,7 +303,11 @@ class UserService {
await this.store.successfullyLogin(user);
return user;
}
throw new PasswordMismatch();
throw new UnleashError({
name: 'PasswordMismatchError',
message: `The combination of password and username you provided is invalid. If you have forgotten your password, visit ${this.baseUriPath}/forgotten-password or get in touch with your instance administrator.`,
});
}
/**

4
src/test/e2e/api/admin/api-token.e2e.test.ts
View File

@ -258,7 +258,7 @@ test('should not create token for invalid projectId', async () => {
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/bogus-project-something/,
);
});
@ -275,7 +275,7 @@ test('should not create token for invalid environment', async () => {
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/bogus-environment-something/,
);
});

4
src/test/e2e/api/admin/feature.e2e.test.ts
View File

@ -327,9 +327,7 @@ test('refuses to create a new feature toggle with variant when type is json but
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.isJoi).toBe(true);
expect(res.body.details[0].type).toBe('invalidJsonString');
expect(res.body.details[0].message).toBe(
expect(res.body.details[0].description).toBe(
`'value' must be a valid json string when 'type' is json`,
);
});

2
src/test/e2e/api/admin/project/environments.e2e.test.ts
View File

@ -100,7 +100,7 @@ test('Should not remove environment from project if project only has one environ
.delete(`/api/admin/projects/default/environments/default`)
.expect(400)
.expect((r) => {
expect(r.body.details[0].message).toBe(
expect(r.body.details[0].description).toBe(
'You must always have one active environment',
);
});

12
src/test/e2e/api/admin/project/features.e2e.test.ts
View File

@ -184,9 +184,8 @@ test('Trying to add a strategy configuration to environment not connected to tog
})
.expect(400)
.expect((r) => {
expect(r.body.details[0].message).toBe(
'You have not added the current environment to the project',
);
expect(r.body.message.includes('environment'));
expect(r.body.message.includes('project'));
});
});
@ -777,8 +776,11 @@ test('Trying to patch variants on a feature toggle should trigger an OperationDe
])
.expect(403)
.expect((res) => {
expect(res.body.details[0].message).toEqual(
'Changing variants is done via PATCH operation to /api/admin/projects/:project/features/:feature/variants',
expect(res.body.message.includes('PATCH'));
expect(
res.body.message.includes(
'/api/admin/projects/:project/features/:feature/variants',
),
);
});
});

12
src/test/e2e/api/admin/project/variants.e2e.test.ts
View File

@ -489,7 +489,7 @@ test('PUTing an invalid variant throws 400 exception', async () => {
.expect(400)
.expect((res) => {
expect(res.body.details).toHaveLength(1);
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/.*weightType" must be one of/,
);
});
@ -524,7 +524,7 @@ test('Invalid variant in PATCH also throws 400 exception', async () => {
.expect(400)
.expect((res) => {
expect(res.body.details).toHaveLength(1);
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/.*weight" must be less than or equal to 1000/,
);
});
@ -652,7 +652,7 @@ test('PATCHING with no variable variants fails with 400', async () => {
.expect(400)
.expect((res) => {
expect(res.body.details).toHaveLength(1);
expect(res.body.details[0].message).toEqual(
expect(res.body.details[0].description).toEqual(
'There must be at least one "variable" variant',
);
});
@ -847,7 +847,7 @@ test('If sum of fixed variant weight exceed 1000 fails with 400', async () => {
.expect(400)
.expect((res) => {
expect(res.body.details).toHaveLength(1);
expect(res.body.details[0].message).toEqual(
expect(res.body.details[0].description).toEqual(
'The traffic distribution total must equal 100%',
);
});
@ -960,7 +960,7 @@ test('PATCH endpoint validates uniqueness of variant names', async () => {
.send(patch)
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/contains a duplicate value/,
);
});
@ -996,7 +996,7 @@ test('PUT endpoint validates uniqueness of variant names', async () => {
])
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toMatch(
expect(res.body.details[0].description).toMatch(
/contains a duplicate value/,
);
});

4
src/test/e2e/api/admin/tag-types.e2e.test.ts
View File

@ -77,7 +77,7 @@ test('Invalid tag types gets rejected', async () => {
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toBe(
expect(res.body.details[0].description).toBe(
'"name" must be URL friendly',
);
});
@ -151,7 +151,7 @@ test('Invalid tag-types get refused by validator', async () => {
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toBe(
expect(res.body.details[0].description).toBe(
'"name" must be URL friendly',
);
});

2
src/test/e2e/api/admin/tags.e2e.test.ts
View File

@ -87,7 +87,7 @@ test('Can validate a tag', async () =>
.expect(400)
.expect((res) => {
expect(res.body.details.length).toBe(1);
expect(res.body.details[0].message).toBe(
expect(res.body.details[0].description).toBe(
'"type" must be URL friendly',
);
}));

2
src/test/e2e/api/admin/user-admin.e2e.test.ts
View File

@ -145,7 +145,7 @@ test('should require username or email on create', async () => {
.set('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body.details[0].message).toEqual(
expect(res.body.details[0].description).toEqual(
'You must specify username or email',
);
});

2
src/test/e2e/api/auth/reset-password-controller.e2e.test.ts
View File

@ -170,7 +170,7 @@ test('Trying to reset password with same token twice does not work', async () =>
})
.expect(401)
.expect((res) => {
expect(res.body.details[0].message).toBeTruthy();
expect(res.body.details[0].description).toBeTruthy();
});
});

1838
src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap
View File

File diff suppressed because it is too large Load Diff

6
src/test/e2e/services/access-service.e2e.test.ts
View File

@ -776,9 +776,9 @@ test('Should be denied move feature toggle to project where the user does not ha
projectOrigin.id,
);
} catch (e) {
expect(e.toString()).toBe(
'NoAccessError: You need permission=MOVE_FEATURE_TOGGLE to perform this action',
);
expect(e.name).toContain('NoAccess');
expect(e.message.includes('permission'));
expect(e.message.includes(permissions.MOVE_FEATURE_TOGGLE));
}
});

10
src/test/e2e/services/project-service.e2e.test.ts
View File

@ -538,9 +538,8 @@ test('should not change project if feature toggle project does not match current
'wrong-project-id',
);
} catch (err) {
expect(err.message).toBe(
`You need permission=${MOVE_FEATURE_TOGGLE} to perform this action`,
);
expect(err.message.toLowerCase().includes('permission'));
expect(err.message.includes(MOVE_FEATURE_TOGGLE));
}
});
@ -605,9 +604,8 @@ test('should fail if user is not authorized', async () => {
project.id,
);
} catch (err) {
expect(err.message).toBe(
`You need permission=${MOVE_FEATURE_TOGGLE} to perform this action`,
);
expect(err.message.toLowerCase().includes('permission'));
expect(err.message.includes(MOVE_FEATURE_TOGGLE));
}
});