- Issue created by @balintbrews
- Status changed to Postponed
3 months ago 7:06am 19 August 2024 - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
📌 Improve client side error handling Fixed is in, but without ✨ Backend route to allow logging from the UI Active , this cannot be implemented.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
✨ Backend route to allow logging from the UI Active landed!
- First commit to issue fork.
- 🇮🇳India omkar-pd
Started implementing the logApi service.
Currently, I’ve set up basic logging functionality with a hardcoded level: 'error'. However, I need further guidance on determining the appropriate error level dynamically. - 🇳🇱Netherlands balintbrews Amsterdam, NL
The ability to recover from an issue is what I would consider the main factor when it comes to deciding severity. The error boundaries can catch all kinds of problems, and we can't really predict with full certainty when the UI will be able to recover from those. Therefore I recommend that we go with the level
error
by default.Some errors are thrown, or could be thrown, explicitly by our own code, and while we could implement our own class extending
Error
and add the severity on a case-by-base basis, I don't see a lot of value in that. We wouldn't be able to make it very granular anyway.I would make an exception with the
RouteErrorBoundary
andRouteAsyncErrorBoundary
components. When those components are rendered, it's fully certain that there is no recovery other than reloading the page. I would set thecritical
level for those.I'm curious what others think.
- Assigned to soaratul
- Status changed to Needs work
22 days ago 11:05am 18 October 2024 - First commit to issue fork.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
To my non-React-trained eyes, this looks great! 😄
But I'm not seeing any test coverage yet 😅
Assigning to @balintbrews for review, but I do think this would benefit from an end-to-end test.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
The
UI eslint
CI job is not passing … https://git.drupalcode.org/project/experience_builder/-/pipelines/324156 - 🇳🇱Netherlands balintbrews Amsterdam, NL
This is looking great! I requested two code changes. Besides that, as Wim already pointed out correctly in #13, we still need to provide some test coverage for the logging that was added in this MR.
Ideally we would do that by mocking our `log` module in our existing `ui/tests/unit/error-boundary.cy.jsx` test, but mocking modules in Cypress component tests is problematic, and given we have the ongoing discussion of 📌 Consider dropping Nightwatch in favor of Functional Javascript tests Active , which may result in changing the testing framework of XB in the future, I don't think we should try to solve it. We can still verify the logging in an end-to-end test, there is already one in the repository where we intercept a request and mock an error form the server (`Handles and resets errors` in `ui/tests/e2e/xb-general.cy.js`). That test can be extended to intercept and verify the request sent to `/xb/api/log-error` when the error boundary is rendered.