diff --git a/.mergify.yml b/.mergify.yml new file mode 100644 index 00000000000..9491ea01d9b --- /dev/null +++ b/.mergify.yml @@ -0,0 +1,6 @@ +pull_request_rules: + - name: Automatic update of all PRs + conditions: + - base=main # Targeting the main branch + actions: + update: {} # Update PR with base branch diff --git a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx index e2ace7b571f..e9a1f764ac8 100644 --- a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx +++ b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx @@ -9,7 +9,6 @@ import { StyledInput, StyledTextField, } from './ProjectForm.styles'; -import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; import { StickinessSelect } from 'component/feature/StrategyTypes/FlexibleStrategy/StickinessSelect/StickinessSelect'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import Select from 'component/common/select'; @@ -60,9 +59,6 @@ const ProjectForm: React.FC = ({ validateProjectId, clearErrors, }) => { - const { uiConfig } = useUiConfig(); - const { projectScopedStickiness } = uiConfig.flags; - return ( @@ -109,10 +105,7 @@ const ProjectForm: React.FC = ({ /> diff --git a/frontend/src/hooks/useDefaultProjectSettings.ts b/frontend/src/hooks/useDefaultProjectSettings.ts index 7f0adfa4603..553fe6a510e 100644 --- a/frontend/src/hooks/useDefaultProjectSettings.ts +++ b/frontend/src/hooks/useDefaultProjectSettings.ts @@ -1,15 +1,10 @@ -import useUiConfig from './api/getters/useUiConfig/useUiConfig'; import useProject from './api/getters/useProject/useProject'; const DEFAULT_STICKINESS = 'default'; export const useDefaultProjectSettings = (projectId: string) => { - const { uiConfig } = useUiConfig(); - - const { projectScopedStickiness } = uiConfig.flags; - const { project, loading, error } = useProject(projectId); return { - defaultStickiness: Boolean(projectScopedStickiness) + defaultStickiness: Boolean(project.defaultStickiness) ? project.defaultStickiness : DEFAULT_STICKINESS, mode: project.mode, diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index e4c534557db..8a6a7652297 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -46,7 +46,6 @@ export interface IFlags { proPlanAutoCharge?: boolean; notifications?: boolean; bulkOperations?: boolean; - projectScopedStickiness?: boolean; personalAccessTokensKillSwitch?: boolean; demo?: boolean; strategyTitle?: boolean; diff --git a/package.json b/package.json index 8c635dd5b7e..9db14c23989 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "unleash-server", "description": "Unleash is an enterprise ready feature toggles service. It provides different strategies for handling feature toggles.", - "version": "5.0.0-beta.0", + "version": "5.0.0", "keywords": [ "unleash", "feature toggle", diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 050279a6bf2..374f6f5dc26 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -85,7 +85,6 @@ exports[`should create default config 1`] = ` "optimal304Differ": false, "personalAccessTokensKillSwitch": false, "proPlanAutoCharge": false, - "projectScopedStickiness": false, "responseTimeWithAppNameKillSwitch": false, "strategyDisable": false, "strategyTitle": false, @@ -113,7 +112,6 @@ exports[`should create default config 1`] = ` "optimal304Differ": false, "personalAccessTokensKillSwitch": false, "proPlanAutoCharge": false, - "projectScopedStickiness": false, "responseTimeWithAppNameKillSwitch": false, "strategyDisable": false, "strategyTitle": false, diff --git a/src/lib/error/api-error.test.ts b/src/lib/error/api-error.test.ts index 1675c273884..8077246fd89 100644 --- a/src/lib/error/api-error.test.ts +++ b/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', () => { @@ -21,9 +59,9 @@ describe('OpenAPI error conversion', () => { const { description } = fromOpenApiValidationError({})(error); // it tells the user that the property is required - expect(description.includes('required')); + expect(description.includes('required')).toBeTruthy(); // it tells the user the name of the missing property - expect(description.includes(error.params.missingProperty)); + expect(description.includes(error.params.missingProperty)).toBeTruthy(); }); it('Gives useful error messages for type errors', () => { @@ -45,9 +83,11 @@ describe('OpenAPI error conversion', () => { })(error); // it provides the message - expect(description.includes(error.message)); + expect(description.includes(error.message)).toBeTruthy(); // it tells the user what they provided - expect(description.includes(JSON.stringify(parameterValue))); + expect( + description.includes(JSON.stringify(parameterValue)), + ).toBeTruthy(); }); it('Gives useful pattern error messages', () => { @@ -69,11 +109,11 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the pattern it should match is - expect(description.includes(error.params.pattern)); + expect(description.includes(error.params.pattern)).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('description')); + expect(description.includes('description')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(requestDescription)); + expect(description.includes(requestDescription)).toBeTruthy(); }); it('Gives useful min/maxlength error messages', () => { @@ -95,11 +135,13 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the pattern it should match is - expect(description.includes(error.params.limit.toString())); + expect( + description.includes(error.params.limit.toString()), + ).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('description')); + expect(description.includes('description')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(requestDescription)); + expect(description.includes(requestDescription)).toBeTruthy(); }); it('Handles numerical min/max errors', () => { @@ -123,13 +165,15 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the limit is - expect(description.includes(error.params.limit.toString())); + expect( + description.includes(error.params.limit.toString()), + ).toBeTruthy(); // it tells the user what kind of comparison it performed - expect(description.includes(error.params.comparison)); + expect(description.includes(error.params.comparison)).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('newprop')); + expect(description.includes('newprop')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(propertyValue.toString())); + expect(description.includes(propertyValue.toString())).toBeTruthy(); }); it('Handles multiple errors', () => { @@ -175,6 +219,65 @@ describe('OpenAPI error conversion', () => { ); }); + describe('Disallowed additional properties', () => { + it('gives useful messages for base-level properties', () => { + const openApiError = { + keyword: 'additionalProperties', + instancePath: '', + dataPath: '.body', + schemaPath: + '#/components/schemas/addonCreateUpdateSchema/additionalProperties', + params: { additionalProperty: 'bogus' }, + message: 'should NOT have additional properties', + }; + + const error = fromOpenApiValidationError({ bogus: 5 })( + openApiError, + ); + + expect( + error.description.includes( + openApiError.params.additionalProperty, + ), + ).toBeTruthy(); + expect(error.description).toMatch(/\broot\b/i); + expect(error.description).toMatch(/\badditional properties\b/i); + }); + + it('gives useful messages for nested properties', () => { + const request2 = { + nestedObject: { + nested2: { extraPropertyName: 'illegal property' }, + }, + }; + const openApiError = { + keyword: 'additionalProperties', + instancePath: '', + dataPath: '.body.nestedObject.nested2', + schemaPath: + '#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/nested2/additionalProperties', + params: { additionalProperty: 'extraPropertyName' }, + message: 'should NOT have additional properties', + }; + + const error = fromOpenApiValidationError(request2)(openApiError); + + expect( + error.description.includes('nestedObject.nested2'), + ).toBeTruthy(); + expect( + error.description.includes( + openApiError.params.additionalProperty, + ), + ).toBeTruthy(); + expect( + error.description + .toLowerCase() + .includes('additional properties'), + ).toBeTruthy(); + }); + }); + it('Handles deeply nested properties gracefully', () => { const error = { keyword: 'type', @@ -191,9 +294,9 @@ describe('OpenAPI error conversion', () => { })(error); // it should hold the full path to the error - expect(description.includes('nestedObject.a.b')); + expect(description).toMatch(/\bnestedObject.a.b\b/); // it should include the value that the user sent - expect(description.includes('[]')); + expect(description.includes('[]')).toBeTruthy(); }); it('Handles deeply nested properties on referenced schemas', () => { @@ -212,8 +315,8 @@ describe('OpenAPI error conversion', () => { })(error); // it should hold the full path to the error - expect(description.includes('nestedObject.a.b')); + expect(description).toMatch(/\bnestedObject.a.b\b/); // it should include the value that the user sent - expect(description.includes(illegalValue)); + expect(description.includes(illegalValue)).toBeTruthy(); }); }); diff --git a/src/lib/error/api-error.ts b/src/lib/error/api-error.ts index 1a80d005f15..70fa805118d 100644 --- a/src/lib/error/api-error.ts +++ b/src/lib/error/api-error.ts @@ -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', @@ -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) { @@ -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, }; } @@ -228,11 +227,10 @@ export const fromLegacyError = (e: Error): UnleashError => { if ( [ - 'ValidationError', - 'BadRequestError', 'BadDataError', + 'BadRequestError', 'InvalidTokenError', - 'MinimumOneEnvironmentError', + 'ValidationError', ].includes(name) ) { return new UnleashError({ @@ -240,10 +238,9 @@ export const fromLegacyError = (e: Error): UnleashError => { | '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 }], }); } @@ -279,8 +276,24 @@ export const fromOpenApiValidationError = description, message: description, }; + } else if (validationError.keyword === 'additionalProperties') { + const path = + (propertyName ? propertyName + '.' : '') + + validationError.params.additionalProperty; + const description = `The ${ + propertyName ? `\`${propertyName}\`` : 'root' + } object of the request body does not allow additional properties. Your request included the \`${path}\` property.`; + return { + path, + description, + message: description, + }; } else { - const youSent = JSON.stringify(requestBody[propertyName]); + const input = propertyName + .split('.') + .reduce((x, prop) => x[prop], requestBody); + + const youSent = JSON.stringify(input); const description = `The .${propertyName} property ${validationError.message}. You sent ${youSent}.`; return { description, diff --git a/src/lib/features/export-import-toggles/export-import-service.ts b/src/lib/features/export-import-toggles/export-import-service.ts index a0397bd6775..d478cabfcb6 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -372,9 +372,13 @@ export default class ExportImportService { '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.`, - })), + details: unsupportedContextFields.map((field) => { + const description = `${field.name} is not supported.`; + return { + description, + message: description, + }; + }), }); } } @@ -453,9 +457,14 @@ export default class ExportImportService { '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.`, - })), + details: unsupportedStrategies.map((strategy) => { + const description = `${strategy.name} is not supported.`; + + return { + description, + message: description, + }; + }), }); } } diff --git a/src/lib/features/export-import-toggles/export-import.e2e.test.ts b/src/lib/features/export-import-toggles/export-import.e2e.test.ts index b61f31d3485..7d75ed7fb03 100644 --- a/src/lib/features/export-import-toggles/export-import.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import.e2e.test.ts @@ -673,11 +673,7 @@ test('reject import with unknown context fields', async () => { 400, ); - expect( - body.errors.includes((error) => - error.description.includes('ContextField1'), - ), - ); + expect(body.details[0].description).toMatch(/\bContextField1\b/); }); test('reject import with unsupported strategies', async () => { @@ -697,11 +693,7 @@ test('reject import with unsupported strategies', async () => { 400, ); - expect( - body.errors.includes((error) => - error.description.includes('customStrategy'), - ), - ); + expect(body.details[0].description).toMatch(/\bcustomStrategy\b/); }); test('validate import data', async () => { diff --git a/src/lib/routes/admin-api/strategy.test.ts b/src/lib/routes/admin-api/strategy.test.ts index 1a6ce3bcaf7..f1d13402f3e 100644 --- a/src/lib/routes/admin-api/strategy.test.ts +++ b/src/lib/routes/admin-api/strategy.test.ts @@ -57,7 +57,7 @@ test('require a name when creating a new strategy', async () => { ['name', 'property', 'required'].every((word) => res.body.details[0].description.includes(word), ), - ); + ).toBeTruthy(); }); }); diff --git a/src/lib/routes/util.ts b/src/lib/routes/util.ts index 078c6d89e99..3a262c7e804 100644 --- a/src/lib/routes/util.ts +++ b/src/lib/routes/util.ts @@ -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); } diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 509712cf146..b0d40a79443 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -53,10 +53,6 @@ const flags = { process.env.UNLEASH_BULK_OPERATIONS, false, ), - projectScopedStickiness: parseEnvVarBoolean( - process.env.PROJECT_SCOPED_STICKINESS, - false, - ), personalAccessTokensKillSwitch: parseEnvVarBoolean( process.env.UNLEASH_PAT_KILL_SWITCH, false, diff --git a/src/server-dev.ts b/src/server-dev.ts index 35ab7503732..4b782ba9404 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -40,7 +40,6 @@ process.nextTick(async () => { responseTimeWithAppNameKillSwitch: false, newProjectOverview: true, bulkOperations: true, - projectScopedStickiness: true, optimal304: true, optimal304Differ: false, }, diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 451299d1e21..c249bb27b0e 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -184,8 +184,10 @@ test('Trying to add a strategy configuration to environment not connected to tog }) .expect(400) .expect((r) => { - expect(r.body.message.includes('environment')); - expect(r.body.message.includes('project')); + expect( + r.body.details[0].message.includes('environment'), + ).toBeTruthy(); + expect(r.body.details[0].message.includes('project')).toBeTruthy(); }); }); @@ -776,12 +778,12 @@ test('Trying to patch variants on a feature toggle should trigger an OperationDe ]) .expect(403) .expect((res) => { - expect(res.body.message.includes('PATCH')); + expect(res.body.message.includes('PATCH')).toBeTruthy(); expect( res.body.message.includes( '/api/admin/projects/:project/features/:feature/variants', ), - ); + ).toBeTruthy(); }); }); diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 7805365a4d8..fbf8c848993 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -777,8 +777,10 @@ test('Should be denied move feature toggle to project where the user does not ha ); } catch (e) { expect(e.name).toContain('NoAccess'); - expect(e.message.includes('permission')); - expect(e.message.includes(permissions.MOVE_FEATURE_TOGGLE)); + expect(e.message.includes('permission')).toBeTruthy(); + expect( + e.message.includes(permissions.MOVE_FEATURE_TOGGLE), + ).toBeTruthy(); } }); diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 44afcb4a2a7..2fe56648f1a 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -538,8 +538,8 @@ test('should not change project if feature toggle project does not match current 'wrong-project-id', ); } catch (err) { - expect(err.message.toLowerCase().includes('permission')); - expect(err.message.includes(MOVE_FEATURE_TOGGLE)); + expect(err.message.toLowerCase().includes('permission')).toBeTruthy(); + expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy(); } }); @@ -604,8 +604,8 @@ test('should fail if user is not authorized', async () => { project.id, ); } catch (err) { - expect(err.message.toLowerCase().includes('permission')); - expect(err.message.includes(MOVE_FEATURE_TOGGLE)); + expect(err.message.toLowerCase().includes('permission')).toBeTruthy(); + expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy(); } });