Skip to content

Commit 4c60bbe

Browse files
compulimcompulim
authored
Fix polymiddleware error should not propagate to React runtime (#5833)
* Fix polymiddleware error should not propagate to React runtime * Add PR number * Fix flakiness * Add tests
1 parent c138220 commit 4c60bbe

13 files changed

Lines changed: 254 additions & 5 deletions

‎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 [#5833](https://github.com/microsoft/BotFramework-WebChat/pull/5833)
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

‎__tests__/html2/middleware/errorBox/proxy.html‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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+
"jest-mock": "https://esm.sh/jest-mock",
24+
"react": "https://esm.sh/react@18.3.1",
25+
"react-dom": "https://esm.sh/react-dom@18.3.1",
26+
"react-dom/": "https://esm.sh/react-dom@18.3.1/"
27+
}
28+
}
29+
</script>
30+
<script crossorigin="anonymous" src="/test-harness.js"></script>
31+
<script crossorigin="anonymous" src="/test-page-object.js"></script>
32+
<script type="module">
33+
import React from 'react';
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+
<style type="text/css">
39+
.webchat > .error-box {
40+
border: solid 1px red;
41+
}
42+
</style>
43+
</head>
44+
<body>
45+
<main id="webchat"></main>
46+
<script type="module">
47+
import { fn, spyOn } from 'jest-mock';
48+
import { createElement, Fragment, memo } from 'react';
49+
import { createRoot } from 'react-dom/client';
50+
51+
run(async function () {
52+
const {
53+
testHelpers: { createDirectLineEmulator },
54+
WebChat: {
55+
Components: { Composer, ThemeProvider },
56+
DebugProvider,
57+
middleware: { ErrorBoxPolymiddlewareProxy },
58+
ReactWebChat
59+
}
60+
} = window;
61+
62+
const { directLine, store } = createDirectLineEmulator();
63+
64+
const SystemUnderTest = () =>
65+
createElement(
66+
'div',
67+
{ className: 'error-box' },
68+
createElement(ErrorBoxPolymiddlewareProxy, { error: new Error('Hello, World!'), where: 'Artificial' })
69+
);
70+
71+
const hasDebugProvider = !new URL(location.href).searchParams.has('no-debug-provider');
72+
const onTelemetry = fn();
73+
74+
createRoot(document.getElementById('webchat')).render(
75+
createElement(
76+
hasDebugProvider ? DebugProvider : Fragment,
77+
{},
78+
createElement(
79+
Composer,
80+
{
81+
directLine,
82+
onTelemetry,
83+
store
84+
},
85+
createElement(SystemUnderTest)
86+
)
87+
)
88+
);
89+
90+
await pageConditions.uiConnected();
91+
92+
// THEN: Should emit as exception event.
93+
const exceptionEvent = onTelemetry.mock.calls.find(call => call[0]?.type === 'exception');
94+
95+
expect(exceptionEvent).toBeTruthy();
96+
expect(exceptionEvent[0].error.message).toBe('Hello, World!');
97+
98+
// THEN: Should render a red box with DebugProvider.
99+
await host.snapshot('local');
100+
101+
// THEN: Should not warn with RCoR fallthrough.
102+
expect(console.warn).not.toHaveBeenCalledWith(
103+
expect.stringContaining('the request has fall through all middleware, set "fallbackComponent" as a catchall'),
104+
expect.anything()
105+
);
106+
});
107+
</script>
108+
</body>
109+
</html>

__tests__/html2/middleware/errorBox/simple.html.snap-1.png renamed to __tests__/html2/middleware/errorBox/proxy.html.snap-1.png

File renamed without changes.

__tests__/html2/middleware/errorBox/simple.noDebugProvider.html renamed to __tests__/html2/middleware/errorBox/proxy.noDebugProvider.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<html lang="en-US">
33
<head>
44
<script>
5-
location.href = './simple?no-debug-provider';
5+
location.href = './proxy?no-debug-provider';
66
</script>
77
</head>
88
<body></body>

‎__tests__/html2/middleware/errorBox/proxy.noDebugProvider.html.snap-1.png‎

1.97 KB
Loading

__tests__/html2/middleware/errorBox/simple.html renamed to __tests__/html2/middleware/errorBox/useBuildRenderCallback.html

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,21 @@
22
<html lang="en-US">
33
<head>
44
<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>
520
<script type="importmap">
621
{
722
"imports": {
@@ -39,7 +54,7 @@
3954
WebChat: {
4055
Components: { Composer, ThemeProvider },
4156
DebugProvider,
42-
middleware: { createErrorBoxPolymiddleware, errorBoxComponent, useBuildRenderErrorBoxCallback },
57+
middleware: { useBuildRenderErrorBoxCallback },
4358
ReactWebChat
4459
}
4560
} = window;
@@ -83,6 +98,12 @@
8398

8499
// THEN: Should render a red box with DebugProvider.
85100
await host.snapshot('local');
101+
102+
// THEN: Should not warn with RCoR fallthrough.
103+
expect(console.warn).not.toHaveBeenCalledWith(
104+
expect.stringContaining('the request has fall through all middleware, set "fallbackComponent" as a catchall'),
105+
expect.anything()
106+
);
86107
});
87108
</script>
88109
</body>

‎__tests__/html2/middleware/errorBox/useBuildRenderCallback.html.snap-1.png‎

5.36 KB
Loading

‎__tests__/html2/middleware/errorBox/useBuildRenderCallback.noDebugProvider.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!doctype html>
2+
<html lang="en-US">
3+
<head>
4+
<script>
5+
location.href = './useBuildRenderCallback?no-debug-provider';
6+
</script>
7+
</head>
8+
<body></body>
9+
</html>

0 commit comments

Comments
 (0)