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

🔨 set up eslint rule to error on dangling promises + fix 245 violations #3358

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

danyx23
Copy link
Contributor

@danyx23 danyx23 commented Mar 16, 2024

With the introduction of a more rigorous use of transactions, dangling promises have become more problematic. It used to the be the case that if a promise was not awaited in an api route handler, node would just wait for it to complete. But since we now open transaction scopes in the route handlers, it can happen that a the router handler finishes and closes a transaction and then the dangling promise continues and tries to access the db via that transaction which then leads to an error and crashes the admin.

This PR enables an ESLint rule that makes unhandled promises an error. You can either await the result, or, for cases where you genuinely do not need to await the promise, mark the promise as intentionally not awaited by prefixing it with the JS void operator.

This PR then fixes all the ~250 places that this eslint rule complained about

@danyx23 danyx23 marked this pull request as ready for review March 16, 2024 13:38
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch 2 times, most recently from b4aa5dc to 5a640e2 Compare March 16, 2024 14:00
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch 2 times, most recently from 0366771 to 34fca5b Compare March 16, 2024 17:31
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch from f078be0 to 78bb8aa Compare March 18, 2024 18:59
@danyx23 danyx23 requested a review from ikesau March 18, 2024 19:00
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch from 78bb8aa to 29b9ac2 Compare March 19, 2024 16:02
@danyx23 danyx23 changed the base branch from db-migrate-gdocs to db-fix-sql-query March 20, 2024 10:05
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch from 29b9ac2 to b8f843c Compare March 20, 2024 10:05
Base automatically changed from db-fix-sql-query to db-migrate-gdocs March 20, 2024 10:50
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch from b8f843c to 849b260 Compare March 20, 2024 20:31
Copy link
Contributor Author

danyx23 commented Mar 26, 2024

Merge activity

  • Mar 26, 10:04 AM EDT: @danyx23 started a stack merge that includes this pull request via Graphite.
  • Mar 26, 10:19 AM EDT: Graphite rebased this pull request as part of a merge.
  • Mar 26, 10:22 AM EDT: @danyx23 merged this pull request with Graphite.

Base automatically changed from db-migrate-gdocs to master March 26, 2024 14:18
@danyx23 danyx23 force-pushed the eslint-no-floating-promises branch from 95652fc to acbfb58 Compare March 26, 2024 14:19
@danyx23 danyx23 merged commit 2b471a6 into master Mar 26, 2024
17 of 20 checks passed
@danyx23 danyx23 deleted the eslint-no-floating-promises branch March 26, 2024 14:22
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