Skip to content
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

fix: Fix actionCreator/organization promises and enable no-async-promise-executor eslint rule #83270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Jan 10, 2025

https://eslint.org/docs/latest/rules/no-async-promise-executor#rule-details

This rule uncovered a case where an error inside of loadOrganization() could result in a stalled sudo auth page, and where loadTeamsAndProjects() would swallow errors.

loadOrganization
Before: if a 401 or 403 response was returned and getErrorMessage() couldn't read .detail or .detail.message then we'd arrive at the lonely return; statement on line 178 without resolving or rejecting the parent promise. This would stall the promise forever.

Now: the promise always resolves or rejects, one way or another.

This was only a problem inside of:

All other callsites do no await the promise.

loadTeamsAndProjects

The promise executor inside loadTeamsAndProjects had no try/catch blocks before. So any errors thrown from inside any of the function calls (including fetchProjectsAndTeams) would not be caught or re-thown by the new Promise class instance. They're just gone, and the promise returned by loadTeamsAndProjects() would never reject.
This caused the same issue as loadOrganization() above and impacts the same callsites. Now the method always resolves or rejects.

@ryan953 ryan953 requested a review from a team as a code owner January 10, 2025 21:47
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 10, 2025
@ryan953 ryan953 requested a review from a team January 10, 2025 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant