- Issue created by @lauriii
- š§šŖBelgium wim leers Ghent š§šŖšŖšŗ
We don't need to improve error handling. We need to figure out how this is possible, given:
### 3.1 `SDC` `component`s #### 3.1.1 Criteria for `SDC` `component`s For an `SDC` to be compatible/eligible for use in XB, it: ā¦ - MUST have `example` for each required prop ā¦
š This means it should be impossible for the error in the screenshot to ever occur.
Which SDC did this happen for?
- š«š®Finland lauriii Finland
This is not an issue to fix the specific error. I opened š PHP warning on image props without examples Active for that. This issue is to improve the error handling, because I would not expect PHP errors from the small preview iframes to render in the preview canvas. I also would not expect errors to render inside the small preview iframes.
- š§šŖBelgium wim leers Ghent š§šŖšŖšŗ
Fair. But the title is super vague/misleading then. Fixed.
- š«š®Finland lauriii Finland
This is not so much about rendering PHP errors vs not. This was about where and how those errors should be rendered. For example, the error in the issue summary, is not even something that happens as part of the preview request, it happens on the component preview request. We should improve the error handling so that errors that happen during API requests, never end up in the preview.
- š§šŖBelgium wim leers Ghent š§šŖšŖšŗ
#6: Yep, that's how I interpreted #4, and what I captured in the updated issue title? š
That kind of super broad exception+error catching is something that I've seen only happen in "Drupal userland" one other time: in https://www.drupal.org/project/acquia_migrate ā , see:
- sort of https://git.drupalcode.org/project/acquia_migrate/-/commit/238a173d454ee...
- sort of https://git.drupalcode.org/project/acquia_migrate/-/commit/eb881eb8101bb...
- but especially https://git.drupalcode.org/project/acquia_migrate/-/commit/7931c1f843be9...
We should do a combination of both catching
\Exception
(i.e. ALL exceptions) and using PHP's low-levelset_error_handler()
inApiComponentsController
andApiPreviewController
(probably actually in the code that they call) ā those two combined would be able to achieve what you describe, @lauriii šIssue summary updated with outline of implementation based on your feedback.
WDYT? š
- š«š®Finland lauriii Finland
Proposal makes sense. Thank you @wim leers! šÆ
- š§šŖBelgium wim leers Ghent š§šŖšŖšŗ
Comments 11 through 19 of š Preview entire page not just content area Active are AFAICT essentially re-hashing this issue š š¬
- š¦šŗAustralia larowlan š¦šŗš.au GMT+10
Also FWIW I've seen errors break the whole page when using the auto-save controller and pushing invalid data - because SDC has a twig function to validate the component and that throws a RuntimeError that is uncaught.
I had a workaround in an earlier version of https://git.drupalcode.org/project/experience_builder/-/merge_requests/430 that turned this off so at least one invalid component didn't break the whole preview.
We may need to revive that here
- š§šŖBelgium wim leers Ghent š§šŖšŖšŗ
- Assigned to larowlan
- Status changed to Needs review
14 days ago 3:33pm 8 January 2025