[backport v2.14.1] Guard against unhandled exceptions when using waitFor()#17086
Merged
Conversation
This helps to prevent permanently hung promises by always rejecting regardless whether a message was passed to `waitFor()`. Switching to a `throw` when a message is omitted subverts expectations and is prone to error if a developer does not anticipate this change in behavior. Relying on reject is more predictable and will properly propagate through the Promise chain. Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
This adds some guards to resolve issues with `waitForSettingsSchema` and `waitForSettings` where unhandled exceptions can cause the promise chain to hang. When in this state, promises are permanently unresolved and users are unable to interact with certain links throughout Dashboard. - Add msg parameters to `waitForSettingsSchema` and `waitForSettings` so that timeouts will reject with a clear message - Wrap both calls to `waitForSettingsSchema` and `waitForSettings` in a try..catch block so that the worker can be cleaned up instead of risking blocking the caller - Return early instead of throwing a null reference error on `postMessage` so that `allHashSettled` can safely settle Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
codyrancher
approved these changes
Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated request to port PR #17034 by @rak-phillip
fixes #16791
Original PR body:
Summary
This change resolves issues surrounding permanently hung promises when unhandled exceptions are thrown by
waitFor(). This change aims to reduce the number of scenarios where developers can inadvertently cause promises to hang(cede2d3) while also adding some additional safety nets in the event that an unhandled exception is raised(21e08935b346c71d2e2b6f00274add8b1812c490)Original text redacted by port-pr.yaml
Occurred changes and/or fixed issues
waitFor()waitFor()Technical notes summary
Permanently hung promises are prevented by always rejecting regardless whether a message was passed to
waitFor(). Switching to athrowwhen a message is omitted subverts expectations and is prone to error if a developer does not anticipate this change in behavior. Relying on reject is more predictable and will properly propagate through the Promise chain.This also adds some guards to resolve issues with
waitForSettingsSchemaandwaitForSettingswhere unhandled exceptions can cause the promise chain to hang. When in this state, promises are permanently unresolved and users are unable to interact with certain links throughout Dashboard.waitForSettingsSchemaandwaitForSettingsso that timeouts will reject with a clear messagewaitForSettingsSchemaandwaitForSettingsin a try..catch block so that the worker can be cleaned up instead of risking blocking the callerpostMessageso thatallHashSettledcan safely settleAreas or cases that should be tested
The existing behavior can be tested more quickly by setting
timeoutMsto some incredibly low value:timeoutMs = 50000https://github.com/rak-phillip/dashboard/blob/f0c2fb0089fab197bb24d3479f638edf7af2968f/shell/utils/async.ts#L1
I also set several auth ttl values to low values so that we can speed up the automatic logout behavior:
In order to assert that this change works. First, set up your environment so that you can easily repro in master. Second, checkout this change with the same repro previously performed. We can expect two behaviors:
Areas which could experience regressions
I think there are some risks in modifying the behavior of our websocket connections. Most notably, I can be incorrect about my assumptions in how to properly clean up workers in 21e08935b346c71d2e2b6f00274add8b1812c490.
Screenshot/Video
NA - the demonstrable change is of hard to prove with video/screenshots. I highly recommend a repro to assert that the issue is resolved.
Checklist
Admin,Standard UserandUser Base