Skip to content

Commit a71bd9c

Browse files
compulimcompulim
committed
Fix polymiddleware error should not propagate to React runtime
1 parent c138220 commit a71bd9c

4 files changed

Lines changed: 112 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ Breaking changes in this release:
423423
- Fixed Fluent/Copilot typing indicator animation background color, in PR [#5770](https://github.com/microsoft/BotFramework-WebChat/pull/5770), by [@OEvgeny](https://github.com/OEvgeny)
424424
- Fixed `<AddFullBundle>` should not re-render when `attachment[ForScreenReader]Middleware` is updated without noticeable different (`iterateEquals`), by [@compulim](https://github.com/compulim), in PR [#5779](https://github.com/microsoft/BotFramework-WebChat/pull/5779)
425425
- Fixed send box should narrate `aria-label` prop, by [@compulim](https://github.com/compulim), in PR [#5805](https://github.com/microsoft/BotFramework-WebChat/pull/5805)
426+
- Fixed polymiddleware error should not propagate to React runtime if `<DebugProvider>` is not mounted, by [@compulim](https://github.com/compulim), in PR [#XXX](https://github.com/microsoft/BotFramework-WebChat/pull/XXX)
426427

427428
## [4.18.0] - 2024-07-10
428429

‎__tests__/html2/adaptiveCard/renderError.withoutDebug.html‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<!doctype html>
2+
<html lang="en-US">
3+
<head>
4+
<link href="/assets/index.css" rel="stylesheet" type="text/css" />
5+
<script>
6+
const warn = console.warn.bind(console);
7+
8+
// We need to fake the mock ourselves without using jest-mock.
9+
// jest-mock is ESM and cannot mock before Web Chat.
10+
console.warn = (...args) => {
11+
console.warn.mock.calls.push(args);
12+
13+
warn(...args);
14+
};
15+
16+
console.warn._isMockFunction = true;
17+
console.warn.getMockName = () => 'warn';
18+
console.warn.mock = { calls: [] };
19+
</script>
20+
<script type="importmap">
21+
{
22+
"imports": {
23+
"react": "https://esm.sh/react@18.3.1",
24+
"react-dom": "https://esm.sh/react-dom@18.3.1",
25+
"react-dom/": "https://esm.sh/react-dom@18.3.1/"
26+
}
27+
}
28+
</script>
29+
<script crossorigin="anonymous" src="/test-harness.js"></script>
30+
<script crossorigin="anonymous" src="/test-page-object.js"></script>
31+
<script type="module">
32+
import React from 'react';
33+
34+
window.React = React;
35+
</script>
36+
<script crossorigin="anonymous" defer src="/__dist__/webchat-es5.js"></script>
37+
<script crossorigin="anonymous" defer src="/__dist__/botframework-webchat-debug-theme.development.js"></script>
38+
</head>
39+
<body>
40+
<main id="webchat"></main>
41+
<script type="module">
42+
import React, { createElement } from 'react';
43+
import { createRoot } from 'react-dom/client';
44+
45+
run(async function () {
46+
const {
47+
testHelpers: { createStore },
48+
WebChat: { ReactWebChat }
49+
} = window;
50+
51+
const { directLine, store } = testHelpers.createDirectLineEmulator();
52+
53+
// GIVEN: Web Chat is being rendered without <DebugProvider>.
54+
createRoot(document.getElementById('webchat')).render(createElement(ReactWebChat, { directLine, store }));
55+
56+
await pageConditions.uiConnected();
57+
58+
await directLine.emulateIncomingActivity({
59+
attachments: [
60+
{
61+
contentType: 'application/vnd.microsoft.card.adaptive',
62+
content: {
63+
// WHEN: We want to render a failing Adaptive Cards, adding "*" here to fail the renderer.
64+
type: '*AdaptiveCard*',
65+
$schema: 'http://adaptivecards.io/schemas/adaptive-card.json',
66+
version: '1.5',
67+
body: [
68+
{
69+
text: 'Hello, World!',
70+
type: 'TextBlock'
71+
}
72+
]
73+
}
74+
}
75+
],
76+
type: 'message'
77+
});
78+
79+
// THEN: Should have "render Adaptive Cards" error message.
80+
expect(console.warn).toHaveBeenCalledWith(
81+
expect.stringContaining('Failed to render Adaptive Cards.'),
82+
expect.anything()
83+
);
84+
85+
// THEN: Should not show RCoR error as we have injected catchall.
86+
expect(console.warn).not.toHaveBeenCalledWith(
87+
expect.stringContaining('the request has fall through all middleware, set "fallbackComponent" as a catchall'),
88+
expect.anything()
89+
);
90+
91+
// THEN: Should not show red box.
92+
await host.snapshot('local');
93+
});
94+
</script>
95+
</body>
96+
</html>

‎__tests__/html2/adaptiveCard/renderError.withoutDebug.html.snap-1.png‎

5.71 KB
Loading

‎packages/api-middleware/src/errorBoxPolymiddleware.tsx‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { validateProps } from '@msinternal/botframework-webchat-react-valibot';
2-
import React, { memo, useMemo } from 'react';
2+
import React, { memo, useCallback, useMemo } from 'react';
33
import { object, pipe, readonly, string, unknown, type InferInput } from 'valibot';
44

55
import templatePolymiddleware, {
@@ -18,7 +18,7 @@ const {
1818
Provider: ErrorBoxPolymiddlewareProvider,
1919
Proxy,
2020
reactComponent: errorBoxComponent,
21-
useBuildRenderCallback: useBuildRenderErrorBoxCallback
21+
useBuildRenderCallback
2222
} = templatePolymiddleware<{ readonly error: unknown; readonly where: string }, { readonly children?: never }>(
2323
'ErrorBox'
2424
);
@@ -41,15 +41,27 @@ const ErrorBoxPolymiddlewareProxyPropsSchema = pipe(
4141

4242
type ErrorBoxPolymiddlewareProxyProps = Readonly<InferInput<typeof ErrorBoxPolymiddlewareProxyPropsSchema>>;
4343

44+
// If no error box is defined, do not fallthrough into RCoR and it would error out. Render nothing instead.
45+
const NullComponent = () => null;
46+
4447
// A friendlier version than the organic <Proxy>.
4548
const ErrorBoxPolymiddlewareProxy = memo(function ErrorBoxPolymiddlewareProxy(props: ErrorBoxPolymiddlewareProxyProps) {
4649
const { error, where } = validateProps(ErrorBoxPolymiddlewareProxyPropsSchema, props);
4750

4851
const request = useMemo(() => ({ error, where }), [error, where]);
4952

50-
return <Proxy request={request} />;
53+
return <Proxy fallbackComponent={NullComponent} request={request} />;
5154
});
5255

56+
const useBuildRenderErrorBoxCallback: typeof useBuildRenderCallback = () => {
57+
const buildRenderCallback = useBuildRenderCallback();
58+
59+
return useCallback(
60+
(request, options) => buildRenderCallback(request, Object.freeze({ fallbackComponent: NullComponent, ...options })),
61+
[buildRenderCallback]
62+
);
63+
};
64+
5365
export {
5466
createErrorBoxPolymiddleware,
5567
errorBoxComponent,

0 commit comments

Comments
 (0)