fix: Display fallback instead of error if cannot unfurl URL (#10370)

* fix: Display fallback instead of error if cannot unfurl URL

* Optimised images with calibre/image-actions

* fix: Write loaded to props to attrs

* Optimised images with calibre/image-actions

* white background

* Optimised images with calibre/image-actions

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

shared/editor/components/Mentions.tsx

@@ -22,13 +22,13 @@ import useStores from "../../hooks/useStores";
 import theme from "../../styles/theme";
 import {
   IntegrationService,
+  UnfurlResourceType,
   type JSONValue,
-  type UnfurlResourceType,
   type UnfurlResponse,
 } from "../../types";
 import { cn } from "../styles/utils";
 import { ComponentProps } from "../types";
-import { sanitizeUrl } from "@shared/utils/urls";
+import { toDisplayUrl, cdnPath } from "../../utils/urls";
 
 type Attrs = {
   className: string;
@@ -144,10 +144,15 @@ type IssuePrProps = ComponentProps & {
   ) => void;
 };
 
-export const MentionURL = (props: ComponentProps) => {
+type IssueUrlProps = ComponentProps & {
+  onChangeUnfurl: (unfurl: UnfurlResponse[UnfurlResourceType.URL]) => void;
+};
+
+export const MentionURL = (props: IssueUrlProps) => {
   const { unfurls } = useStores();
   const isMounted = useIsMounted();
   const [loaded, setLoaded] = React.useState(false);
+  const onChangeUnfurl = React.useRef(props.onChangeUnfurl).current; // stable reference to callback function.
 
   const { isSelected, node } = props;
   const {
@@ -156,17 +161,39 @@ export const MentionURL = (props: ComponentProps) => {
     ...attrs
   } = getAttributesFromNode(node);
 
+  const url = String(attrs.href);
   const unfurl = unfurls.get(attrs.href)?.data ?? unfurlAttr;
 
   React.useEffect(() => {
     const fetchUnfurl = async () => {
-      await unfurls.fetchUnfurl({ url: attrs.href });
+      try {
+        const unfurlModel = await unfurls.fetchUnfurl({ url });
 
-      if (!isMounted()) {
-        return;
+        if (!isMounted()) {
+          return;
+        }
+
+        if (unfurlModel) {
+          onChangeUnfurl(
+            unfurlModel.data satisfies UnfurlResponse[UnfurlResourceType.URL]
+          );
+        } else {
+          // If we didn't get a result back, we still want to add a basic unfurl
+          // to avoid refetching again in future. This will just show the URL
+          // with a generic link icon.
+          unfurls.add({
+            id: url,
+            type: UnfurlResourceType.URL,
+            fetchedAt: new Date().toISOString(),
+            data: {
+              title: toDisplayUrl(url),
+              faviconUrl: cdnPath("/images/link.png"),
+            },
+          });
+        }
+      } finally {
+        setLoaded(true);
       }
-
-      setLoaded(true);
     };
 
     void fetchUnfurl();
@@ -191,9 +218,7 @@ export const MentionURL = (props: ComponentProps) => {
       rel="noopener noreferrer nofollow"
     >
       <Flex align="center" gap={6}>
-        {unfurl.faviconUrl ? (
-          <Logo src={sanitizeUrl(unfurl.faviconUrl)} alt="" />
-        ) : null}
+        {unfurl.faviconUrl ? <Logo src={unfurl.faviconUrl} alt="" /> : null}
         <Text>
           <Backticks content={unfurl.title} />
         </Text>

shared/editor/nodes/Mention.tsx

@@ -145,7 +145,12 @@ export default class Mention extends Node {
           />
         );
       case MentionType.URL:
-        return <MentionURL {...props} />;
+        return (
+          <MentionURL
+            {...props}
+            onChangeUnfurl={this.handleChangeUnfurl(props)}
+          />
+        );
       default:
         return null;
     }
@@ -323,7 +328,8 @@ export default class Mention extends Node {
 
       const label =
         unfurl.type === UnfurlResourceType.Issue ||
-        unfurl.type === UnfurlResourceType.PR
+        unfurl.type === UnfurlResourceType.PR ||
+        unfurl.type === UnfurlResourceType.URL
           ? unfurl.title
           : undefined;
 

shared/utils/urls.ts

@@ -230,3 +230,18 @@ export function urlRegex(url: string | null | undefined): RegExp | undefined {
 export function getUrls(text: string) {
   return Array.from(text.match(/(?:https?):\/\/[^\s]+/gi) || []);
 }
+
+/**
+ * Converts a url to a display friendly format, removing the protocol and trailing slash.
+ *
+ * @param url The url to convert.
+ * @returns The display friendly url.
+ */
+export function toDisplayUrl(url: string) {
+  try {
+    const parsed = new URL(url);
+    return parsed.host + (parsed.pathname === "/" ? "" : parsed.pathname);
+  } catch {
+    return url;
+  }
+}