Skip to content

fix: prevent unhandled promise rejection in apps:destroy#3679

Merged
eablack merged 3 commits intomainfrom
eb/fix-apps-destroy-unhandled-promise
Apr 22, 2026
Merged

fix: prevent unhandled promise rejection in apps:destroy#3679
eablack merged 3 commits intomainfrom
eb/fix-apps-destroy-unhandled-promise

Conversation

@eablack
Copy link
Copy Markdown
Contributor

@eablack eablack commented Apr 22, 2026

Summary

Fixes unhandled promise rejection when removing git remotes after app destruction in apps:destroy. The command successfully deleted the app but crashed during git remote cleanup with exit code 1.

Root cause: Two bugs in the Promise.all usage:

  1. Nested array issue - .map() returns Array<Promise>, but Promise.all([array, array]) doesn't await inner promises, making them fire-and-forget
  2. Duplicate removal - listRemotes() returns same remote name twice (fetch + push), causing second rmRemote() to fail with "No such remote"

Solution:

  • Collect remote names first and deduplicate using Set
  • Then create promises array and await properly with Promise.all

Type of Change

Patch Updates (patch semver update)

  • fix: Bug fix

Testing

Notes:
This fix resolves the issue reported in #3677 where apps:destroy would crash after successfully deleting the app.

Steps::

git init test-destroy
cd test-destroy
git commit --allow-empty -m "init"
heroku create
heroku apps:destroy --confirm $(heroku info --json | jq -r '.app.name')
echo "Exit code: $?"  # Should be 0, not 1

Related Issues

GitHub issue: #3677

@eablack eablack requested a review from a team as a code owner April 22, 2026 15:42
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 15:42 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 15:42 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 15:42 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 15:42 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:18 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:18 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:18 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:18 — with GitHub Actions Inactive
eablack added 2 commits April 22, 2026 12:24
Fixes unhandled promise rejection when removing git remotes after
app destruction. The command successfully deleted the app but crashed
during git remote cleanup.

Two bugs fixed:

1. Promise.all nested array issue
   - .map() returns Array<Promise>, not a single Promise
   - Promise.all([array, array]) doesn't await inner promises
   - Resulted in fire-and-forget promises with unhandled rejections

2. Duplicate remote removal
   - listRemotes() groups by URL, same remote name appears twice
   - Once for (fetch) and once for (push)
   - Second rmRemote() call failed: "No such remote"

Solution:
- Flatten promise arrays by collecting remote names first
- Deduplicate names using Set (removes fetch/push duplicates)
- Then create promises and await them properly

Closes #3677
Expanded GitService class to include all git operations needed for
testing, allowing easy stubbing of git methods in unit tests.

Added two new tests:
- Verifies deduplication works with single remote (fetch + push)
- Verifies deduplication works with multiple remotes

Fixes #3677
@eablack eablack force-pushed the eb/fix-apps-destroy-unhandled-promise branch from 5a7ece8 to b69a518 Compare April 22, 2026 16:24
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:24 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:24 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:24 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:24 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@sbosio sbosio left a comment

Choose a reason for hiding this comment

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

Looks like it'll require small lint fixes, but otherwise looking great, so I'm approving!

@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 22, 2026 16:54 — with GitHub Actions Inactive
@eablack eablack merged commit 3e4c838 into main Apr 22, 2026
17 checks passed
@eablack eablack deleted the eb/fix-apps-destroy-unhandled-promise branch April 22, 2026 17:09
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.

2 participants