Skip to content

Fix unhandled promise rejection in utils.parallelForeach #16362

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

Merged
merged 9 commits into from
Jun 17, 2025

Conversation

samwho
Copy link
Collaborator

@samwho samwho commented Jun 16, 2025

The original implementation had a critical bug where Promise.race() was called without proper error handling, causing unhandled promise rejections when parallel tasks failed. Unhandled promises cause container restarts in prod.

The original implementation had a critical bug where Promise.race() was called
without proper error handling, causing unhandled promise rejections when
parallel tasks failed. This was particularly problematic in GoogleSheets
integration where getRows() failures would become unhandled rejections.

Changes:
- Completely rewrote parallelForeach with proper error handling
- Added comprehensive tests to verify no unhandled promise rejections
- Fixed concurrency control logic and promise cleanup
- All existing functionality preserved while eliminating the race condition

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link

qa-wolf bot commented Jun 16, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/m labels Jun 16, 2025
samwho and others added 4 commits June 16, 2025 17:36
Rewrote parallelForeach with a simpler, more robust worker-based approach:
- Eliminated complex Promise.race logic that was causing unhandled rejections
- Uses worker functions that process items sequentially from shared index
- Proper error propagation through Promise.all
- All tests now pass with 0 unhandled promise rejections
- Cleaned up test structure and removed old test file

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@samwho samwho marked this pull request as ready for review June 16, 2025 16:40
@samwho samwho enabled auto-merge June 17, 2025 11:39
@samwho samwho merged commit 83d3462 into master Jun 17, 2025
22 checks passed
@samwho samwho deleted the fix-unhandled-promise-rejection-parallel-foreach branch June 17, 2025 11:49
@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants