-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(loop-foreach): forEach loop iterations #581
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
✅ No security or compliance issues detected. Reviewed everything up to 848ee0d. Security Overview
Detected Code Changes
Reply to this PR with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Fixed forEach loop iteration handling by implementing a more robust iteration limit system that respects input array sizes while maintaining safety limits.
- Modified
apps/sim/executor/loops.tsto increase the default safety limit to 5000 items for forEach loops while maintaining 5 iterations for regular loops - Added logic in
apps/sim/executor/handlers/loop-handler.tsto properly handle both array and object-type collections for iteration - Improved logging system for better debugging of loop iteration behavior
- Implemented proper iteration limit checks that honor explicitly configured limits while maintaining safety bounds
2 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
|
@aadamgough the upper limit for forEach should probably be the same as the upper limit for the regular for loop (100) to avoid confusion. |
dd2ccdd to
805b245
Compare
0a3f498 to
8a28768
Compare
* fix(loop-foreach): forEach loop iterations (#581) * fix: working on fixing foreach loop max * fix: removed default of 5 iterations * fix: bun run lint * fix: greptile comments (#581) * fix: removed safety max (#581) --------- Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> * improvement(sockets conn): sockets connection refresh warning (#580) * working conn status * improvement(sockets conn): sockets connection refresh warning * fix styling --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> * fix(variable resolution): use variable references to not have escaping issues (#587) * fix(variable-resolution): don't inject stringified json, use var refs * fix lint * remove unused var --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan> --------- Co-authored-by: Adam Gough <77861281+aadamgough@users.noreply.github.com> Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan>
* fix(loop-foreach): forEach loop iterations (#581) * fix: working on fixing foreach loop max * fix: removed default of 5 iterations * fix: bun run lint * fix: greptile comments (#581) * fix: removed safety max (#581) --------- Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> * improvement(sockets conn): sockets connection refresh warning (#580) * working conn status * improvement(sockets conn): sockets connection refresh warning * fix styling --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> * fix(variable resolution): use variable references to not have escaping issues (#587) * fix(variable-resolution): don't inject stringified json, use var refs * fix lint * remove unused var --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan> --------- Co-authored-by: Adam Gough <77861281+aadamgough@users.noreply.github.com> Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan>
* fix(loop-foreach): forEach loop iterations (simstudioai#581) * fix: working on fixing foreach loop max * fix: removed default of 5 iterations * fix: bun run lint * fix: greptile comments (simstudioai#581) * fix: removed safety max (simstudioai#581) --------- Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> * improvement(sockets conn): sockets connection refresh warning (simstudioai#580) * working conn status * improvement(sockets conn): sockets connection refresh warning * fix styling --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> * fix(variable resolution): use variable references to not have escaping issues (simstudioai#587) * fix(variable-resolution): don't inject stringified json, use var refs * fix lint * remove unused var --------- Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan> --------- Co-authored-by: Adam Gough <77861281+aadamgough@users.noreply.github.com> Co-authored-by: Adam Gough <adamgough@Mac.attlocal.net> Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-MacBook-Air.local> Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@vikhyaths-air.lan>
Description
For each loop was maxing out at default 5 iterations despite input array being larger.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added arrays larger than 5 items and confirmed they were running through all of the items.
Checklist:
bun run test)Security Considerations: