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

fix: prevent infinite loop #8540

Merged
merged 1 commit into from Oct 26, 2021
Merged

fix: prevent infinite loop #8540

merged 1 commit into from Oct 26, 2021

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Oct 25, 2021

Description of changes

The loop in getResourcesToBeCreated() pushes items into the array that it is iterating over. However, there was no check for cycles, meaning that circular dependencies pushes resources into the array until an out of memory error occurs. This commit adds a check to prevent adding the same item to the array more than once.

Issue #, if available

Closes #8517

Description of how you validated changes

Ran the failing command in the linked issue against the customer's project.

Checklist

  • PR description included
  • yarn test passes

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

The loop in getResourcesToBeCreated() pushes items into the
array that it is iterating over. However, there was no check
for cycles, meaning that circular dependencies pushes resources
into the array until an out of memory error occurs. This commit
adds a check to prevent adding the same item to the array more
than once.
@codecov-commenter
Copy link

Codecov Report

Merging #8540 (4cbc922) into master (bd435e2) will increase coverage by 1.22%.
The diff coverage is 75.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8540      +/-   ##
==========================================
+ Coverage   55.92%   57.14%   +1.22%     
==========================================
  Files         695      724      +29     
  Lines       38881    40846    +1965     
  Branches     7871     8361     +490     
==========================================
+ Hits        21743    23343    +1600     
- Misses      16343    16693     +350     
- Partials      795      810      +15     
Impacted Files Coverage Δ
...egory-api/src/provider-utils/supported-services.ts 100.00% <ø> (ø)
packages/amplify-category-auth/src/index.js 15.74% <0.00%> (ø)
...auth/src/provider-utils/awscloudformation/index.js 3.90% <0.00%> (ø)
...s/amplify-category-geo/src/commands/geo/console.ts 53.84% <0.00%> (+3.84%) ⬆️
...amplify-cli-core/src/feature-flags/featureFlags.ts 83.62% <ø> (ø)
packages/amplify-cli/src/domain/amplify-toolkit.ts 2.39% <0.00%> (+<0.01%) ⬆️
...index-transformer/src/graphql-index-transformer.ts 100.00% <ø> (ø)
...amplify-graphql-index-transformer/src/resolvers.ts 99.57% <ø> (+<0.01%) ⬆️
...es/amplify-graphql-index-transformer/src/schema.ts 98.19% <ø> (+0.05%) ⬆️
...transformer/src/graphql-predictions-transformer.ts 97.80% <ø> (-1.62%) ⬇️
... and 86 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 59117c2...4cbc922. Read the comment docs.

@ammarkarachi ammarkarachi merged commit aeaceab into aws-amplify:master Oct 26, 2021
@cjihrig cjihrig deleted the breaker branch November 4, 2021 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FATAL ERROR: invalid array length Allocation failed - JavaScript heap out of memory
3 participants