Ensure iframely fallback is not executed for connected unfurl integration (#8995)

* Ensure iframely fallback is not executed for connected unfurl integration

* tsc

plugins/github/server/github.ts

@@ -206,12 +206,12 @@ export class GitHub {
       );
       const { data } = await client.requestResource(resource);
       if (!data) {
-        return;
+        return { error: "Resource not found" };
       }
       return { ...data, type: resource.type };
     } catch (err) {
       Logger.warn("Failed to fetch resource from GitHub", err);
-      return;
+      return { error: err.message || "Unknown error" };
     }
   };
 }

plugins/iframely/server/iframely.ts

@@ -1,6 +1,6 @@
 import { JSONObject, UnfurlResourceType } from "@shared/types";
 import Logger from "@server/logging/Logger";
-import { UnfurlSignature } from "@server/types";
+import { UnfurlError, UnfurlSignature } from "@server/types";
 import fetch from "@server/utils/fetch";
 import env from "./env";
 
@@ -10,7 +10,7 @@ class Iframely {
   public static async requestResource(
     url: string,
     type = "oembed"
-  ): Promise<JSONObject | undefined> {
+  ): Promise<JSONObject | UnfurlError> {
     const isDefaultHost = env.IFRAMELY_URL === this.defaultUrl;
 
     // Cloud Iframely requires /api path, while self-hosted does not.
@@ -25,7 +25,7 @@ class Iframely {
       return await res.json();
     } catch (err) {
       Logger.error(`Error fetching data from Iframely for url: ${url}`, err);
-      return;
+      return { error: err.message || "Unknown error" };
     }
   }
 
@@ -36,7 +36,9 @@ class Iframely {
    */
   public static unfurl: UnfurlSignature = async (url: string) => {
     const data = await Iframely.requestResource(url);
-    return { ...data, type: UnfurlResourceType.OEmbed };
+    return "error" in data // In addition to our custom UnfurlError, sometimes iframely returns error in the response body.
+      ? ({ error: data.error } as UnfurlError)
+      : { ...data, type: UnfurlResourceType.OEmbed };
   };
 }
 

server/routes/api/urls/urls.test.ts

@@ -20,7 +20,7 @@ jest.mock("dns", () => ({
 
 jest
   .spyOn(Iframely, "requestResource")
-  .mockImplementation(() => Promise.resolve(undefined));
+  .mockImplementation(() => Promise.resolve({}));
 
 const server = getTestServer();
 

server/routes/api/urls/urls.ts

@@ -98,11 +98,12 @@ router.post(
     }
 
     for (const plugin of plugins) {
-      const data = await plugin.value.unfurl(url, actor);
-      if (data) {
-        if ("error" in data) {
+      const unfurl = await plugin.value.unfurl(url, actor);
+      if (unfurl) {
+        if (unfurl.error) {
           return (ctx.response.status = 204);
         } else {
+          const data = unfurl as Unfurl;
           await CacheHelper.setData(
             CacheHelper.getUnfurlKey(actor.teamId, url),
             data,

server/types.ts

@@ -578,10 +578,12 @@ export type CollectionJSONExport = {
 
 export type Unfurl = { [x: string]: JSONValue; type: UnfurlResourceType };
 
+export type UnfurlError = { error: string };
+
 export type UnfurlSignature = (
   url: string,
   actor?: User
-) => Promise<Unfurl | void>;
+) => Promise<Unfurl | UnfurlError | undefined>;
 
 export type UninstallSignature = (integration: Integration) => Promise<void>;