Problem/Motivation
When layout builder renders a layout preview on entity view display defaults (not overrides), it creates an entity from sample data. This sample data can cause problems downstream in code that assumes the entity, or it's entity references, will actually exist and have IDs.
For example, if you have a taxonomy term reference field block configured to use "rendered entity" formatter, then template_preprocess_taxonomy_term
will run, which calls $term->toUrl()->toString();
, which throws an uncaught EntityMalformedException
because you cannot generate URL for an entity without an ID.
This problem was somewhat recognized in #2950892: Exceptions thrown during Layout Builder preview are not caught. The solution implemented there was to swallow any exception that occurs during the process used to build the render array for the field and just return a placeholder block. This solution does not work if the exception occurs during rendering though.
Proposed resolution
This is a tricky one. I think you'd need to force early rendering of the field block to bubble any exceptions up so that they can be swallowed too. I also think that it may be worth just disabling field block previews entirely when working on entity view display defaults? I'm going to try and write a patch for that so I can get past this blocker for my project.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#13 | 3069001-13.patch | 1.86 KB | andypost |
#2 | 3069001-force-placeholders-sample-data-field-blocks.patch | 1.78 KB | bkosborne |
Comments
Comment #2
bkosborneHmm, so I was initially thinking of just short-circuiting the field block output with this:
But that breaks the actual preview layout in core and would break layout builder for actual entities (non-default).
The real problem is the sample data, not the preview system.
This patch is a quick fix against 8.7.x that adds another property to the entity indicating it was constructed with sample data. FieldBlock backs out of rendering if it detects this. I'm sure it will break many tests.
Comment #3
bkosborneComment #4
bkosborneComment #5
sdmeyers CreditAttribution: sdmeyers as a volunteer commentedI have a similar (maybe) issue that was marked as a duplicate to this... (https://www.drupal.org/project/drupal/issues/3069019) . The patch above has no effect; however, in I use:
in place of the
in the patch. the error goes away.
To me this seem logical since the issue is that there is no id to begin with. What am I missing.
Comment #6
bkosborneYou may need to clear your tempstore, since the entity is cached there, and the "has_sample_data" property from the patch won't get set if the cache is warm. There's no easy way of doing that without just deleting all items in database table "key_value_expire" that have collection name "tempstore.shared.layout_builder.sample_entity"
Comment #9
AnybodyWe can completely confirm this issue. It's nice to have a preview functionality, but it makes layout builder a lot less robust and it tends to throw exceptions with a WSOD which a site owner can not fix himself without a developer!
So my suggestion would be to be a lot more defensive here and switch to simple block titles instead
a) in general by default and use the preview as enhancement, but not by default
or
b) if an exception occurs (at least for a) block
That would give at least site owners much more possibilities to fix errors themselves and resave the layout!
From my personal perspective it was not the right decision to always render the preview by default, if a robust label is enough for a site owner / developer.
A label would be
And with the option to show the preview, we'd keep UX.
In the last month we ran into problems like described in the IS so often, that we argued a lot about the maturity of layout builder... and it's mainly because of the preview.
Additonal point: We also had related problems because values in the tempstore seemed to be corrupted / wrong (missing entity dependency). That should simply result in broken LB block(s) and not a WSOD. And again the preview causes a WSOD / hard exception and (that's the point) prevents us from researching and fixing the problem in the layout builder UI.
Comment #10
Anybody@bkosborne: I had to think about the tempstore point... we all ran into a problem where tempstore caused conflicts and finally an exception. Perhaps it's worth to mention that in the issue summary and think about it in the solving strategy?
A regular site owner doesn't know about tempstore but if it breaks things (exception) he can't handle any more, it would perhaps also make sense to some kind of reset tempstore on such an exception or add any kind of "reset" or "abort" for that case?
Perhaps there are strategies for that in views which also save the current edit state until save?
I'm not sure yet and I'd very much appreciate, if a layout_builder expert like @tim.plunkett could share his opinion about the issue?
Comment #11
tim.plunkettI am of two minds on this.
On one hand, it is not nice to have other modules' bad assumptions break your site.
On the other, those formatters are broken. Preview is not a concept invented by Layout Builder, it is just more widely used. I'd just as soon let the contrib modules learn that they are broken, and fix them one by one.
Comment #12
AnybodyHi @tim.plunkett,
perhaps it doesn't have to be one or the other, as I tried to explain above. I'd suggest to use a way of "progressive enhancement" where the preview works on top, if everything is fine and the user enables it (or even by default). But based on the lightweight, technical text version. Currently it's the other way around, which makes it less stable, at least from my perspective and experience. I agree that these things should not happen, but they do and many times it locks is in a state where the overview is not accessible anymore, while for us at least, as technical site builders, the block names are even more helpful than the preview.
With progressive enhancement you could still access the configuration even if the front end / preview is broken for any reason.
Comment #13
andypostre-roll for 8.9.x
Comment #17
Amavi CreditAttribution: Amavi commentedSame problem on Drupal 9.3...
If I remove one Field on my content, Layout builder works, but some time after: The website encountered an unexpected error.
If I check "Allow each content item to have its own custom layout" Layout come back....
Comment #20
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedComment #21
tim.plunkettThere was a suggestion made for 8.9 but that isn't passing tests and may not be the agreed upon solution.
Comment #22
AnybodyI'd still appreciate feedback on my progressive enhancement (examples on top, to not break things and have lightweight labels by default) suggestion in #12 if anyone feels to have a valid opinion ;) :)
Comment #24
gcbI have to agree with @Anybody here. While it certainly sounds reasonable to ask all contrib modules to respond coherently in preview mode, it's not realistic: this is the perfect being the enemy of the good. If Drupal's standards for publishing modules were different, or if the feature sets offered by fully mature modules were more complete, this would be fine. It's not, and this issue has cost our clients a LOT of money, as well as frustration.
How hard would it be to catch exceptions and put something in the preview like, "preview failed for this field" below the field name in those cases? This would highlight contrib fields that needed attention without showing a whitescreen to users. It would also make it easier for developers to debug!
I have this whitescreen right now and it appears to be coming from a combination of entity references and the use of Tokens. Token is a pretty well-supported module... but there's a bug reported for it in the queue that points back to this issue, and the maintainer basically saying "sounds like a layout builder problem".
As a site developer, my options here are to stop using token, to hunt down the bug in token and submit a patch (which the module developer is convinced they don't need, so I may have to maintain it indefinitely), or to apply this funky patch to layout builder that sort of blows up the entire preview behavior anyway.
I've been through this with a dozen different modules at this point. As a result, my org is very unlikely to ever use layout builder on a new site again -- it just wastes too much time hunting these things down. I like layout builder, and I want to use it! Can't we just be a little bit more accepting of our imperfections and put some safety measures in place?