Skip to content

Commit

Permalink
fix(cli): remove unnecessary stack trace log when adding services (#4610
Browse files Browse the repository at this point in the history
)
  • Loading branch information
sebastiancrossa committed Jul 9, 2020
1 parent a3feb15 commit 56efb32
Show file tree
Hide file tree
Showing 13 changed files with 104 additions and 17 deletions.
1 change: 0 additions & 1 deletion packages/amplify-category-analytics/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/amplify-category-api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async function migrate(context, serviceName) {
if (providerController) {
if (!serviceName || serviceName === amplifyMeta[category][resourceName].service) {
migrateResourcePromises.push(
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName)
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName),
);
}
}
Expand Down Expand Up @@ -149,7 +149,7 @@ async function getPermissionPolicies(context, resourceOpsMapping) {
context,
amplifyMeta[category][resourceName].service,
resourceName,
resourceOpsMapping[resourceName]
resourceOpsMapping[resourceName],
);
permissionPolicies.push(policy);
resourceAttributes.push({ resourceName, attributes, category });
Expand All @@ -171,7 +171,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
1 change: 0 additions & 1 deletion packages/amplify-category-auth/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
1 change: 0 additions & 1 deletion packages/amplify-category-function/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/amplify-category-interactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ async function migrate(context) {
const providerController = require(`./provider-utils/${amplifyMeta[category][resourceName].providerPlugin}/index`);
if (providerController) {
migrateResourcePromises.push(
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName)
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName),
);
} else {
context.print.error(`Provider not configured for ${category}: ${resourceName}`);
Expand Down Expand Up @@ -42,7 +42,7 @@ async function getPermissionPolicies(context, resourceOpsMapping) {
context,
amplifyMeta[category][resourceName].service,
resourceName,
resourceOpsMapping[resourceName]
resourceOpsMapping[resourceName],
);
permissionPolicies.push(policy);
resourceAttributes.push({ resourceName, attributes, category });
Expand All @@ -64,7 +64,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
1 change: 0 additions & 1 deletion packages/amplify-category-notifications/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module.exports = {
}
})
.catch(err => {
context.print.info(err.stack);
context.print.error(err.message);
context.print.error('An error occurred when removing the predictions resource');
context.usageData.emitError(err);
});
Expand Down
1 change: 0 additions & 1 deletion packages/amplify-category-predictions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/amplify-category-storage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async function migrate(context) {
const providerController = require(`./provider-utils/${amplifyMeta[category][resourceName].providerPlugin}/index`);
if (providerController) {
migrateResourcePromises.push(
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName)
providerController.migrateResource(context, projectPath, amplifyMeta[category][resourceName].service, resourceName),
);
} else {
context.print.error(`Provider not configured for ${category}: ${resourceName}`);
Expand Down Expand Up @@ -68,7 +68,7 @@ async function getPermissionPolicies(context, resourceOpsMapping) {
context,
service,
resourceName,
resourceOpsMapping[resourceName]
resourceOpsMapping[resourceName],
);
permissionPolicies.push(policy);
resourceAttributes.push({ resourceName, attributes, category });
Expand All @@ -90,7 +90,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
1 change: 0 additions & 1 deletion packages/amplify-category-xr/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ async function executeAmplifyCommand(context) {
} else {
commandPath = path.join(commandPath, category, context.input.command);
}

const commandModule = require(commandPath);
await commandModule.run(context);
}
Expand Down
46 changes: 46 additions & 0 deletions packages/amplify-cli/src/__tests__/executeAmplifyCommand.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import fs from 'fs-extra';
import { executeAmplifyCommandForPlugin } from '../../lib/execution-manager';

describe('executeAmplifyCommand: ', () => {
const mockExecuteAmplifyCommand = jest.fn();
const mockContext = {
print: {
info: jest.fn(),
},
};

const mockPluginModule = {
executeAmplifyCommand: mockExecuteAmplifyCommand,
};

describe('case: executeAmplifyCommand is run on a directory where amplify is not initialized', () => {
const err = new Error(
"You are not working inside a valid amplify project.\nUse 'amplify init' in the root of your app directory to initialize your project with Amplify",
);

beforeEach(() => {
mockExecuteAmplifyCommand.mockReturnValue(err);
(fs.existsSync as any).mockReturnValue(true);
});

it('executeAmplifyCommand should fail to add a service and print an error message', async () => {
await (executeAmplifyCommandForPlugin as any)(mockContext, mockPluginModule);
expect(mockExecuteAmplifyCommand).toReturnWith(err);
});
});

describe('case: executeAmplifyCommand returns a stack trace error', () => {
const err = new Error('An unexpected error has occurred');

beforeEach(() => {
mockExecuteAmplifyCommand.mockReturnValue(err.stack);
(fs.existsSync as any).mockReturnValue(true);
});

it('executeAmplifyCommand should get an unexpected error and print a stack trace', async () => {
await (executeAmplifyCommandForPlugin as any)(mockContext, mockPluginModule);

expect(mockExecuteAmplifyCommand).toReturnWith(err.stack);
});
});
});
18 changes: 17 additions & 1 deletion packages/amplify-cli/src/execution-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
AmplifyPostPullEventData,
} from './domain/amplify-event';

const ERROR_NOT_AMPLIFY_PROJECT =
"You are not working inside a valid amplify project.\nUse 'amplify init' in the root of your app directory to initialize your project with Amplify";

export async function executeCommand(context: Context) {
const pluginCandidates = getPluginsWithNameAndCommand(context.pluginPlatform, context.input.plugin!, context.input.command!);

Expand Down Expand Up @@ -140,6 +143,18 @@ async function selectPluginForExecution(context: Context, pluginCandidates: Plug
return result;
}

export async function executeAmplifyCommandForPlugin(context: Context, plugin: any) {
try {
await plugin.executeAmplifyCommand(context);
} catch (err) {
if (err.message === ERROR_NOT_AMPLIFY_PROJECT) {
context.print.error(err.message);
} else {
context.print.info(err.stack);
}
}
}

async function executePluginModuleCommand(context: Context, plugin: PluginInfo) {
const { commands, commandAliases } = plugin.manifest;
if (!commands!.includes(context.input.command!)) {
Expand All @@ -155,7 +170,8 @@ async function executePluginModuleCommand(context: Context, plugin: PluginInfo)
typeof pluginModule[constants.ExecuteAmplifyCommand] === 'function'
) {
attachContextExtensions(context, plugin);
await pluginModule.executeAmplifyCommand(context);

executeAmplifyCommandForPlugin(context, pluginModule);
} else {
// if the module does not have the executeAmplifyCommand method,
// fall back to the old approach by scanning the command folder and locate the command file
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const plugin_manager = require('../../lib/plugin-manager');

describe('executeAmplifyCommand: ', () => {
const mockExecuteAmplifyCommand = jest.fn();
const mockPluginCandidates = [];

const mockPluginModule = {
executeAmplifyCommand: mockExecuteAmplifyCommand,
};

const mockContext = {
print: {
info: jest.fn(),
error: jest.fn(),
},
};

it('executeAmplifyCommand method should exist', () => {
expect(mockPluginModule.executeAmplifyCommand).toBeDefined();
});

describe('case: executeAmplifyCommand is run on a directory where amplify has not been initialized', () => {
beforeEach(() => {
mockExecuteAmplifyCommand.mockReturnValue(undefined);
});

it('executeAmplifyCommand should fail to add a service and print an error message', async () => {
await mockPluginModule.executeAmplifyCommand(mockContext);
expect(mockContext.print.error).toBeCalledWith(
"You are not working inside a valid amplify project.\nUse 'amplify init' in the root of your app directory to initialize your project with Amplify",
);
});
});
});

0 comments on commit 56efb32

Please sign in to comment.