Skip to content

fix(router): queue rejections in firstValueFrom to avoid unhandled rejections#66542

Open
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/router-unhandled
Open

fix(router): queue rejections in firstValueFrom to avoid unhandled rejections#66542
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/router-unhandled

Conversation

@arturovt
Copy link
Contributor

When an Observable passed to firstValueFrom() errors synchronously (e.g., canMatch guard completes without emitting), the Promise rejects before the caller can attach a .catch() handler. Zone.js detects this as an unhandled rejection and re-throws the error, even though it's properly handled in processSegment's try-catch.

Use queueMicrotask() to defer the rejection, ensuring the catch handler is attached before Zone.js performs its unhandled rejection check.

…jections

When an Observable passed to firstValueFrom() errors synchronously (e.g.,
canMatch guard completes without emitting), the Promise rejects before the
caller can attach a .catch() handler. Zone.js detects this as an unhandled
rejection and re-throws the error, even though it's properly handled in
processSegment's try-catch.

Use queueMicrotask() to defer the rejection, ensuring the catch handler is
attached before Zone.js performs its unhandled rejection check.
@ngbot ngbot bot added this to the Backlog milestone Jan 14, 2026
@arturovt
Copy link
Contributor Author

@atscott as discussed, just leaving a draft if we could consider accept it as a potential fix.

@atscott
Copy link
Contributor

atscott commented Jan 14, 2026

As I mentioned, I don't quite follow the reasoning here. I can't reproduce it in the Router or in a simpler reproduction that "synchronously" returns a Promise.reject (https://stackblitz.com/edit/stackblitz-starters-7u8npqtn). Until we have these answers, I don't think we can move forward with this change.

@atscott atscott closed this Jan 14, 2026
@atscott atscott reopened this Jan 14, 2026
@atscott atscott added the requires: TGP This PR requires a passing TGP before merging is allowed label Jan 14, 2026
@atscott atscott marked this pull request as ready for review January 14, 2026 22:55
@pullapprove pullapprove bot requested a review from atscott January 14, 2026 22:55
@pullapprove pullapprove bot removed the requires: TGP This PR requires a passing TGP before merging is allowed label Jan 14, 2026
@JeanMeche JeanMeche added memory leak Issue related to a memory leak target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker and removed memory leak Issue related to a memory leak labels Jan 19, 2026
@JeanMeche JeanMeche removed the request for review from atscott January 19, 2026 12:30
@atscott atscott added state: blocked and removed action: merge The PR is ready for merge by the caretaker labels Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: router state: blocked target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants