Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error message for invalid UUID on the URL #3045

Closed
ghost opened this issue Sep 4, 2019 · 8 comments
Closed

Error message for invalid UUID on the URL #3045

ghost opened this issue Sep 4, 2019 · 8 comments

Comments

@ghost
Copy link

ghost commented Sep 4, 2019

Hi.

Having an API Resource with the following identifier configuration (I got it from the demo), when I send a request like this: curl -X GET "https://demo.api-platform.com/books/wrong-id" -H "accept: application/ld+json" It fails because of the denormalization, returning a 404 error thrown by the ReadListener

 /**
  * @var UuidInterface
  *
  * @ORM\Column(type="uuid", unique=true)
  * @ORM\Id
  * @ORM\GeneratedValue(strategy="CUSTOM")
  * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")
  */
 private $id;

So, the error handling works perfectly. But, the message response of the request is: "Not found, because of an invalid identifier configuration". My question: What is the best approach to customize this message? It isn't a configuration mismatch on that case.

@teohhanhui
Copy link
Contributor

teohhanhui commented Sep 6, 2019

The exception message is indeed misleading. We'll have to see if we can tell the different cases apart, but in this case it should say something like: "Not found, because of an invalid identifier format"

WDYT?

@rgarnica
Copy link

rgarnica commented Sep 9, 2019

Oh. I just realized I opened this with the wrong user.

Anyway. I agree. We could throw a different exception type from the ApiPlatform\Core\Bridge\RamseyUuid\Identifier\Normalizer\UuidNormalizer::denormalize when the parse failed.

@teohhanhui
Copy link
Contributor

@rgarnica
Copy link

rgarnica commented Sep 9, 2019

Great. It's ok if I work on it?

@teohhanhui
Copy link
Contributor

Please go ahead. PR welcome!

@soyuka
Copy link
Member

soyuka commented Sep 13, 2019

Mb related #2191

Taminoful added a commit to Taminoful/core that referenced this issue Oct 1, 2019
As discussed in issue api-platform#3045, the given error message was misleading.
I also updated the thrown exception according to the Exception
mentioned in the same issue. Of course I updated the related PHPUnit
test too.
Taminoful added a commit to Taminoful/core that referenced this issue Oct 1, 2019
As discussed in issue api-platform#3045, the given error message was misleading.
I also updated the thrown exception according to the Exception
mentioned in the same issue. Of course I updated the related PHPUnit
test too.
@djpremier
Copy link
Contributor

@soyuka
Copy link
Member

soyuka commented Aug 12, 2020

we fixed this in #3132 afaik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants