fix: JS error when no integrations are connected

* fix: Various cases that could leave file handles open on export

* Consolidate error handling

app/components/Sidebar/Settings.tsx

@@ -23,18 +23,21 @@ import ToggleButton from "./components/ToggleButton";
 import Version from "./components/Version";
 
 function SettingsSidebar() {
-  const { ui } = useStores();
+  const { ui, integrations } = useStores();
   const { t } = useTranslation();
   const history = useHistory();
   const location = useLocation();
-  let configs = useSettingsConfig();
+  const configs = useSettingsConfig();
 
-  configs = configs.filter((item) =>
-    "isActive" in item ? item.isActive : true
+  const groupedConfig = groupBy(
+    configs.filter((item) =>
+      item.group === "Integrations" && item.pluginId
+        ? integrations.findByService(item.pluginId)
+        : true
+    ),
+    "group"
   );
 
-  const groupedConfig = groupBy(configs, "group");
-
   const returnToApp = React.useCallback(() => {
     history.push("/home");
   }, [history]);

app/hooks/useEmbeds.ts

@@ -18,8 +18,7 @@ export default function useEmbeds(loadIfMissing = false) {
   React.useEffect(() => {
     async function fetchEmbedIntegrations() {
       try {
-        await integrations.fetchPage({
-          limit: 100,
+        await integrations.fetchAll({
           type: IntegrationType.Embed,
         });
       } catch (err) {

app/hooks/useSettingsConfig.ts

@@ -54,7 +54,7 @@ export type ConfigItem = {
   preload?: () => void;
   enabled: boolean;
   group: string;
-  isActive?: boolean;
+  pluginId?: string;
 };
 
 const useSettingsConfig = () => {
@@ -231,6 +231,7 @@ const useSettingsConfig = () => {
             ? integrationSettingsPath(plugin.id)
             : settingsPath(plugin.id),
         group: t(group),
+        pluginId: plugin.id,
         description: plugin.value.description,
         component: plugin.value.component.Component,
         preload: plugin.value.component.preload,
@@ -238,10 +239,6 @@ const useSettingsConfig = () => {
           ? plugin.value.enabled(team, user)
           : can.update,
         icon: plugin.value.icon,
-        isActive: integrations.orderedData.some(
-          (integration) =>
-            integration.service.toLowerCase() === plugin.id.toLowerCase()
-        ),
       } as ConfigItem);
     });
 

app/scenes/Settings/Integrations.tsx

@@ -1,3 +1,4 @@
+import groupBy from "lodash/groupBy";
 import * as React from "react";
 import { Trans, useTranslation } from "react-i18next";
 import styled from "styled-components";
@@ -7,28 +8,34 @@ import InputSearch from "~/components/InputSearch";
 import Scene from "~/components/Scene";
 import Text from "~/components/Text";
 import useSettingsConfig from "~/hooks/useSettingsConfig";
+import useStores from "~/hooks/useStores";
 import { settingsPath } from "~/utils/routeHelpers";
 import IntegrationCard from "./components/IntegrationCard";
 import { StickyFilters } from "./components/StickyFilters";
 
 export function Integrations() {
   const { t } = useTranslation();
-  let items = useSettingsConfig();
+  const { integrations } = useStores();
+  const items = useSettingsConfig();
   const [query, setQuery] = React.useState("");
 
   const handleQuery = (event: React.ChangeEvent<HTMLInputElement>) => {
     setQuery(event.target.value);
   };
 
-  items = items
-    .filter(
+  const groupedItems = groupBy(
+    items.filter(
       (item) =>
         item.group === "Integrations" &&
         item.enabled &&
         item.path !== settingsPath("integrations") &&
         item.name.toLowerCase().includes(query.toLowerCase())
-    )
-    .sort((item) => (item.isActive ? -1 : 1));
+    ),
+    (item) =>
+      item.pluginId && integrations.findByService(item.pluginId)
+        ? "connected"
+        : "available"
+  );
 
   return (
     <Scene title={t("Integrations")}>
@@ -47,16 +54,19 @@ export function Integrations() {
         />
       </StickyFilters>
 
-      <CardsFlex gap={30} wrap>
-        {items.map((item) => (
+      <Cards gap={30} wrap>
+        {groupedItems.connected?.map((item) => (
+          <IntegrationCard key={item.path} integration={item} isConnected />
+        ))}
+        {groupedItems.available?.map((item) => (
           <IntegrationCard key={item.path} integration={item} />
         ))}
-      </CardsFlex>
+      </Cards>
     </Scene>
   );
 }
 
-const CardsFlex = styled(Flex)`
+const Cards = styled(Flex)`
   margin-top: 20px;
   width: "100%";
 `;

app/scenes/Settings/components/IntegrationCard.tsx

@@ -10,20 +10,22 @@ import Text from "../../../components/Text";
 
 type Props = {
   integration: ConfigItem;
+  isConnected?: boolean;
 };
 
-function IntegrationCard({ integration }: Props) {
+function IntegrationCard({ integration, isConnected }: Props) {
   const { t } = useTranslation();
+
   return (
     <Card as={Link} to={integration.path}>
       <Flex align="center" gap={8}>
         <integration.icon size={48} />
         <Flex auto column>
           <Name>{integration.name}</Name>
-          {integration.isActive && <Status>{t("Connected")}</Status>}
+          {isConnected && <Status>{t("Connected")}</Status>}
         </Flex>
-        <Button as={Link} to={integration.path} neutral>
-          {integration.isActive ? t("Configure") : t("Connect")}
+        <Button as="span" neutral>
+          {isConnected ? t("Configure") : t("Connect")}
         </Button>
       </Flex>
 

app/stores/IntegrationsStore.ts

@@ -10,6 +10,12 @@ class IntegrationsStore extends Store<Integration> {
     super(rootStore, Integration);
   }
 
+  findByService(service: string) {
+    return this.orderedData.find(
+      (integration) => integration.service === service
+    );
+  }
+
   @computed
   get orderedData(): Integration[] {
     return naturalSort(Array.from(this.data.values()), "name");

plugins/googleanalytics/client/Settings.tsx

@@ -40,12 +40,6 @@ function GoogleAnalytics() {
     },
   });
 
-  React.useEffect(() => {
-    void integrations.fetchPage({
-      type: IntegrationType.Analytics,
-    });
-  }, [integrations]);
-
   React.useEffect(() => {
     reset({ measurementId: integration?.settings.measurementId });
   }, [integration, reset]);

plugins/matomo/client/Settings.tsx

@@ -42,12 +42,6 @@ function Matomo() {
     },
   });
 
-  React.useEffect(() => {
-    void integrations.fetchPage({
-      type: IntegrationType.Analytics,
-    });
-  }, [integrations]);
-
   React.useEffect(() => {
     reset({
       measurementId: integration?.settings.measurementId,

plugins/slack/client/Settings.tsx

@@ -34,13 +34,7 @@ function Slack() {
   const error = query.get("error");
 
   React.useEffect(() => {
-    void collections.fetchPage({
-      limit: 100,
-    });
-    void integrations.fetchPage({
-      service: IntegrationService.Slack,
-      limit: 100,
-    });
+    void collections.fetchAll();
   }, [collections, integrations]);
 
   const commandIntegration = integrations.find({

plugins/umami/client/Settings.tsx

@@ -44,12 +44,6 @@ function Umami() {
     },
   });
 
-  React.useEffect(() => {
-    void integrations.fetchPage({
-      type: IntegrationType.Analytics,
-    });
-  }, [integrations]);
-
   React.useEffect(() => {
     reset({
       umamiWebsiteId: integration?.settings.measurementId,

server/utils/ZipHelper.ts

@@ -41,7 +41,7 @@ export default class ZipHelper {
           prefix: "export-",
           postfix: ".zip",
         },
-        (err, path) => {
+        (err, filePath) => {
           if (err) {
             return reject(err);
           }
@@ -51,13 +51,24 @@ export default class ZipHelper {
             currentFile: null,
           };
 
+          const handleError = (error: Error) => {
+            dest.destroy();
+            fs.remove(filePath)
+              .catch((rmErr) => {
+                Logger.error("Failed to remove tmp file", rmErr);
+              })
+              .finally(() => {
+                reject(error);
+              });
+          };
+
           const dest = fs
-            .createWriteStream(path)
+            .createWriteStream(filePath)
             .on("finish", () => {
-              Logger.debug("utils", "Writing zip complete", { path });
-              return resolve(path);
+              Logger.debug("utils", "Writing zip complete", { path: filePath });
+              return resolve(filePath);
             })
-            .on("error", reject);
+            .on("error", handleError);
 
           zip
             .generateNodeStream(
@@ -85,11 +96,9 @@ export default class ZipHelper {
                 }
               }
             )
-            .on("error", (rErr) => {
-              dest.end();
-              reject(rErr);
-            })
-            .pipe(dest);
+            .on("error", handleError)
+            .pipe(dest)
+            .on("error", handleError);
         }
       );
     });
@@ -126,32 +135,38 @@ export default class ZipHelper {
               const fileName = Buffer.from(entry.fileName).toString("utf8");
               Logger.debug("utils", "Extracting zip entry", { fileName });
 
+              const processNext = (error?: NodeJS.ErrnoException | null) => {
+                if (error) {
+                  zipfile.close();
+                  reject(error);
+                  return;
+                }
+                zipfile.readEntry();
+              };
+
               if (validateFileName(fileName)) {
                 Logger.warn("Invalid zip entry", { fileName });
-                zipfile.readEntry();
-              } else if (/\/$/.test(fileName)) {
+                processNext();
+                return;
+              }
+
+              if (/\/$/.test(fileName)) {
                 // directory file names end with '/'
-                fs.mkdirp(
-                  path.join(outputDir, fileName),
-                  function (mErr: Error) {
-                    if (mErr) {
-                      return reject(mErr);
-                    }
-                    zipfile.readEntry();
-                  }
+                fs.mkdirp(path.join(outputDir, fileName), (mkErr) =>
+                  processNext(mkErr)
                 );
               } else {
                 // file entry
                 zipfile.openReadStream(entry, function (rErr, readStream) {
                   if (rErr) {
-                    return reject(rErr);
+                    return processNext(rErr);
                   }
                   // ensure parent directory exists
                   fs.mkdirp(
                     path.join(outputDir, path.dirname(fileName)),
                     function (mkErr) {
                       if (mkErr) {
-                        return reject(mkErr);
+                        return processNext(mkErr);
                       }
 
                       const location = trimFileAndExt(
@@ -163,15 +178,20 @@ export default class ZipHelper {
                       );
                       const dest = fs
                         .createWriteStream(location)
-                        .on("error", reject);
+                        .on("error", (error) => {
+                          readStream.destroy();
+                          dest.destroy();
+                          processNext(error);
+                        });
 
                       readStream
-                        .on("error", (rsErr) => {
-                          dest.end();
-                          reject(rsErr);
+                        .on("error", (error) => {
+                          dest.destroy();
+                          readStream.destroy();
+                          processNext(error);
                         })
                         .on("end", function () {
-                          zipfile.readEntry();
+                          processNext();
                         })
                         .pipe(dest);
                     }
@@ -180,8 +200,14 @@ export default class ZipHelper {
               }
             });
             zipfile.on("close", resolve);
-            zipfile.on("error", reject);
+            zipfile.on("error", (error) => {
+              zipfile.close();
+              reject(error);
+            });
           } catch (zErr) {
+            if (zipfile) {
+              zipfile.close();
+            }
             reject(zErr);
           }
         }