Improve client side error handling

Adding some error boundaries would help here

I ran into an error with the new sidebar which ended up crashing the whole client side app. Another data point that we need some improvements in regards to this.

We did something similar for the AM:A React UI in https://git.drupalcode.org/project/acquia_migrate/-/commit/42d8cf15889c6... using https://react.dev/reference/react/Component#catching-rendering-errors-wi....

Having this should also make it easier to provide good bug reports, and should result in more meaningful test failures — especially after 📌 End-to-end test that tests both the client (UI) and server Active .

How is that going, @syeda-farheen? 😊

@wim-leers I am working on it, will update here asap

It seems there are actually three different aspects to this issue. So, for clarity, I see the breakdown as follows:

1. The backend failing to render the preview and thus the preview is just a white screen
2. Centralising the handling of any other error states returned when fetching from the backend/api e.g. when fetching the list of components
3. React client-side errors being thrown causing the entire app to not render

I'm not sure, but 1 and 2 may actually be able to be addressed by the same error handling.

3 should use as described in #4

@syeda-farheen It's been a week now, but still zero commits on the issue fork. Can you give us an update? 🙏

Sorry for the delayed update here, had paused on it due to health reason, un assigning it for now.

A whole slew of CI jobs are failing: eslint, stylelint, unit tests, E2E test … this is not in a reviewable state 😅

This needs tests as well. In Cypress Intercepting requests and returning junk should make it possible to trigger some errors for testing.

📌 Create an Open API spec for the current mock HTTP responses Fixed landed, which should mean fewer failures due to the server returning a non-compliant response. But … there can always be a network problem. So this definitely is still important!

I'm giving this a fresh start by creating our own reusable error boundary component on top of bvaughn/react-error-boundary which is even mentioned in the React docs. I'll have the first draft MR up shortly.

Tests are present now! 🚀 While this MR is still a draft, this could benefit from early reviews. Keeping assigned to @balintbrews to signal that he's continuing to work on this :)

I accidentally checked the logging placeholder task off the list yesterday. I've completed that just now. I did, however, run into a bunny hole with trying to unit test that the logging method is being called. Turns out mocking/stubbing/spying ES module imports is not solved within Cypress Component Testing. There is @cypress/vite-plugin-cypress-esm which attempts to solve this, but it's very experimental. I gave it a go, managed to merge it with our existing Vite config just for Cypress, then as a result, Vite stopped resolving file path aliases. It might have worked otherwise, but this is where I stopped trying to make this happen. So no unit testing for calling the logging method as of yet. Moving forward for the sake of progress.

Wim Leers → closed merge request !126

balintbrews → changed the visibility of the branch client-side-error-handling to hidden.

So great to have now :)

Next up:

1. ✨ Backend route to allow logging from the UI Active , then:
2. ✨ [PP-1] Log client-side errors Postponed

Automatically closed - issue fixed for 2 weeks with no activity.