Core: Customize Doctrine Exception Message

Created on 8 Mar 2017  路  28Comments  路  Source: api-platform/core

Hello,
I was working on configuration of status code for exception.

For the moment, I define this in config yml file:
exception_to_status:
ApiPlatformCoreExceptionInvalidArgumentException: 400
DoctrineDBALExceptionUniqueConstraintViolationException: 409

For the first exception, the message return is correct and I do not need to modify it
(Ex: The type of the "my_field" attribute must be "int", "double" given.)

For the second Exception (Doctrine UniqueConstraintViolationException), I have a nice SQL request returned (which is not good for security and in general in production server)

Have you something to catch it or modify the Doctrine message ?
Or is it something it can be developped in API platform ?
At the moment, any Doctrine Exception will return a SQL message:

  • NotNullConstraintViolationException
  • ForeignKeyConstraintViolationException
  • UniqueConstraintViolationException
  • SyntaxErrorException
  • NonUniqueFieldNameException
  • InvalidFieldNameException
  • TableNotFoundException
  • TableExistsException

Proposed Solution:

In API Platform, the event class responsible to send the data to doctrine (ApiPlatformCoreBridgeDoctrineEventListenerWriteListener), flush the data without catching the Doctrine Exception (line 68)

We need to develop the exception classes above in API Platform and catch the flush operation with theses classes and a better message, am I right ?

Thanks,
Jordan

question

All 28 comments

You're right. But those exceptions should always be catched by your code and never reach the end user level. Why do you have such exceptions?

You're right about the default message. We should probably do something to not display it in production by default and use generic messages like "Internal Server Error" to not expose sensitive information.

@dunglas Thanks for your response.

Bad example. In fact it was a problem with our Entity declaration

Anyway you're right, probably Show generic message for error 500 in production.
I propose a PR for that

929

@dunglas @soyuka @meyerbaptiste @teohhanhui

In my Team, for an internal reason, we call API POST with a predefined ID.
{
id: 'ea62c86c-d20f-4ce9-a502-441ca96b9434',
name: 'test',
}
In production, when you call 2 times the json above, you have a 409 conflict http error (Client error according Response Object) with a nice DBALException displaying our SQL table ^^
WDYT ? ^^

@jordscream I'm not sure I get what you mean. You're returning a HTTP 409 Conflict, right? So what's with the DBALException? Don't you catch the DBALException and throw your own exception?

@teohhanhui , Doctrine returns a 409 conflict cause we try to insert the same ID. But 409 conflict displays in ErrorNormalizer a SQL sensitive details even in production. So for the moment We decorates ErrorNormalizer to catch 409 conflict and hide the SQL details when we are in production. Don't you think this error should be catched in the same way as here ?

Thanks
Jordan

I'm not sure to understand. If Doctrine fails, it should be a 500 HTTP error.

Btw you should prevent such errors using the Validator component: http://symfony.com/doc/current/reference/constraints/UniqueEntity.html

@dunglas , when doctrine tries to insert with a constraint unique entity, it retourns a 409 conflict.
Anyway, When we use uniqueEntity, it works. Thanks for your advice

I don't get how it can be a 409. Is Symfony, or the DoctrineBundle doing something like this? Can you provide a way to reproduce this?

Are you sure that you don't have a specific config? A one inspirited from http://symfony.com/doc/current/bundles/FOSRestBundle/4-exception-controller-support.html for instance?

Hello @dunglas , I work with @jordscream,

The 409 code was generated by the exception_to_status config , I have mapped DoctrineDBALExceptionUniqueConstraintViolationException to 409 code.

@dunglas my bad ^^

@Timherlaud @jsamouh I'm trying to map DoctrineDBALExceptionUniqueConstraintViolationException to 409 status code, but it doesn't work for me ...

My entity :

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\UniqueConstraint;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;


/**
 * @ORM\Entity()
 * @ORM\Table(name="MyTable",
 *     uniqueConstraints={@UniqueConstraint(name="myfield_idx", columns={"myfield"})}
 * )
 *
 * @UniqueEntity("myfield")
 *
 * @ApiResource(
 *     // ...
 * )
 *
 */
class MyTable
{
    /**
     * @ORM\Column(type="bigint")
     */
    private $myfield;

    // ...
}

yaml configuration :

api_platform:

    // ...
    exception_to_status:
        # The 4 following handlers are registered by default, keep those lines to prevent unexpected side effects
        Symfony\Component\Serializer\Exception\ExceptionInterface: 400 # Use a raw status code (recommended)
        ApiPlatform\Core\Exception\InvalidArgumentException: 'HTTP_BAD_REQUEST' # Or a `Symfony\Component\HttpFoundation\Response`'s constant
        ApiPlatform\Core\Exception\FilterValidationException: 400
        Doctrine\ORM\OptimisticLockException: 409

        # Custom mapping
        Doctrine\DBAL\Exception\UniqueConstraintViolationException: 409

My response (when i post data with an already existing "myfield" value) :

{
    "@context": "/api/contexts/ConstraintViolationList",
    "@type": "ConstraintViolationList",
    "hydra:title": "An error occurred",
    "hydra:description": "myfield: This value is already used.",
    "violations": [
        {
            "propertyPath": "myfield",
            "message": "This value is already used."
        }
    ]
}

The response is fine, but i still get a 400 Bad Request http status code ...

Any idea ?

Hey Guys, I am having an issue of similar nature.
I API platform and Doctrine on top of PostgreSQL
I have a entity Topic and it's name is declared unique
``` /**
* @AssertNotBlank()
* @AssertLength(min="1", max="40")
* @ORMColumn(type="string", unique=true, length=40)
* @Groups({"get", "post", "put"})
*/
private $name;

and other entity **Channel** has name field but it isn't unique
```    /**
     * @Assert\NotBlank()
     * @Assert\Length(min="1", max="40")
     * @ORM\Column(type="string", nullable=false, length=40)
     * @Groups({"get", "post", "put"})
     */
    private $name;

And I have added annotation in both entities

 * @UniqueEntity(
 *     fields={"name"},
 *     errorPath="name",
 *     message="API.ERROR.ENTITY.CHANNEL.NAME.NOT_UNIQUE"
 *     )

And for Channels I get nicely formatted response but for Topics I get 500 with general hydra response "Internal server error"

Now the question is - why is Symfony validation skipped in case of Topics, why declaring field unique causes a problem ? Am i missing something ?
Mine goal is to have field unique and with validation, but in Topics case validating is skipped, and saving to DB fails.

Thanks in advance
Srdjan

p.s. I ask here because I have ran out of options :(

@xavier-rodet That's normal, because you've added the UniqueEntity Validator constraint. It never has a chance to hit the WriteListener / (Doctrine) DataPersister.

If you want it to hit Doctrine anyway, just remove that constraint.

@srdjanradisa We'd need more information to be able to help you. Please provide the stack trace.

@teohhanhui Thanks, if i remove my Validator constraint, i do get a 409 BUT my api will now return a SQL error instead of a proper explanation message ...

How to have a custom status code along with a proper explanation message ?

@xavier-rodet I suppose you could do your own check and throw a custom exception class for that.

But don't forget that the message would just say "Conflict" in the prod env. You should not rely on it in your client.

@teohhanhui Well i guess i'll create a custom action controller to throw my own exception on validation failure, thanks!

It would have been great to be able to configure globally the response code in case of duplicated entry, as i personally think 400 is not the appropriated code response in that scenario (the client request format is fine).

What did you mean about the message saying "conflict" ? My client rely on status code only (409 in that case) but would receive my custom exception message for logs, no ?

Indeed, but that'd mean not using the @UniqueEntity validation constraint anyway.

Oh i know what you mean now ...

That's pretty ugly because i would really like to have my entities properly annotated with @UniqueEntity

I tried to do so by creating an EventSubscriber but :

  • I cannot listen to POST_VALIDATE as API Platform has already sent a 400 response
  • I can listen to PRE_VALIDATE but as soon as i manually call for validation : API Platform is sending a 400 response

It looks like i'm facing a wall here ...

Well i just had to mix things together the best way i could to achieve that, see my "solution" below.

I still have my @UniqueEntity constraint properly defined in my entity !

I just created an EventSubsriber listenning to PRE_VALIDATE, so i could manually check if there is an existing key in db before validation :

<?php
# CreateMyTableSubscriber.php
namespace App\EventSubscriber;

final class CreateMyTableSubscriber implements EventSubscriberInterface
{
    const ALLOWED_OPERATIONS = ['post'];

    private $myTableRepository;
    private $em;

    public function __construct(MyTableRepository $myTableRepository)
    {
        $this->myTableRepository = $myTableRepository;
    }

    public static function getSubscribedEvents()
    {
        return [
            KernelEvents::VIEW => [
                ['checkForDuplicateEntry', EventPriorities::PRE_VALIDATE],
            ]
        ];
    }

    public function checkForDuplicateEntry(ViewEvent $event)
    {
        $myTable = $event->getControllerResult();
        $method = $event->getRequest()->getMethod();
        $operationName = $event->getRequest()->attributes->get('_api_collection_operation_name');

        if (!$myTable instanceof MyTable || Request::METHOD_POST !== $method || !in_array($operationName, self::ALLOWED_OPERATIONS)) {
            return;
        }

        // I manually check if this creation could lead into a dupiclate entry or not
        if($this->myTableRepository->findOneBy(['myfield' => $myTable->getMyField()])) {

            throw new MyTableAlreadyExistException(sprintf('MyTable with myfield "%s" already exist.', $myTable->getMyField()));
        }
    }
}

And I just configured MyTableAlreadyExistException to 409 status code !

This way i the response that i expected in that case scenario (with a 409 Conflit status code) :

{
    "@context": "/api/contexts/Error",
    "@type": "hydra:Error",
    "hydra:title": "An error occurred",
    "hydra:description": "MyTable with myfield \"1234\" already exist."
}

You can define a validation group which is only used for the duplicate check. It's better than checking manually.

Hmm i'm note sure to understand ?

My issue (i think) is that if i let validation do his job, it will return a 400 on my @UniqueEntity constraint failure, that's why i'm doing a manual check before (to set the proper response).
This way, if i forget to code this manuel check, i'm sure that my constraint will be checked anyway (just not sending the status code i wanted).

How validation group would change anything aboute my issue ?

In your event subscriber (PRE_VALIDATE), call the Symfony Validator with the duplicate_check group (for example). If there are constraint violations, throw your custom exception.

So, as you suggested, i added this validation group in my entity and pass it through validator in my EventSubscriber !

As I expected, i had still the same issue, then i just realized i was injecting ApiPlatform\Core\Validator\ValidatorInterface instead of Symfony\Component\Validator\Validator\ValidatorInterface, which is why i couldn't handle errors (it was instantly sending a 400 error) !

With the right Validator + the validation group i can check it properly now :)

Thanks 馃憤

Sorry Guys, quick response from me, I had custom handler and controller for POST method in my enrity, I used that to shift mine entries by position in DB when I add new item. And in that handler I had try catch block that was catching wrong exception type. And that exception that floated up caused 500. Once again sorry guys, for poluting the thread.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

teohhanhui picture teohhanhui  路  34Comments

renta picture renta  路  24Comments

danaki picture danaki  路  24Comments

vincentchalamon picture vincentchalamon  路  30Comments

vincentchalamon picture vincentchalamon  路  30Comments