Problem/Motivation

What are the steps required to reproduce the bug?

These are the specific steps that caused it for me, but it likely could happen in other situations where a field display renders a form.

1. Set up a fresh install with layout builder and dblog enabled
2. Download and install the 8.x-3.x dev branch of the addtocal contrib module
3. Patch the module with the latest patch from the issue at https://www.drupal.org/project/addtocal/issues/2861672 (the patch from comment #4 at the time of this writing)
4. Create a content type and add a date range field with the layout configured to render that field using the addtocal formatter
5. Create an instance of your content type and view it
6. Use one of the addtocal links
7. Opening the link will redirect you to the current node, but without the addtocal field rendered
8. If you check the logs you'll see a warning like: The field "field_date" failed to render with the error of "".

What behavior were you expecting?

The field should render. In the case above the addtocal link should redirect to an external calender (e.g. via Google) or download a file.

What behavior were you expecting?

The field failed to render and a warning was logged.

Proposed resolution

Catch and rethrow the special EnforcedResponseException thrown by the Form API to allow form processing to continue.

Remaining tasks

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Comments

Dylan Donkersgoed created an issue. See original summary.

dylan donkersgoed’s picture

Status: Active » Needs review
StatusFileSize
new1.36 KB

The easiest way to fix this and the one with the least side effects seems to be to catch and rethrow the specific \Drupal\Core\Form\EnforcedResponseException that's causing the immediate issue. I'm attaching a patch that does that.

However, I don't know if this is the best solution. It seems like the blanket exception catch might cause other issues. I can't recall details but I believe I've seen exceptions used in similar ways elsewhere in Drupal. E.g. I've turned off my editor/xdebug's functionality for stopping on every exception because it will stop many times even when the request works fine, and not just on EnforcedResponseException exceptions.

tim.plunkett’s picture

Ughhhhh good find.
Ideally Drupal wouldn't use exceptions for code flow! But as long as we are, I think it's reasonable to include this.
Let's put an @todo in the code pointing to #2367555: Deprecate EnforcedResponseException support, documenting why we're special-casing that exception.

Additionally, we could modify that logger warning to include the exception class in addition to the message, that might help with debugging?

tim.plunkett’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs tests
tim.plunkett’s picture

Issue tags: +Blocks-Layouts

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

Status: Needs work » Needs review
StatusFileSize
new1.13 KB

Reroll. Still needs tests.

tim.plunkett’s picture

Issue tags: -Needs tests
StatusFileSize
new1.63 KB
new2.76 KB

Test added

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

@tim.plunkett kindly explained to me what the idea of EnforcedResponseException is. After doing a bit of research, I'm at least pleased to see that its existence inspires pretty much universal horror amongst those who routinely dig around in the guts of core. Maybe someday we'll triumphantly remove it. But until that day arrives, it exists and we therefore need to deal with it in whatever way is necessary.

So, that said, the patches here make sense to me, and the test is about as simple and straightforward as it could be. RTBC once the failing patch fails and the passing patch passes.

The last submitted patch, 8: 2995893-exception-8-FAIL.patch, failed testing. View results

  • larowlan committed d9e9f0c on 8.8.x
    Issue #2995893 by tim.plunkett, Dylan Donkersgoed: Layout builder chokes...

  • larowlan committed b2de97b on 8.7.x
    Issue #2995893 by tim.plunkett, Dylan Donkersgoed: Layout builder chokes...

phenaproxima’s picture

Status: Reviewed & tested by the community » Fixed

Looks pretty committed! Thanks, all.

larowlan’s picture

Version: 8.8.x-dev » 8.7.x-dev

Committed d9e9f0c and pushed to 8.8.x. Thanks!

c/p to 8.7

Status: Fixed » Closed (fixed)

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