Skip to content

Commit

Permalink
fix: provide better error message when unknown feature flags are pres…
Browse files Browse the repository at this point in the history
…ent (#6114)

* fix: provide better error message when unknown feature flags are present

* chore: add extra test case
  • Loading branch information
Attila Hajdrik committed Dec 11, 2020
1 parent af0ced4 commit d452e83
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 13 deletions.
25 changes: 25 additions & 0 deletions packages/amplify-cli-core/src/__tests__/featureFlags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,31 @@ describe('feature flags', () => {
}).rejects.toThrowError(`Invalid number value: 'invalid' for 'transformerversion' in section 'graphqltransformer'`);
});

test('initialize feature flag provider fail unknown flags', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
return {
envName: 'dev',
};
},
};

const envProvider: CLIEnvironmentProvider = new CLIContextEnvironmentProvider(context);
const projectPath = path.join(__dirname, 'testFiles', 'testProject-initialize-unknown-flag');

// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toMatchObject({
message: 'Invalid feature flag configuration',
name: 'JSONValidationError',
unknownFlags: ['graphqltransformer.foo', 'graphqltransformer.bar'],
otherErrors: ['graphqltransformer.transformerversion: should be number'],
});
});

const getTestFlags = (): Record<string, FeatureFlagRegistration[]> => {
return {
graphQLTransformer: [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"usageTracking": {
"enabled": true // Usage tracking is helping the CLI team
},
"features": {
"graphQLTransformer": {
"transformerVersion": "it-is-a-string",
"foo": "unknown",
"bar": "unknown"
}
}
}
33 changes: 28 additions & 5 deletions packages/amplify-cli-core/src/feature-flags/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,18 +307,41 @@ export class FeatureFlags {

private validateFlags = (allFlags: { name: string; flags: FeatureFlagConfiguration }[]): void => {
const schema = this.buildJSONSchemaFromRegistrations();
const ajv = new Ajv();
const ajv = new Ajv({
allErrors: true,
});
const schemaValidate = ajv.compile(schema);

const validator = (target: string, flags: FeatureFlagsEntry) => {
const valid = schemaValidate(flags);

if (!valid && schemaValidate.errors) {
const jsonError = schemaValidate.errors[0];
const additionalProperty = (<AdditionalPropertiesParams>jsonError?.params)?.additionalProperty;
const propertyMessage = additionalProperty ? `: '${additionalProperty}'` : '';
const unknownFlags = [];
const otherErrors = [];

for (const error of schemaValidate.errors) {
if (error.keyword === 'additionalProperties') {
const additionalProperty = (<AdditionalPropertiesParams>error.params)?.additionalProperty;
let flagName = error.dataPath.length > 0 && error.dataPath[0] === '.' ? `${error.dataPath.slice(1)}.` : '';

if (additionalProperty) {
flagName += additionalProperty;
}

if (flagName.length > 0) {
unknownFlags.push(flagName);
}
} else {
const errorMessage =
error.dataPath.length > 0 && error.dataPath[0] === '.'
? `${error.dataPath.slice(1)}: ${error.message}`
: `${error.dataPath}: ${error.message}`;

otherErrors.push(errorMessage);
}
}

throw new JSONValidationError(`${target}: ${ajv.errorsText(schemaValidate.errors)}${propertyMessage}`);
throw new JSONValidationError('Invalid feature flag configuration', unknownFlags, otherErrors);
}
};

Expand Down
2 changes: 1 addition & 1 deletion packages/amplify-cli-core/src/jsonValidationError.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export class JSONValidationError extends Error {
constructor(message: string) {
constructor(message: string, public unknownFlags: string[], public otherErrors: string[]) {
super(message);

this.name = 'JSONValidationError';
Expand Down
64 changes: 57 additions & 7 deletions packages/amplify-cli/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import { isCI } from 'ci-info';
import {
$TSContext,
CLIContextEnvironmentProvider,
Expand All @@ -8,6 +9,7 @@ import {
stateManager,
exitOnNextTick,
TeamProviderInfoMigrateError,
JSONValidationError,
} from 'amplify-cli-core';
import { Input } from './domain/input';
import { getPluginPlatform, scan } from './plugin-manager';
Expand All @@ -28,6 +30,7 @@ import { ensureMobileHubCommandCompatibility } from './utils/mobilehub-support';
import { migrateTeamProviderInfo } from './utils/team-provider-migrate';
import { deleteOldVersion } from './utils/win-utils';
import { logInput } from './conditional-local-logging-init';

EventEmitter.defaultMaxListeners = 1000;

// entry from commandline
Expand Down Expand Up @@ -116,14 +119,61 @@ export async function run() {
}

return exitCode;
} catch (e) {
} catch (error) {
// ToDo: add logging to the core, and log execution errors using the unified core logging.
errorHandler(e);
if (e.message) {
print.error(e.message);
}
if (e.stack) {
print.info(e.stack);
errorHandler(error);

if (error.name === 'JSONValidationError') {
const jsonError = <JSONValidationError>error;
let printSummary = false;

print.error(error.message);

if (jsonError.unknownFlags?.length > 0) {
print.error('');
print.error(
`These feature flags are defined in the "amplify/cli.json" configuration file and are unknown to the currently running Amplify CLI:`,
);

for (const unknownFlag of jsonError.unknownFlags) {
print.error(` - ${unknownFlag}`);
}

printSummary = true;
}

if (jsonError.otherErrors?.length > 0) {
print.error('');
print.error(`The following feature flags have validation errors:`);

for (const otherError of jsonError.otherErrors) {
print.error(` - ${otherError}`);
}

printSummary = true;
}

if (printSummary) {
print.error('');
print.error(
`This issue likely happens when the project has been pushed with a newer version of Amplify CLI, try updating to a newer version.`,
);

if (isCI) {
print.error('');
print.error(`Ensure that the CI/CD pipeline is not using an older or pinned down version of Amplify CLI.`);
}

print.error('');
print.error(`Learn more about feature flags: https://docs.amplify.aws/cli/reference/feature-flags`);
}
} else {
if (error.message) {
print.error(error.message);
}
if (error.stack) {
print.info(error.stack);
}
}
exitOnNextTick(1);
}
Expand Down

0 comments on commit d452e83

Please sign in to comment.