Fosrestbundle: Error 500 on Exception mapper

[jawira]

Hello, I'm using the exception mapper to catch exceptions and return them in _json_ format.

The problem is exception code is only mapped into _http header_, but not in the _json response_.

This is the response I got when my app threw a ValidationServiceException:

This is my config:

# Read the documentation: https://symfony.com/doc/master/bundles/FOSRestBundle/index.html
fos_rest:
  param_fetcher_listener:  true
  #  allowed_methods_listener:  true
  #  routing_loader: true
  view:
    view_response_listener:  true
  exception:
    enabled: true
    codes:
      'App\Service\ValidationServiceException': 412
    messages:
      'App\Service\ValidationServiceException': true
    debug: true
    map_exception_codes: true
    flatten_exception_format: 'legacy'
  body_listener: true
  format_listener:
    rules:
      - { path: ^/api/v1, prefer_extension: true, fallback_format: json, priorities: [ json ] }
      # https://github.com/FriendsOfSymfony/FOSRestBundle/issues/631#issuecomment-30321824
      - { path: ^/, prefer_extension: true, fallback_format: ~, priorities: [ html ] }
  disable_csrf_role: ROLE_API

Please note I'm using map_exception_codes and flatten_exception_format options. Also I'm using FosRestBundle v3.0.0.

Could you help me please? I spend all my afternoon with this issue, the sole documentation I found is the one for v2 in Symfony's website.

[olix21]

I have the same issue. And I guess it's not supported:

The map_exception_codes will enable the ResponseStatusCodeListener that will change the response status

$statusCode = $this->exceptionValueMap->resolveFromClassName(get_class($event->getThrowable()));

But when the exception is normalized (FlattenExceptionHandler::convertToArray())

if ($context->hasAttribute('status_code')) {
     $statusCode = $context->getAttribute('status_code');
} else {
      $statusCode = $exception->getStatusCode();
}

So if a status_code is defined in the context, it will be using it, if not, it takes the exception message.
And when the context is created (SerializerErrorRenderer::render()):

$context = new Context();
$context->setAttribute('exception', $exception);
$context->setAttribute('debug', is_callable($this->debug) ? ($this->debug)($exception) : $this->debug);

$headers = [
    'Content-Type' => Request::getMimeTypes($format)[0] ?? $format,
    'Vary' => 'Accept',
];

return $flattenException->setAsString($this->serializer->serialize($flattenException, $format, $context))->setHeaders($flattenException->getHeaders() + $headers);

So I think the status_code will never be set. And we'll never go in the if statement in convertToArray

Or am I missing something?

[xabbuh]

Did this work in 2.7.x? Does it only fail in 2.8/3.0? And if so could anyone of you please create a small example application that allows to reproduce?

[olix21]

It was working on 2.7.x and 2.8. It's no more working in 3.0. I'll try to create a small project to reproduce the issue.

[olix21]

@xabbuh here is a reproducer: https://github.com/olix21/fosrestv3-reproducer

I did a small controller throwing a 403. This 403 should be rewritten as a 404.
The response status code is mapped correctly, but the body still mention a 403:

you can run bin/phpunit to see the result

[olix21]

@xabbuh As I said, I can work on this one, I'm just not sure where to put the mapping. Should it be part of the serialization context or should we inject the exceptionValueMap service into the normalizer/serializer

[xabbuh]

I think we should investigate why this was working in 2.8 but broke in 3.0. Maybe we are simply missing to pass some config values to a service because of updated method signatures.

[xabbuh]

maybe the fix can be as simple as #2220 (I mean the reason for the failure can be equally trivial)

[olix21]

I don't think it's the issue. I think it's related to the deprecated ExceptionController. The conversion of the status code was made there

private function getStatusCodeFromThrowable(\Throwable $exception): int
{
    // If matched
    if ($statusCode = $this->exceptionCodes->resolveThrowable($exception)) {
        return $statusCode;
    }

    // Otherwise, default
    if ($exception instanceof HttpExceptionInterface) {
        return $exception->getStatusCode();
    }

    return 500;
}

And the deprecated ExceptionHandler was reading the template data to evaluate the serialized code

if ($context->hasAttribute('template_data')) {
    $templateData = $context->getAttribute('template_data');

    if (array_key_exists('status_code', $templateData)) {
        $data['code'] = $statusCode = $templateData['status_code'];
    } elseif ($context->hasAttribute('status_code')) {
        $data['code'] = $context->getAttribute('status_code');
    }
} elseif ($context->hasAttribute('status_code')) {
    $data['code'] = $context->getAttribute('status_code');
}

Using the new FlattenExceptionHandler, it seems that the logic is also there:

if ($context->hasAttribute('status_code')) {
    $statusCode = $context->getAttribute('status_code');
} else {
    $statusCode = $exception->getStatusCode();
}

But it seems that there is no class in charge of setting the status_code. The SerializerErrorRenderer set the context in render but does not handle the status_code.
I guess that's the missing part. We should inject the ExceptionValueMap in the SerializerErrorRenderer and use it to map the status code.

I can do the change if you confirm that's the good way to do it @xabbuh

[xabbuh]

@olix21 Thank you for your investigations. I thought about your suggestion a bit and also played with the reproducer application. But I think patching the SerializerErrorRenderer is not the right fix as that would not fix the issue for anyone not using our own SerializerErrorRenderer but using the Symfony one. Instead I think we should fix the serializer handler and normalizer. Can you confirm that #2253 fixes it?

[olix21]

I can confirm it's okay with your fix! Thank you very much 😄

[xabbuh]

Everything is merged up, requiring ^2.8.2@dev/^3.0.2@dev should fix your apps until the next patch release.