[gries]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

Hi,
I added the possibility to specify an error code for JsonApi Errors as specified in the JsonApi Spec.
https://jsonapi.org/format/#errors

In short when throwing an Exception like:

throw new MyError('my bad error', 1234);

the Normalizer now uses the Exception Code like so:

{
  "title": "..",
  "description": "...",
  "code": 1234
}

It's a minor change so I'm not sure if master is the correct base branch.

[dunglas]

The target branch is good, as it's a new feature!

[gries]

I made the requested changes rebased and squashed hope it's fine now.

It looks like behat tests are failing allthough I only rebased the code onto master @dunglas

[gries]

any update on this?

[soyuka]

There are some concerns on whether this is a good idea or not (exposing exception codes).

[teohhanhui]

The JSON:API spec says it should be an "application-specific error code". We cannot assume that all exceptions thrown are controlled by / originate from the application. I don't think it's a good idea to map the exception code to error code indiscriminately.

I'm -1 unless we introduce a mapping or some other way of doing that conversion.

[gries]

Currently the exception code is only set of its not the default code which is 0.
So somewhere the application must have explicitly set the exception code.

the idea of an exception code to error code mapping sounds interesting so if you want I can add that if you point me to the right config place where I should put it.

Another idea would be to add an Interface that can be used to throw exception that have an application error code. I'm not sure about the naming but something like:

// using anonymous classes
throw new class('my exception', 12345) extends \Exception implements ApplicationErrorInterface

[teohhanhui]

Currently the exception code is only set of its not the default code which is 0.
So somewhere the application must have explicitly set the exception code.

I think that's a dangerous assumption. The exception code could be set by another library, not the app. So we may be exposing some exception code that makes no sense to the client.

But the main problem is that the error code could be a string. There is no reason to just take the exception code. So yeah, let's introduce a mapping (probably too cumbersome to use...) or an interface like this:

interface ErrorCodeExceptionInterface extends \Throwable
{
    public static function getErrorCode(): string;
}

(Static method because the error code should not change between different instances of the class.)

[gries]

Hi,
I have added an implementation that uses an ErrorCodeExceptionInterface please have a look, I'm not fully happy with the way it is implemented in the normalizer but I saw no other way as we are dealing with FlattenExceptions.

[gries]

just a bump to see if something if missing or can this be merged?

[teohhanhui]

Closing as I've explained why this is not the right approach.

[teohhanhui]

Sorry, I see you've updated to a different approach.

[teohhanhui]

Only minor changes. LGTM! 👍

[gries]

I have applied your suggestions thanks for the help :)

[gries]

@teohhanhui I fixed errors caused by my PR I think the other problems are not related to my changes.

[teohhanhui]

Thanks @gries! 🎉