🔨 set up eslint rule to error on dangling promises + fix 245 violatio…

…ns (#3358) 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
owid · Mar 26, 2024 · 2b471a6 · 2b471a6
2 parents 395c86a + acbfb58
commit 2b471a6
Show file tree

Hide file tree

Showing 118 changed files with 274 additions and 261 deletions.
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
@@ -51,8 +51,7 @@ module.exports = {
         "react/no-render-return-value": "warn",
         "react/no-unescaped-entities": ["warn", { forbid: [">", "}"] }],
         "react/prop-types": "warn",
-        // TODO: consider adding this and whitelisting all promises that don't need to be awaited with "void"
-        // "@typescript-eslint/no-floating-promises": "error",
+        "@typescript-eslint/no-floating-promises": "error",
     },
     settings: {
         "import/resolver": {

diff --git a/adminSiteClient/Admin.tsx b/adminSiteClient/Admin.tsx
@@ -163,6 +163,7 @@ export class Admin {
     }
 
     @action.bound private removeRequest(request: Promise<Response>): void {
+        // eslint-disable-next-line @typescript-eslint/no-floating-promises
         this.currentRequests = this.currentRequests.filter(
             (req) => req !== request
         )

diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts
@@ -279,7 +279,7 @@ export class ChartEditor {
 
         if (window.confirm(`Publish chart at ${url}?`)) {
             this.grapher.isPublished = true
-            this.saveGrapher({
+            void this.saveGrapher({
                 onError: () => (this.grapher.isPublished = undefined),
             })
         }
@@ -292,7 +292,7 @@ export class ChartEditor {
                 : "Are you sure you want to unpublish this chart?"
         if (window.confirm(message)) {
             this.grapher.isPublished = undefined
-            this.saveGrapher({
+            void this.saveGrapher({
                 onError: () => (this.grapher.isPublished = true),
             })
         }

diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx
@@ -362,13 +362,13 @@ export class ChartEditorPage
     }
 
     @action.bound refresh(): void {
-        this.fetchGrapher()
-        this.fetchDetails()
-        this.fetchData()
-        this.fetchLogs()
-        this.fetchRefs()
-        this.fetchRedirects()
-        this.fetchPageviews()
+        void this.fetchGrapher()
+        void this.fetchDetails()
+        void this.fetchData()
+        void this.fetchLogs()
+        void this.fetchRefs()
+        void this.fetchRedirects()
+        void this.fetchPageviews()
 
         // (2024-02-15) Disabled due to slow query performance
         // https://github.com/owid/owid-grapher/issues/3198

diff --git a/adminSiteClient/ChartIndexPage.tsx b/adminSiteClient/ChartIndexPage.tsx
@@ -115,6 +115,6 @@ export class ChartIndexPage extends React.Component {
     }
 
     componentDidMount() {
-        this.getData()
+        void this.getData()
     }
 }
diff --git a/adminSiteClient/ChartList.tsx b/adminSiteClient/ChartList.tsx
@@ -95,7 +95,7 @@ export class ChartList extends React.Component<{
     }
 
     componentDidMount() {
-        this.getTags()
+        void this.getTags()
     }
 
     render() {

diff --git a/adminSiteClient/ChartRow.tsx b/adminSiteClient/ChartRow.tsx
@@ -36,7 +36,7 @@ export class ChartRow extends React.Component<{
     }
 
     @action.bound onSaveTags(tags: DbChartTagJoin[]) {
-        this.saveTags(tags)
+        void this.saveTags(tags)
     }
 
     render() {

diff --git a/adminSiteClient/DatasetEditPage.tsx b/adminSiteClient/DatasetEditPage.tsx
@@ -267,7 +267,7 @@ class DatasetEditor extends React.Component<{ dataset: DatasetPageData }> {
                     <form
                         onSubmit={(e) => {
                             e.preventDefault()
-                            this.save()
+                            void this.save()
                         }}
                     >
                         <p>
@@ -422,6 +422,6 @@ export class DatasetEditPage extends React.Component<{ datasetId: number }> {
         this.UNSAFE_componentWillReceiveProps()
     }
     UNSAFE_componentWillReceiveProps() {
-        this.getData()
+        void this.getData()
     }
 }
diff --git a/adminSiteClient/DatasetList.tsx b/adminSiteClient/DatasetList.tsx
@@ -49,7 +49,7 @@ class DatasetRow extends React.Component<{
     }
 
     @action.bound onSaveTags(tags: DbChartTagJoin[]) {
-        this.saveTags(tags)
+        void this.saveTags(tags)
     }
 
     render() {
@@ -113,7 +113,7 @@ export class DatasetList extends React.Component<{
     }
 
     componentDidMount() {
-        this.getTags()
+        void this.getTags()
     }
 
     render() {

diff --git a/adminSiteClient/DatasetsIndexPage.tsx b/adminSiteClient/DatasetsIndexPage.tsx
@@ -114,6 +114,6 @@ export class DatasetsIndexPage extends React.Component {
     }
 
     componentDidMount() {
-        this.getData()
+        void this.getData()
     }
 }
diff --git a/adminSiteClient/DeployStatusPage.tsx b/adminSiteClient/DeployStatusPage.tsx
@@ -25,7 +25,7 @@ export class DeployStatusPage extends React.Component {
     refreshIntervalId?: number
 
     componentDidMount() {
-        this.getData()
+        void this.getData()
         document.addEventListener(
             "visibilitychange",
             this.handleVisibilityChange
@@ -120,7 +120,7 @@ export class DeployStatusPage extends React.Component {
     }
 
     @action.bound handleVisibilityChange = () => {
-        if (document.visibilityState === "visible") this.getData()
+        if (document.visibilityState === "visible") void this.getData()
     }
 
     @action.bound async triggerDeploy() {

diff --git a/adminSiteClient/EditorExportTab.tsx b/adminSiteClient/EditorExportTab.tsx
@@ -163,28 +163,28 @@ export class EditorExportTab extends React.Component<EditorExportTabProps> {
     }
 
     @action.bound private onDownloadDesktopSVG() {
-        this.download(
+        void this.download(
             `${this.baseFilename}-desktop.svg`,
             GrapherStaticFormat.landscape
         )
     }
 
     @action.bound private onDownloadDesktopPNG() {
-        this.download(
+        void this.download(
             `${this.baseFilename}-desktop.png`,
             GrapherStaticFormat.landscape
         )
     }
 
     @action.bound private onDownloadMobileSVG() {
-        this.download(
+        void this.download(
             `${this.baseFilename}-mobile.svg`,
             GrapherStaticFormat.square
         )
     }
 
     @action.bound private onDownloadMobilePNG() {
-        this.download(
+        void this.download(
             `${this.baseFilename}-mobile.png`,
             GrapherStaticFormat.square
         )

diff --git a/adminSiteClient/EditorHistoryTab.tsx b/adminSiteClient/EditorHistoryTab.tsx
@@ -73,7 +73,7 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
         delete chartConfigObject.version
         delete chartConfigObject.isPublished
         const chartConfigAsYaml = dump(chartConfigObject)
-        copyToClipboard(chartConfigAsYaml)
+        void copyToClipboard(chartConfigAsYaml)
         notification["success"]({
             message: "Copied YAML to clipboard",
             description: "You can now paste this into the ETL",

diff --git a/adminSiteClient/EditorReferencesTab.tsx b/adminSiteClient/EditorReferencesTab.tsx
@@ -262,7 +262,7 @@ class AddRedirectForm extends React.Component<{
             <form
                 onSubmit={(e) => {
                     e.preventDefault()
-                    this.onSubmit()
+                    void this.onSubmit()
                 }}
             >
                 <div className="input-group mb-3">

diff --git a/adminSiteClient/ExplorerTagsPage.tsx b/adminSiteClient/ExplorerTagsPage.tsx
@@ -29,7 +29,7 @@ export class ExplorerTagsPage extends React.Component {
     @observable newExplorerTags: DbChartTagJoin[] = []
 
     componentDidMount() {
-        this.getData()
+        void this.getData()
     }
 
     // Don't show explorers that already have tags
@@ -101,7 +101,7 @@ export class ExplorerTagsPage extends React.Component {
                                                 tags={explorer.tags}
                                                 suggestions={this.tags}
                                                 onSave={(tags) => {
-                                                    this.saveTags(
+                                                    void this.saveTags(
                                                         explorer.slug,
                                                         tags
                                                     )

diff --git a/adminSiteClient/GdocsPreviewPage.tsx b/adminSiteClient/GdocsPreviewPage.tsx
@@ -108,7 +108,7 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
             }
         }
         if (!originalGdoc) {
-            fetchGdocs()
+            void fetchGdocs()
         }
     }, [originalGdoc, fetchGdoc, handleError, admin])
 

diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx
@@ -298,7 +298,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
         // Add the disposer to the list of things we need to clean up on unmount
         this.disposers.push(disposer)
 
-        this.getFieldDefinitions()
+        void this.getFieldDefinitions()
     }
 
     @action.bound private resetViewStateAfterFetch(): void {
@@ -435,7 +435,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
                     currentColumnFieldDescription.default !== undefined &&
                     currentColumnFieldDescription.default === prevVal,
             }
-            this.doAction({ patches: [patch] })
+            void this.doAction({ patches: [patch] })
         } else {
             setValueRecursiveInplace(grapher, pointer, prevVal)
         }
@@ -942,7 +942,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
             }
         }
 
-        this.doAction({ patches })
+        void this.doAction({ patches })
     }
     validateCellChanges(
         changes: Handsontable.CellChange[] | null,
@@ -1084,7 +1084,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
         }
         // Perform a do operation that will add this to the undo stack and immediately
         // post this as a PATCH request to the server
-        this.doAction({ patches })
+        void this.doAction({ patches })
     }
 
     async getFieldDefinitions() {

diff --git a/adminSiteClient/PostEditorPage.tsx b/adminSiteClient/PostEditorPage.tsx
@@ -43,7 +43,7 @@ export class PostEditorPage extends React.Component<{ postId?: number }> {
     }
 
     componentDidMount() {
-        this.fetchPost()
+        void this.fetchPost()
     }
 
     render() {

diff --git a/adminSiteClient/PostsIndexPage.tsx b/adminSiteClient/PostsIndexPage.tsx
@@ -85,7 +85,7 @@ class PostRow extends React.Component<PostRowProps> {
     }
 
     @action.bound onSaveTags(tags: DbChartTagJoin[]) {
-        this.saveTags(tags)
+        void this.saveTags(tags)
     }
 
     @action.bound async onConvertGdoc(allowRecreate: boolean = false) {
@@ -106,7 +106,7 @@ class PostRow extends React.Component<PostRowProps> {
                 "This will overwrite the existing GDoc. Are you sure?"
             )
         ) {
-            this.onConvertGdoc(true)
+            void this.onConvertGdoc(true)
         }
     }
 
@@ -376,7 +376,7 @@ export class PostsIndexPage extends React.Component {
     }
 
     componentDidMount() {
-        this.getData()
-        this.getTags()
+        void this.getData()
+        void this.getTags()
     }
 }
diff --git a/adminSiteClient/RedirectsIndexPage.tsx b/adminSiteClient/RedirectsIndexPage.tsx
@@ -115,6 +115,6 @@ export class RedirectsIndexPage extends React.Component {
     }
 
     componentDidMount() {
-        this.getData()
+        void this.getData()
     }
 }
diff --git a/adminSiteClient/SaveButtons.tsx b/adminSiteClient/SaveButtons.tsx
@@ -7,11 +7,11 @@ import { isEmpty } from "@ourworldindata/utils"
 @observer
 export class SaveButtons extends React.Component<{ editor: ChartEditor }> {
     @action.bound onSaveChart() {
-        this.props.editor.saveGrapher()
+        void this.props.editor.saveGrapher()
     }
 
     @action.bound onSaveAsNew() {
-        this.props.editor.saveAsNewGrapher()
+        void this.props.editor.saveAsNewGrapher()
     }
 
     @action.bound onPublishToggle() {

diff --git a/adminSiteClient/SourceEditPage.tsx b/adminSiteClient/SourceEditPage.tsx
@@ -98,7 +98,7 @@ class SourceEditor extends React.Component<{ source: SourcePageData }> {
                     <form
                         onSubmit={(e) => {
                             e.preventDefault()
-                            this.save()
+                            void this.save()
                         }}
                     >
                         {isBulkImport && (
@@ -189,6 +189,6 @@ export class SourceEditPage extends React.Component<{ sourceId: number }> {
         this.UNSAFE_componentWillReceiveProps()
     }
     UNSAFE_componentWillReceiveProps() {
-        this.getData()
+        void this.getData()
     }
 }