Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
Problem/Motivation
- Enable rest
- Enable serialize
- Access http://d8.dev/node/3?_format=banana
Result:
The website encountered an unexpected error. Please try again later.undefined</br>undefined</br>undefined<em class="placeholder">Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException</em>: Not acceptable in undefined<em class="placeholder">Drupal\Core\EventSubscriber\AcceptNegotiation406->onViewDetect406()</em> (line undefined<em class="placeholder">37</em> of undefined<em class="placeholder">core/lib/Drupal/Core/EventSubscriber/AcceptNegotiation406.php</em>). undefined<pre class="backtrace">Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->fetch(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->lookup(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1)
Stack\StackedHttpKernel->handle(Object, 1, 1)
Drupal\Core\DrupalKernel->handle(Object)
</pre>
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#28 | DefaultExceptionSubscriber_php_-_www_-____Sites_drupal_d8_www_.png | 35.08 KB | clemens.tolboom |
#26 | interdiff-18-26.txt | 1.85 KB | therealssj |
#26 | 2594777-26.patch | 3.66 KB | therealssj |
#18 | interdiff.txt | 1.9 KB | Wim Leers |
#18 | 2594777-18.patch | 3.63 KB | Wim Leers |
Comments
Comment #2
dawehnerComment #3
clemens.tolboom[edit]
It's even worse.Visiting http://drupal.d8/node/1?_format=whatever throws the same exception. (doh)[/edit]There should not be a PHP exception induced by bad formats.
Comment #4
damiankloip CreditAttribution: damiankloip commentedThis returns a 406 response for me, which I think is fine. I just think the problem might be that we have the trace there?
Comment #5
damiankloip CreditAttribution: damiankloip commentedSo the problem is that DefaultExceptionSubscriber is handling this I guess. Because the exception implements HttpExceptionInterface the correct response code is set, but the trace is always added to the response content.
Comment #6
damiankloip CreditAttribution: damiankloip commentedIt's rough, but maybe we need to do something like this for formats we don't know about?
Comment #7
dawehnerComment #8
Wim LeersNR for #6.
Comment #9
dawehnerThis is looking great but I guess we better have some kind of integration test coverage
Comment #10
dawehnerWorking on the tests now.
Comment #11
dawehnerHere is one. Sadly shit actually just works if you test it.
Comment #12
dawehnerHere is the test.
Comment #13
dawehnerBeing able to trigger 500s easily on any page is IMHO major for me.
Comment #14
Wim LeersComment #15
damiankloip CreditAttribution: damiankloip commentedSorry, small thing.
I guess we need to change that comment now too.
Otherwise, looks like some nice changes to my patch. Thanks! After that, agree it is RTBC.
Comment #16
damiankloip CreditAttribution: damiankloip commentedAlso, as with the example in the summary. It is not immediately clear that any request for a non-existent format fails. hal_json *can* exist. E.g. the same outcome would occur with a request like http://d8-dev/node/1?_format=bananas Not sure if the tests should reflect that too? As it's easy to get confused as to why hal_json wouldn't be a valid format IMO.
Comment #17
Wim Leers+1 for #16.
I'll address the feedback in a reroll.
Comment #18
Wim LeersComment #19
dawehner+1. Works for me
Comment #20
dawehnerComment #22
catchCommitted/pushed to 8.1.x, thanks!
For 8.0.x, we probably need to explicitly @internal the new method and underscore prefix it, so moving to 'to be ported' for that.
Comment #23
therealssj CreditAttribution: therealssj commentedA Patch for 8.0.x in accordance with changes mentioned in #22
Comment #24
therealssj CreditAttribution: therealssj commentedComment #26
therealssj CreditAttribution: therealssj commentedPatch for 8.x
Tested.
Comment #27
dawehnerWell, I'm not sure whether we still care about 8.0.x at that point, but sure, this change looks right.
Comment #28
clemens.tolboomThis does not work for 8.2.x as I still get a 500
The exception is definitely not
Comment #29
therealssj CreditAttribution: therealssj commentedI just tried to reproduce this is 8.2.x and i don't see any issue.
@clemens.tolboom could you tell the steps you followed to get that 500?
Comment #30
dawehner@clemens.tolboom
Please open up a new issue ...
Comment #31
clemens.tolboomSorry for wasting your time. I had a view not liking the format. That's another issue (I hope)
Comment #32
alexpott8.0.x has entered it's last RC so this is no longer eligible for commit.