Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove unnecessary stack trace log when adding services #4610

Merged

Conversation

sebastiancrossa
Copy link
Contributor

Issue #, if available:
#4283

Description of changes:
The unnecessary stack traces that were displayed through the console when a user tried to run amplify add without initializing amplify first was because of a lacking try/catch block which displays only the error message, so I added that onto the parts where the addition of services was being called.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #4610 into master will decrease coverage by 0.02%.
The diff coverage is 76.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4610      +/-   ##
==========================================
- Coverage   61.57%   61.55%   -0.03%     
==========================================
  Files         333      333              
  Lines       14550    14558       +8     
  Branches     2764     2932     +168     
==========================================
+ Hits         8959     8961       +2     
+ Misses       5164     5152      -12     
- Partials      427      445      +18     
Impacted Files Coverage Δ
...ejs-function-runtime-provider/src/utils/execute.ts 75.47% <50.00%> (-0.46%) ⬇️
...aphql-auth-transformer/src/ModelAuthTransformer.ts 87.96% <50.00%> (+0.10%) ⬆️
...-model-plugin/src/visitors/appsync-java-visitor.ts 90.75% <100.00%> (+0.16%) ⬆️
packages/graphql-transformer-core/src/index.ts 96.77% <100.00%> (-0.20%) ⬇️
...ackages/graphql-transformer-core/src/validation.ts 97.67% <100.00%> (ø)
packages/graphql-mapping-template/src/print.ts 35.71% <0.00%> (ø)
packages/amplify-util-mock/src/storage/storage.ts 0.00% <0.00%> (ø)
...ges/amplify-util-mock/src/CFNParser/stack/index.ts 88.09% <0.00%> (ø)
...ges/graphql-transformer-core/src/util/fileUtils.ts 11.76% <0.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c04c28...a5b5220. Read the comment docs.

@ammarkarachi ammarkarachi self-assigned this Jun 22, 2020
@@ -20,7 +20,7 @@ module.exports = {
context.print.success(`Successfully updated resource ${resourceName} locally`);
})
.catch(err => {
context.print.info(err.stack);
context.print.error(err.message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to swallow errors on update there is a possibility of users losing information of their errors on update of a valid project

);
print.info('');
})
.catch(err => {
context.print.info(err.stack);
context.print.error(err.message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, we still need the stack trace in case of other errors.

@ammarkarachi
Copy link
Contributor

@sebastiancrossa
It's great that you have written tests for the amplify execute command but the way to go about it would be to have the smallest impact to change the current functionality.

If the project is not an amplify project any other command that does not initialize, configure or pull an existing project should result in an error/information without a stack trace. Refer to docs for more information.
amplify pull
amplify configure
amplify init

@ammarkarachi ammarkarachi merged commit 5bee574 into aws-amplify:master Jul 9, 2020
ammarkarachi added a commit that referenced this pull request Jul 10, 2020
ammarkarachi pushed a commit to ammarkarachi/amplify-cli that referenced this pull request Jul 10, 2020
…ices (aws-amplify#4610)"

This reverts commit 5bee574.

Revert "feat: add check for extra command line args provided with amplify delete (aws-amplify#4576)"

This reverts commit 82d1093.

test(amplify-category-auth): fixed push test
UnleashedMind pushed a commit to UnleashedMind/amplify-cli that referenced this pull request Jul 10, 2020
ammarkarachi pushed a commit that referenced this pull request Jul 10, 2020
* feat(amplify-provider-awscloudformation): add region ca-central-1

* fix test

* Revert "Add region ca-central-1 (#4796)"

This reverts commit c8b6bd8.

* Revert "feat: add check for extra command line args provided with amplify delete (#4576)"

This reverts commit 82d1093.

* Revert "perf: fulfill promises to upload files to S3 concurrently (#4575)"

This reverts commit 96d1914.

* Revert "fix(cli): remove unnecessary stack trace log when adding services (#4610)"

This reverts commit 5bee574.

* Revert "fix test"

This reverts commit 63c3c78.

Co-authored-by: UnleashedMind <zhoweimi@186590ce137f.ant.amazon.com>
ammarkarachi added a commit that referenced this pull request Jul 27, 2020
nikhname pushed a commit to nikhname/amplify-cli that referenced this pull request Jul 27, 2020
nikhname pushed a commit to nikhname/amplify-cli that referenced this pull request Jul 27, 2020
* feat(amplify-provider-awscloudformation): add region ca-central-1

* fix test

* Revert "Add region ca-central-1 (aws-amplify#4796)"

This reverts commit af29f35.

* Revert "feat: add check for extra command line args provided with amplify delete (aws-amplify#4576)"

This reverts commit 48b27b7.

* Revert "perf: fulfill promises to upload files to S3 concurrently (aws-amplify#4575)"

This reverts commit 9fbee8a.

* Revert "fix(cli): remove unnecessary stack trace log when adding services (aws-amplify#4610)"

This reverts commit 56efb32.

* Revert "fix test"

This reverts commit 50a8073fbbdb56931191912938fb50f542197832.

Co-authored-by: UnleashedMind <zhoweimi@186590ce137f.ant.amazon.com>
sebastiancrossa added a commit to MLH-Fellowship/amplify-cli that referenced this pull request Aug 4, 2020
sebastiancrossa pushed a commit to MLH-Fellowship/amplify-cli that referenced this pull request Aug 4, 2020
* feat(amplify-provider-awscloudformation): add region ca-central-1

* fix test

* Revert "Add region ca-central-1 (aws-amplify#4796)"

This reverts commit af29f35.

* Revert "feat: add check for extra command line args provided with amplify delete (aws-amplify#4576)"

This reverts commit 48b27b7.

* Revert "perf: fulfill promises to upload files to S3 concurrently (aws-amplify#4575)"

This reverts commit 9fbee8a.

* Revert "fix(cli): remove unnecessary stack trace log when adding services (aws-amplify#4610)"

This reverts commit 56efb32.

* Revert "fix test"

This reverts commit 50a8073fbbdb56931191912938fb50f542197832.

Co-authored-by: UnleashedMind <zhoweimi@186590ce137f.ant.amazon.com>
sebastiancrossa added a commit to MLH-Fellowship/amplify-cli that referenced this pull request Aug 25, 2020
sebastiancrossa pushed a commit to MLH-Fellowship/amplify-cli that referenced this pull request Aug 25, 2020
* feat(amplify-provider-awscloudformation): add region ca-central-1

* fix test

* Revert "Add region ca-central-1 (aws-amplify#4796)"

This reverts commit af29f35.

* Revert "feat: add check for extra command line args provided with amplify delete (aws-amplify#4576)"

This reverts commit 48b27b7.

* Revert "perf: fulfill promises to upload files to S3 concurrently (aws-amplify#4575)"

This reverts commit 9fbee8a.

* Revert "fix(cli): remove unnecessary stack trace log when adding services (aws-amplify#4610)"

This reverts commit 56efb32.

* Revert "fix test"

This reverts commit 50a8073fbbdb56931191912938fb50f542197832.

Co-authored-by: UnleashedMind <zhoweimi@186590ce137f.ant.amazon.com>
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants