A Drupal 8 AjaxResponse that must return a 403, returns a 403 and prints "A fatal error occurred: "

"A fatal error occurred" sounds like an actual error happened somewhere and it's being badly reported. That suggests an issue in the error handling system to me.

I know Symfony and Silex have very good debug output in case of uncaught exceptions. Symfony also has a nice debug toolbar. We should perhaps look into both of them and see if there's code we can borrow, or at least concepts.

Drupal\Core\Controller\ExceptionController

  public function execute(FlattenException $exception, Request $request) {
    $method = 'on' . $exception->getStatusCode() . $this->negotiation->getContentType($request);

    if (method_exists($this, $method)) {
      return $this->$method($exception, $request);
    }

    return new Response('A fatal error occurred: ' . $exception->getMessage(), $exception->getStatusCode(), $exception->getHeaders());
  }

The method it's looking for is "on403drupal_ajax", which is a rather weird method name :) Maybe link that to Json on that level? We do expect json, right?

That whole thing seems quite un-extendable? What would have have to do if I want to return XML, or actually, request XML and get a 403 or 404?

So, how is Drupal\Core\EventSubscriber\ViewSubscriber supposed to work? It has similar code:

      $method = 'on' . $this->negotiation->getContentType($request);

      if (method_exists($this, $method)) {
        $event->setResponse($this->$method($event));
      }
      else {
        $event->setResponse(new Response('Not Acceptable', 406));
      }

... but defines a onAjax() but no ondrupal_ajax(). I suppose we are not actually using the correct mimetype yet?

I echo Berdir's sentiment: we are missing a proper rendering layer. Symfony Full Stack doesn't have a proper rendering layer either, but at least it can guess templates based on the format of the incoming request (as implemented in Sensio\Bundle\FrameworkExtraBundle\EventListener\TemplateListener.

Also, is there any reason we are not using the proper Request::getRequestFormat() / Request::setRequestFormat() instead of hardwiring our content negotiation service everywhere?

Damien: We have an open issue with no code yet to standardize on getRequestFormat() properly. You're correct that we should be.

Also, we're discussing in the WSCCI channel as we speak about how ViewSubscriber needs to die a painful death. :-) It was never intended to live this long; refactoring HtmlPageController and ViewSubscriber (which are quite muddled right now) is something I've been thinking about the past few days. ExceptionController is equally hackish. It's out of scope for this issue to rewrite, but I want to try rewriting them.

@Crell: Is there any specific reason why we are wiring all this in "wrapping" controllers, instead of using the KernelEvents::VIEW event like Symfony full stack does (and like ViewSubscriber currently does)?

Also, the default Symfony\Component\HttpKernel\EventListener\ExceptionListener handles exceptions as sub-requests to a special controller, which makes a ton of sense to me too. But of course, that requires separating rendering from the controllers, and brings us back to the previous point.

Damien: HtmlPageController was written with the expectation that it would get replaced by SCOTCH, but that's not happened. ViewSubscriber was written 7 months earlier with the expectation that it would get replaced as well, by SCOTCH or otherwise. Refactoring both of those so that we're using the view event properly, with a classed object, is exactly what I want to do. That's tangentially related to this issue but would likely overlap with it. (I'm waiting for the title issue to get in so that I'm not editing the exact same lines at the same time.)

added related issues

What's the state here? This is still a problem and makes debugging ajax requests very painfull. It's not just a problem for 404/403 but also real errors where right now, ajax actions just silently fail.

I believe #2323759: Modularize kernel exception handling will allow us to address this issue.

So

So #2323759: Modularize kernel exception handling is in so active now?

Can we get a failing test here to confirm it's still an issue and wasn't fixed implicitly as part of the exception refactor? If it's still an issue we need a test anyway but that should make it much easier to fix.

Let's see whether this works. On top of that I wonder whether we this will cause issues in ie9, see #2339491: Ajax requests on forms with files fail on IE 9

At least one of the failures of the quick test ones, will be green.

Meh. Not sure whether {} or [] should be the prefered way when there is no text. It seems to be that symfony decided to go with the first one, for some reason? Now this
patch uses "[]" but we could easily switch.

Remember, it's a JSON object. A bare [] isn't valid JSON. A bare {} is a valid JSON null object.

Maybe now.

There we go.

This patch is looking good to me. Just one thing IMO, we should return {"error": "Some fatal error message here"} instead of just ["Some fatal error message here"]. This keeps things more consistent but also allows you to get this with response.error and not response[0]

+++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php
@@ -171,15 +172,12 @@ protected function onJson(GetResponseForExceptionEvent $event) {
-    // @todo We would prefer to use JsonResponse here, but existing code and
-    // tests are not prepared for parsing a JSON response when there are quotes
-    // or other values that would cause escaping issues. Instead, return a
-    // plain string and mark it as such.
-    $response = new Response($message, Response::HTTP_INTERNAL_SERVER_ERROR, [
-      'Content-type' => 'text/plain'
-    ]);

And we even get to fix that thing, too. Nice!

Committed/pushed to 8.0.x, thanks!