KibanaErrorBoundary initial implementation #168754

tsullivan · 2023-10-12T22:14:32Z

Summary

Meta issue: https://github.com/elastic/kibana-team/issues/646
This PR implements tasks in: Kibana React Error Boundary: Stage 1 #167159
Technical doc [Elastic internal]

This PR creates the ErrorBoundary component and its provider for services. It implements the wrapper around a few management apps owned by Appex-SharedUX.

Screenshots

Updated 2023-10-18

Server upgrade scenario: In this case, the caught error is known to be recoverable via window refresh:

Unknown/Custom error: In this case, the error is something outside of known cases where the fix is to refresh:

Testing

Use a script proxy in between the browser and the Kibana server.
- Try https://github.com/tsullivan/simple-node-proxy
- or https://chrome.google.com/webstore/detail/tweak-mock-and-modify-htt/feahianecghpnipmhphmfgmpdodhcapi.
Script the proxy to send 404 responses for the Reporting plugin bundle, and for a bundle of some Management app.
Try the Share > CSV menu in Discover. It should be blocked, and handled with a toast message. Buttons in the toast should work.
Try the SharedUX management apps that use the wrapper. It should be blocked, and handled with an EuiCallout. Refresh button and EuiAccordion should work.

Checklist

Ensure the package code is delivered to the browser in the initial loading of the page (c2559e8)
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

…fix'

vadimkibana

Overall looks very good 👍! Added few non-critical comments.

x-pack/plugins/reporting/public/management/mount_management_section.tsx

packages/shared-ux/error_boundary/src/services/error_boundary_services.tsx

vadimkibana · 2023-10-13T22:57:20Z

packages/shared-ux/error_boundary/src/services/error_boundary_services.tsx

+ * Kibana-specific Provider that maps dependencies to services.
+ */
+export const ErrorBoundaryKibanaProvider: FC = ({ children }) => {
+  const reloadWindow = () => window.location.reload();


Maybe this could be part of the errorService, so that we pass around only one object, like:

errorService.reload();

Then, at least for forceable future, we can simplify the value of our context to a single field—there is just one "service" object which manages all dependencies (in the future), and contains all the business logic for dealing with errors.

Not sure about this, it feels like just a bit of indirection.

In the future, I also plan to add a ToastsService to the mix of services. This will maintain the internal state about the toast objects that get sent to the EuiGlobalToastList.

packages/shared-ux/error_boundary/src/ui/error_messages.tsx

…fix'

elasticmachine · 2023-10-16T21:57:00Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

Dosant

I tested tags management (in Firefox), works good 👍

Dosant · 2023-10-19T08:32:14Z

packages/shared-ux/error_boundary/src/services/error_boundary_services.tsx

+ */
+export const KibanaErrorBoundaryProvider: FC = ({ children }) => {
+  // control side-effects of rendering with useMemo
+  const reloadWindow = useMemo(() => () => window.location.reload(), []);


this probably should be

Suggested change

const reloadWindow = useMemo(() => () => window.location.reload(), []);

const reloadWindow = useCallback(() => window.location.reload(), []);

But I am not sure that there is any benefit to it, since value below is always a new object and children will have to re-render. see https://legacy.reactjs.org/docs/context.html#caveats

realoadWindow is so tiny, it is probably more efficient to create a new function every re-render than to memoize it:

Suggested change

const reloadWindow = useMemo(() => () => window.location.reload(), []);

const reloadWindow = () => window.location.reload();

For the context provider value object to always have the same identity, so that other components don't re-render if this component re-renders, the context value could be a single object:

const value = useMemo(() => new KibanaErrorBoundaryServices(), []); return <Context.Provider value={value}>{children}</Context.Provider>;

Thank you for these suggestions!

Since KibanaErrorBoundaryServices is just an interface, I'll go with:

const value: KibanaErrorBoundaryServices = useMemo( () => ({ reloadWindow: () => window.location.reload(), errorService: new KibanaErrorService(), }), [] );

vadimkibana

LGTM 👍 Below are just a bunch of optional improvements.

packages/shared-ux/error_boundary/src/services/error_boundary_services.tsx

packages/shared-ux/error_boundary/src/ui/error_boundary.tsx

packages/shared-ux/error_boundary/src/services/error_service.ts

packages/shared-ux/error_boundary/src/ui/message_components.tsx

tsullivan · 2023-10-21T16:08:50Z

@elasticmachine merge upstream

kibana-ci · 2023-10-21T17:32:33Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/shared-ux-error-boundary`	-	1	+1
`@kbn/ui-shared-deps-src`	43	44	+1
total			+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`advancedSettings`	53.2KB	53.2KB	+75.0B
`aiops`	587.3KB	587.4KB	+74.0B
`dashboard`	375.8KB	375.9KB	+76.0B
`data`	48.4KB	48.5KB	+74.0B
`dataViewFieldEditor`	173.6KB	173.6KB	+1.0B
`dataViewManagement`	117.4KB	117.5KB	+101.0B
`dataVisualizer`	614.6KB	614.6KB	+74.0B
`discover`	584.7KB	584.8KB	+75.0B
`graph`	388.1KB	388.1KB	+75.0B
`kibanaReact`	209.4KB	209.4KB	+2.0B
`links`	25.3KB	25.4KB	+61.0B
`ml`	3.5MB	3.5MB	+97.0B
`observability`	1.1MB	1.1MB	+107.0B
`securitySolution`	13.0MB	13.0MB	+82.0B
`transform`	402.5KB	402.6KB	+73.0B
total			+1.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`advancedSettings`	9.5KB	9.6KB	+68.0B
`aiops`	8.0KB	8.1KB	+68.0B
`cloudChat`	13.1KB	13.2KB	+139.0B
`cloudLinks`	15.8KB	16.0KB	+139.0B
`core`	368.0KB	368.2KB	+137.0B
`dashboard`	35.9KB	36.0KB	+68.0B
`data`	407.4KB	407.5KB	+68.0B
`dataViewEditor`	12.8KB	12.9KB	+139.0B
`dataViewFieldEditor`	25.3KB	25.4KB	+140.0B
`dataViewManagement`	4.8KB	4.9KB	+68.0B
`dataVisualizer`	22.9KB	23.0KB	+68.0B
`discover`	31.1KB	31.1KB	+68.0B
`embeddable`	78.2KB	78.3KB	+141.0B
`graph`	8.0KB	8.1KB	+68.0B
`kbnUiSharedDeps-srcJs`	2.2MB	2.2MB	+5.2KB
`kibanaReact`	47.3KB	47.4KB	+141.0B
`links`	32.0KB	32.1KB	+68.0B
`management`	13.5KB	13.6KB	+139.0B
`ml`	76.0KB	76.0KB	+68.0B
`navigation`	11.7KB	11.8KB	+75.0B
`observability`	101.3KB	101.4KB	+68.0B
`observabilityLogExplorer`	30.9KB	31.0KB	+141.0B
`securitySolution`	63.2KB	63.3KB	+68.0B
`transform`	18.2KB	18.3KB	+68.0B
total			+7.3KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/shared-ux-error-boundary`	-	4	+4
`@kbn/ui-shared-deps-src`	52	53	+1
total			+5

ESLint disabled line counts

id	before	after	diff
`@kbn/shared-ux-error-boundary`	-	1	+1

Total ESLint disabled count

id	before	after	diff
`@kbn/shared-ux-error-boundary`	-	1	+1

History

💛 Build elastic/kibana#169760 was flaky 23c77fa
💛 Build elastic/kibana#169710 was flaky 238a361
💔 Build elastic/kibana#169708 failed 248576e
💔 Build elastic/kibana#169291 failed 251a1ce
💔 Build elastic/kibana#169260 failed f6f6c66
💔 Build elastic/kibana#169240 failed 5a17940

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Ikuni17

ops changes lgtm

sorenlouv · 2024-08-05T08:52:14Z

@tsullivan I'm running into a problem with this error boundary: when an error occurs during development, I want to see the stack trace and navigate to the file where the error was thrown. Afaict that is no longer possible, since the stack trace is not written to the console and the stack trace printed in the UI is not clickable (does not allow navigating to the file).

It is not easy to decipher the stack trace because it refers to "chunks" and not files. For instance, see this stack traces that's printed by the error boundary:

Error: Cannot set initialMessages if initialConversationId is set
    at useConversation (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/plugin/observabilityAIAssistantApp/1.0.0/observabilityAIAssistantApp.chunk.0.js:11554:11)
    at ChatBody (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/plugin/observabilityAIAssistantApp/1.0.0/observabilityAIAssistantApp.chunk.0.js:8112:86)
    at renderWithHooks (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:358087:18)
    at mountIndeterminateComponent (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:360913:13)
    at beginWork (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:362151:16)
    at HTMLUnknownElement.callCallback (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:347047:14)
    at Object.invokeGuardedCallbackDev (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:347096:16)
    at invokeGuardedCallback (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:347158:31)
    at beginWork$1 (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:367061:7)
    at performUnitOfWork (http://localhost:5601/fix/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:365873:12)

WDYT about printing the stack trace to the console, thereby re-enabling the ability to interact with it?

tsullivan · 2024-08-05T19:27:54Z

Afaict that is no longer possible, since the stack trace is not written to the console

@sorenlouv My apologies for the oversight! It sounds like you expected this logging: #189925

Log the error that was caught to the console, so developers can interact with the stack trace messages. Addresses #168754 (comment)

tsullivan force-pushed the sharedux/error-boundary-ii branch from 6798559 to 7ba611b Compare October 12, 2023 22:24

tsullivan mentioned this pull request Oct 12, 2023

POC: Kibana React Error Boundary #168054

Closed

8 tasks

tsullivan force-pushed the sharedux/error-boundary-ii branch from 7237345 to 87d5809 Compare October 13, 2023 05:47

tsullivan and others added 4 commits October 13, 2023 14:11

KibanaErrorBoundary with Fatal error implementation

d79d5c5

Reporting management app error boundary

3f6d7bd

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

8ae90f5

…fix'

implement "soft" error message

83bb621

tsullivan force-pushed the sharedux/error-boundary-ii branch from f19fbcf to 83bb621 Compare October 13, 2023 22:08

vadimkibana reviewed Oct 13, 2023

View reviewed changes

side effects should not appear directly inside the body of a component

5d407bd

tsullivan added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) ci:build-storybooks labels Oct 14, 2023

tsullivan and others added 7 commits October 15, 2023 12:40

reset reporting

4700a45

Reference implementation for SharedUX management apps

bda5264

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

957ee1b

…fix'

sync with code changes from poc branch

45d8560

Merge branch 'main' into sharedux/error-boundary-ii

d4a4bda

pr cleanup

1f8e327

re-fix useMemo

e8ebb2d

tsullivan force-pushed the sharedux/error-boundary-ii branch from 7a7f067 to e8ebb2d Compare October 16, 2023 03:10

tsullivan mentioned this pull request Oct 16, 2023

Kibana React Error Boundary: Stage 1 #167159

Closed

15 tasks

tsullivan added the release_note:skip Skip the PR/issue when compiling release notes label Oct 16, 2023

tsullivan added 2 commits October 16, 2023 12:07

Merge branch 'main' into sharedux/error-boundary-ii

ba420c5

polish

dc15a23

tsullivan marked this pull request as ready for review October 16, 2023 21:56

tsullivan requested a review from a team as a code owner October 16, 2023 21:56

tsullivan added 3 commits October 16, 2023 15:05

change user-facing text per feedback.

cf81e93

Add jest unit testing

8784b20

Ensure the ErrorBoundary package is part of the shared-deps bundle

c2559e8

tsullivan added 2 commits October 18, 2023 21:11

fix jest test

3f88017

Merge branch 'main' into sharedux/error-boundary-ii

251a1ce

Dosant approved these changes Oct 19, 2023

View reviewed changes

vadimkibana approved these changes Oct 19, 2023

View reviewed changes

tsullivan added 9 commits October 19, 2023 11:29

Merge branch 'main' into sharedux/error-boundary-ii

a5ce5ff

Context value is a single object that never changes

0287190

Services as a distinct prop to the internal component

cb43013

use explicit type for React component

438742b

use React event naming

94fc6ee

Merge branch 'main' into sharedux/error-boundary-ii

248576e

fix ts error

238a361

unit test for KibanaErrorService

70895c5

Merge branch 'main' into sharedux/error-boundary-ii

23c77fa

tsullivan enabled auto-merge (squash) October 20, 2023 20:47

Merge branch 'main' into sharedux/error-boundary-ii

fbdadcd

tsullivan disabled auto-merge October 21, 2023 19:39

tsullivan requested a review from a team October 21, 2023 23:29

Ikuni17 approved these changes Oct 23, 2023

View reviewed changes

tsullivan merged commit 53c83e7 into elastic:main Oct 23, 2023

kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 23, 2023

tsullivan self-assigned this Oct 23, 2023

tsullivan mentioned this pull request Nov 21, 2023

Better Short URL missing link redirection error #169935

Closed

tsullivan deleted the sharedux/error-boundary-ii branch August 5, 2024 19:05

tsullivan mentioned this pull request Aug 5, 2024

[KibanaErrorBoundary] Log the error that was caught #189925

Merged

tsullivan added a commit that referenced this pull request Aug 6, 2024

[KibanaErrorBoundary] Log the error that was caught (#189925)

6b4f613

Log the error that was caught to the console, so developers can interact with the stack trace messages. Addresses #168754 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KibanaErrorBoundary initial implementation #168754

KibanaErrorBoundary initial implementation #168754

tsullivan commented Oct 12, 2023 •

edited

Loading

vadimkibana left a comment •

edited

Loading

vadimkibana Oct 13, 2023 •

edited

Loading

tsullivan Oct 16, 2023

elasticmachine commented Oct 16, 2023

Dosant left a comment

Dosant Oct 19, 2023

vadimkibana Oct 19, 2023

tsullivan Oct 19, 2023

vadimkibana left a comment

tsullivan commented Oct 21, 2023

kibana-ci commented Oct 21, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

Ikuni17 left a comment

sorenlouv commented Aug 5, 2024 •

edited

Loading

tsullivan commented Aug 5, 2024

	const reloadWindow = useMemo(() => () => window.location.reload(), []);
	const reloadWindow = useCallback(() => window.location.reload(), []);

	const reloadWindow = useMemo(() => () => window.location.reload(), []);
	const reloadWindow = () => window.location.reload();

KibanaErrorBoundary initial implementation #168754

KibanaErrorBoundary initial implementation #168754

Conversation

tsullivan commented Oct 12, 2023 • edited Loading

Summary

Screenshots

Testing

Checklist

vadimkibana left a comment • edited Loading

Choose a reason for hiding this comment

vadimkibana Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

tsullivan Oct 16, 2023

Choose a reason for hiding this comment

elasticmachine commented Oct 16, 2023

Dosant left a comment

Choose a reason for hiding this comment

Dosant Oct 19, 2023

Choose a reason for hiding this comment

vadimkibana Oct 19, 2023

Choose a reason for hiding this comment

tsullivan Oct 19, 2023

Choose a reason for hiding this comment

vadimkibana left a comment

Choose a reason for hiding this comment

tsullivan commented Oct 21, 2023

kibana-ci commented Oct 21, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

Ikuni17 left a comment

Choose a reason for hiding this comment

sorenlouv commented Aug 5, 2024 • edited Loading

tsullivan commented Aug 5, 2024

tsullivan commented Oct 12, 2023 •

edited

Loading

vadimkibana left a comment •

edited

Loading

vadimkibana Oct 13, 2023 •

edited

Loading

sorenlouv commented Aug 5, 2024 •

edited

Loading