Skip to content

fix(core): prevent partial resource cleanup when a shutdown hook rejects#17040

Open
asn6878 wants to merge 2 commits into
nestjs:v12.0.0from
asn6878:fix/cleanup-hooks-use-promise-allsettled
Open

fix(core): prevent partial resource cleanup when a shutdown hook rejects#17040
asn6878 wants to merge 2 commits into
nestjs:v12.0.0from
asn6878:fix/cleanup-hooks-use-promise-allsettled

Conversation

@asn6878
Copy link
Copy Markdown

@asn6878 asn6878 commented May 27, 2026

PR Checklist

PR Type

  • Bugfix

What is the current behavior?

Closes #17039.

When a shutdown lifecycle hook (onModuleDestroy, beforeApplicationShutdown, onApplicationShutdown) rejects, Promise.all fails fast and all remaining provider cleanup in the same module — as well as all subsequent modules — is silently skipped.

What is the new behavior?

Replace Promise.all with Promise.allSettled in the three cleanup hook functions. All providers now run their cleanup regardless of sibling failures. Rejected hooks are logged via Logger.error with stack trace. Startup hooks (onModuleInit, onApplicationBootstrap) are unchanged — fail-fast on startup remains intentional.

Does this PR introduce a breaking change?

  • Yes
  • No

@kamilmysliwiec
Copy link
Copy Markdown
Member

Could you please target v12.0.0?

@asn6878
Copy link
Copy Markdown
Author

asn6878 commented May 28, 2026

@kamilmysliwiec I’ll handle the conflicts later today.

Prevents a failing provider from skipping the remaining cleanup sequence.

Closes nestjs#17039
@asn6878 asn6878 force-pushed the fix/cleanup-hooks-use-promise-allsettled branch from ca7d12b to 0af8c0f Compare May 28, 2026 13:19
@asn6878
Copy link
Copy Markdown
Author

asn6878 commented May 28, 2026

Rebased on v12.0.0 and adapted to the new structure and test infrastructure.

The issue was filed against v11 but the same Promise.all fail-fast behavior is still there in v12.0.0's hierarchy-level for loop, so this fix applies here too.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a shutdown reliability bug in @nestjs/core where rejected shutdown lifecycle hooks could cause fail-fast behavior and skip remaining cleanup work across providers/modules.

Changes:

  • Switch shutdown hook execution from Promise.all to Promise.allSettled for onModuleDestroy, beforeApplicationShutdown, and onApplicationShutdown.
  • Log rejected hook errors during shutdown instead of failing fast.
  • Add unit tests asserting non-throwing behavior, continued execution, and error logging for rejected provider hooks.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/core/hooks/on-module-destroy.hook.ts Uses Promise.allSettled for provider destroy hooks and logs rejections.
packages/core/hooks/on-app-shutdown.hook.ts Uses Promise.allSettled for provider shutdown hooks and logs rejections.
packages/core/hooks/before-app-shutdown.hook.ts Uses Promise.allSettled for provider pre-shutdown hooks and logs rejections.
packages/core/test/hooks/on-module-destroy.hook.spec.ts Adds tests for rejected provider destroy hooks (and suggested coverage for module hook rejection).
packages/core/test/hooks/on-app-shutdown.hook.spec.ts Adds tests for rejected provider shutdown hooks (and suggested coverage for module hook rejection).
packages/core/test/hooks/before-app-shutdown.hook.spec.ts Adds tests for rejected provider pre-shutdown hooks (and suggested coverage for module hook rejection).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/hooks/before-app-shutdown.hook.ts
Comment thread packages/core/hooks/on-module-destroy.hook.ts
Comment thread packages/core/hooks/on-app-shutdown.hook.ts
Comment thread packages/core/test/hooks/on-module-destroy.hook.spec.ts
Comment thread packages/core/test/hooks/on-app-shutdown.hook.spec.ts
Comment thread packages/core/test/hooks/before-app-shutdown.hook.spec.ts
wrap the module class lifecycle call in try/catch so a rejecting
module-level hook no longer aborts the remaining shutdown sequence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shutdown lifecycle hooks skip remaining providers when one throws (Promise.all fail-fast)

4 participants