From 5789d65bf5b526eb554b45d9f2ab06d2bbc61ab0 Mon Sep 17 00:00:00 2001 From: Hemachandar <132386067+hmacr@users.noreply.github.com> Date: Thu, 17 Apr 2025 03:52:51 +0530 Subject: [PATCH] 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 | 4 ++-- plugins/iframely/server/iframely.ts | 10 ++++++---- server/routes/api/urls/urls.test.ts | 2 +- server/routes/api/urls/urls.ts | 7 ++++--- server/types.ts | 4 +++- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/plugins/github/server/github.ts b/plugins/github/server/github.ts index 0604488c7e..e7e03b6250 100644 --- a/plugins/github/server/github.ts +++ b/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" }; } }; } diff --git a/plugins/iframely/server/iframely.ts b/plugins/iframely/server/iframely.ts index 3d5328fcf7..6ca473c6e7 100644 --- a/plugins/iframely/server/iframely.ts +++ b/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 { + ): Promise { 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 }; }; } diff --git a/server/routes/api/urls/urls.test.ts b/server/routes/api/urls/urls.test.ts index 6f7f1b96f7..17a6a94d2e 100644 --- a/server/routes/api/urls/urls.test.ts +++ b/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(); diff --git a/server/routes/api/urls/urls.ts b/server/routes/api/urls/urls.ts index f730f1a0bd..f1e06d725d 100644 --- a/server/routes/api/urls/urls.ts +++ b/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, diff --git a/server/types.ts b/server/types.ts index a0d1c44493..f5952b4e01 100644 --- a/server/types.ts +++ b/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; +) => Promise; export type UninstallSignature = (integration: Integration) => Promise;