Use ErrorBoundary for attachment renderer (#3761)

* Use ErrorBoundary for attachment renderer * Add entry
microsoft · Feb 25, 2021 · a972426 · a972426
1 parent 873a827
commit a972426
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -65,6 +65,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 -  Fixes [#3618](https://github.com/microsoft/BotFramework-WebChat/issues/3618). Fix Adaptive Cards anchors being disabled when Adaptive Cards is obsolete, by [@corinagum](https://github.com/corinagum) in PR [#3687](https://github.com/microsoft/BotFramework-WebChat/pull/3687)
 -  Fixes [#3747](https://github.com/microsoft/BotFramework-WebChat/issues/3747). `aria-pressed` and `aria-role` is not properly set on Adaptive Cards submit buttons, by [@amal-khalaf](https://github.com/amal-khalaf) in PR [#3744](https://github.com/microsoft/BotFramework-WebChat/pull/3744)
 -  Fixes [#3750](https://github.com/microsoft/BotFramework-WebChat/issues/3750). Debump Node.js engines requirements for some packages to `12.0.0`, by [@compulim](https://github.com/compulim) in PR [#3753](https://github.com/microsoft/BotFramework-WebChat/pull/3753)
+-  Fixes [#3760](https://github.com/microsoft/BotFramework-WebChat/issues/3760). Use `<ErrorBoundary>` to wrap around attachment renderer, by [@compulim](https://github.com/compulim) in PR [#3761](https://github.com/microsoft/BotFramework-WebChat/pull/3761)
 
 ### Changed
 

diff --git a/...ty-unknown-attachment-js-activity-should-not-show-unknown-attachment-1-snap.png b/...ty-unknown-attachment-js-activity-should-not-show-unknown-attachment-1-snap.png
diff --git a/__tests__/html/activity.unknownAttachment.html b/__tests__/html/activity.unknownAttachment.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html lang="en-US">
+  <head>
+    <script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
+    <script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
+  </head>
+  <body>
+    <div id="webchat"></div>
+    <script type="text/babel" data-presets="env,stage-3,react">
+      const {
+        WebChat: { createDirectLine },
+        WebChatTest: { conditions, createStore, host, pageObjects, timeouts, token }
+      } = window;
+
+      (async function () {
+        window.WebChat.renderWebChat(
+          {
+            directLine: createDirectLine({ token: await token.fetchDirectLineToken() }),
+            store: createStore()
+          },
+          document.getElementById('webchat')
+        );
+
+        await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);
+        await pageObjects.sendMessageViaSendBox('unknown attachment');
+        await pageObjects.wait(conditions.minNumActivitiesShown(2), timeouts.directLine);
+
+        await host.snapshot();
+        await host.done();
+      })().catch(async err => {
+        console.error(err);
+
+        await host.error(err);
+      });
+    </script>
+  </body>
+</html>
diff --git a/__tests__/html/activity.unknownAttachment.js b/__tests__/html/activity.unknownAttachment.js
@@ -0,0 +1,8 @@
+/**
+ * @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
+ */
+
+describe('activity', () => {
+  test('should not show unknown attachment', () =>
+    runHTMLTest('activity.unknownAttachment.html', { ignoreConsoleError: true }));
+});
diff --git a/packages/api/src/hooks/Composer.js b/packages/api/src/hooks/Composer.js
@@ -47,7 +47,11 @@ import Tracker from './internal/Tracker';
 import WebChatReduxContext, { useDispatch } from './internal/WebChatReduxContext';
 import WebChatAPIContext from './internal/WebChatAPIContext';
 
-import applyMiddleware, { forRenderer as applyMiddlewareForRenderer } from './middleware/applyMiddleware';
+import applyMiddleware, {
+  forLegacyRenderer as applyMiddlewareForLegacyRenderer,
+  forRenderer as applyMiddlewareForRenderer
+} from './middleware/applyMiddleware';
+
 import patchStyleOptions from '../patchStyleOptions';
 import singleToArray from './utils/singleToArray';
 
@@ -349,13 +353,17 @@ const Composer = ({
     }
 
     // Attachment renderer
-    return applyMiddleware('attachment', ...singleToArray(attachmentMiddleware), () => () => ({ attachment }) => {
-      if (attachment) {
-        throw new Error(`No renderer for attachment of type "${attachment.contentType}"`);
-      } else {
-        throw new Error('No attachment to render');
+    return applyMiddlewareForLegacyRenderer(
+      'attachment',
+      ...singleToArray(attachmentMiddleware),
+      () => () => ({ attachment }) => {
+        if (attachment) {
+          throw new Error(`No renderer for attachment of type "${attachment.contentType}"`);
+        } else {
+          throw new Error('No attachment to render');
+        }
       }
-    })({});
+    )({});
   }, [attachmentMiddleware, attachmentRenderer]);
 
   const patchedAvatarRenderer = useMemo(() => {

diff --git a/packages/api/src/hooks/middleware/applyMiddleware.js b/packages/api/src/hooks/middleware/applyMiddleware.js
@@ -10,6 +10,27 @@ export default function applyMiddleware(type, ...middleware) {
       throw new Error(`reached terminator of ${type}`);
     });
 }
+
+export function forLegacyRenderer(type, ...middleware) {
+  return (...setupArgs) => {
+    const fn = concatMiddleware(...middleware)(...setupArgs)(() => {
+      throw new Error(`reached terminator of ${type}`);
+    });
+
+    return (...args) => (
+      <UserlandBoundary type={`render of ${type}`}>
+        {() => {
+          try {
+            return fn(...args);
+          } catch (err) {
+            return <ErrorBox error={err} type={`render of ${type}`} />;
+          }
+        }}
+      </UserlandBoundary>
+    );
+  };
+}
+
 /**
  *
  * @param {string} type Required. String equivalent of type of container to be rendered.
@@ -40,6 +61,7 @@ export function forRenderer(type, { strict = false } = {}, ...middleware) {
 
           return <UserlandBoundary type={`render of ${type}`}>{render}</UserlandBoundary>;
         }
+
         return (...renderTimeArgs) => (
           <UserlandBoundary type={`render of ${type}`}>
             {() => {
@@ -52,13 +74,13 @@ export function forRenderer(type, { strict = false } = {}, ...middleware) {
 
                 return element;
               } catch (err) {
-                return <ErrorBox error={err} type={type} />;
+                return <ErrorBox error={err} type={`render of ${type}`} />;
               }
             }}
           </UserlandBoundary>
         );
       } catch (err) {
-        return <ErrorBox error={err} type={type} />;
+        return <ErrorBox error={err} type={`render of ${type}`} />;
       }
     };
   };