core: Error handling with 3.2 is inconsistent & difficult

Description
With 3.2 there are two different ways in which error responses are created:

  1. When the problem occurs while routing or authentication, Symfony’s own ProblemNormalizer is used.
  2. When the problem occurs within ApiPlatforms Processors/Providers an ApiResource\Error is created and serialized. (At least when using no deprecated features / the new problem responses)

This leads to multiple problems:

  • returned content + headers change for similar errors / same status codes: see #5961
  • We can no longer decorate ‘api_platform.hydra.normalizer.error’ to modify all returned error responses, e.g. to translate the returned error ‘detail’ / ‘hydra:description’.
    • instead we have to create a new Processor to decorate ‘api_platform.state_processor.serialize’ to modify the ApiResource\Error error before it is serialized
    • and modify our existing serializer to instead decorate ‘serializer.normalizer.problem’ to do the same thing but on an array
  • ApiResource\Error is immutable, which requires us to create a new instance to modify a single value, e.g. the ‘detail’
    • Why are there no setters, and if it is required to be immutable, can we please get ->with...() clone methods like most of the ApiPlatform\OpenApi\Model classes have (withParameters, withTrace, withDescription, …)? Also the ORM|ODM|ElasticSearch\State\Options or the MetaData class(es) have them.

The feature request would be to simplify the modification of the error results, for all error sources together and to better document how this can be done. Currently the docs (https://api-platform.com/docs/core/errors/) neither shows how to use a Provider nor a Serializer to modify the response, and it does not mention the (current) differences in error handling & output.

Example
We want to translate the error messages generated by Symfony’s routing / Lexik JWT / Validators / custom exceptions to be parseable by the API-Client. Currently we have to use:

    $clone = new Error(
        $data->getTitle(),
        $this->translator->trans($data->getDetail(), [], 'validators'),
        $data->getStatus(),
        $data->originalTrace,
        $data->getInstance(),
        $data->getType(),
        $data->getHeaders(),
    );

and

        $result['detail'] = $this->translator
            ->trans($result['detail'], [], 'validators');

        // currently not available because of #5961 
        //$result['hydra:description'] = $this->translator
        //    ->trans($result['[detail](hydra:description)'], [], 'validators');

which also causes the Error constructor to iterate over the whole stacktrace again to remove the (already removed) args. (yes, we could first provide an empty array as trace and later simply set $data->originalTrace, see below)

Additional context

    defaults:
        extra_properties:
          rfc_7807_compliant_errors: true
          skip_deprecated_exception_normalizers: true
    event_listeners_backward_compatibility_layer: false
    keep_legacy_inflector: false

Additional questions / feedback

  • Why is (only) the originalTrace public in ApiResource\Error? What was the reason to not use a getter like with all other properties and an additional setter?
  • What is the reason behind making all properties private instead of protected? This prevents extending the Error with a custom subclass as it cannot access those props. It is neither marked final nor internal (which is a good thing imho).
    • This is a theme I see often in ApiPlatform (but also in Symfony in newer history): Classes are marked final or most/all properties and methods are private which prevents inheritance, which often causes repetition as we only want to change a small portion of the code but need to copy everything else as it cannot be re-used.

About this issue

  • Original URL
  • State: closed
  • Created 8 months ago
  • Reactions: 4
  • Comments: 33 (25 by maintainers)

Most upvoted comments

well it took quite some time but I’m close to something better which handles a proper bc layer and also makes that 3.2 system actually usable (it’s not if you can’t customize exceptions). It should be out soon I need to try a few more things manually. In my patch I reverted everything to the previous error handling when the flag is false and I added a way to customize exceptions using true!

Really sorry for the delay, I had to work (a lot) on the documentation as we had issues using Next.js and therefore we couldn’t update the documentation pages. By the end of the week there will be a proper documentation page to help with the new Error stuff.

Stay tuned!

Not trying to complain too much for such an awesome platform given to us for free but really the latest error changes are a mess, especially because they are too complex to understand and not really well documented.

For example the latest update from 3.2.4 to 3.2.5 broke all of our tests because validation errors did not contain a violations key anymore. This could be fixed by this:

api_platform:
    defaults:
        extra_properties:
            rfc_7807_compliant_errors: true

So we need to enable the RFC7807 errors to restore the violations key and thus restore the structure from 3.1.x. You can imagine that I don’t understand anything anymore. 😅

Hi everyone sorry really got caught up but we have lots of issues with errors right now so tomorrow I’ll provide the 3.1 ErrorListener back with a flag to use the new system. This will give me a bit more time to work on this as there are too many things that are not working properly! Sorry about this, I really wanted to push forward this Error to Resource thing (which should be really helpful) but it looks like the implementation is too flaky right now.

In the meantime you can override our ErrorListener service with something like:

    protected function duplicateRequest(\Throwable $exception, Request $request): Request
    {
        $this->controller = 'error_controller';
        return parent::duplicateRequest($exception, $request);
     }

But I’d recommend to stay on 3.1 while I provide a proper fix. Thank you for your patience!

We really need a way to preserve old behavior (before 3.2). API is used by frontend client, which relays on specific error format (hydra:description, etc)

Currently both config options: skip_deprecated_exception_normalizers: false rfc_7807_compliant_errors: false does not help.

https://github.com/api-platform/core/issues/4997 is related. It got auto close unfortunately 😦