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
- Put an @todo pointing to #2367555: Deprecate EnforcedResponseException support
- Modify the logger call to include the exception class
- Write tests
User interface changes
N/A
API changes
N/A
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2995893-exception-8-PASS.patch | 2.76 KB | tim.plunkett |
| #8 | 2995893-exception-8-FAIL.patch | 1.63 KB | tim.plunkett |
| #7 | 2995893-exception-7.patch | 1.13 KB | tim.plunkett |
| #2 | 2995893-2-layout-builder-chokes-on-form-exceptions.patch | 1.36 KB | dylan donkersgoed |
Comments
Comment #2
dylan donkersgoed commentedThe 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.
Comment #3
tim.plunkettUghhhhh 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?
Comment #4
tim.plunkettComment #5
tim.plunkettComment #7
tim.plunkettReroll. Still needs tests.
Comment #8
tim.plunkettTest added
Comment #9
phenaproxima@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.
Comment #14
phenaproximaLooks pretty committed! Thanks, all.
Comment #15
larowlanCommitted d9e9f0c and pushed to 8.8.x. Thanks!
c/p to 8.7