New 409 error code called INCOMPATIBLE_STATE

[tlohmar]

What type of PR is this?

• enhancement/feature

What this PR does / why we need it:

The PR adds a new 409 Error Code called INCOMPATIBLE_STATE, indicating that the referenced resource in the request is in a wrong state for the requested operation.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a breaking change?

• Yes
• No

Special notes for reviewers:

Changelog input

New 409 Error Code called INCOMPATIBLE_STATE, indicating that the referenced resource in the request is in an incompatible state. 

Additional documentation

This section can be blank.

docs

[patrice-conil]

LGTM

[Kevsy]

Well...

indicating that the referenced resource in the request is in a wrong state for the requested operation.

...that is the same meaning defined in RFC 9110 for 409 Conflict:

The 409 (Conflict) status code indicates that the request could not be completed due to a conflict with the current state of the target resource.

Unfortunately, the existing common.yaml has redefined 'CONFLICT' to specifically mean 'duplicate entry'

             status: 409
                code: CONFLICT
                message: A specified resource duplicate entry found.

To avoid confusion with the standard, it may be better to align the existing CAMARA definition for CONFLICT with the RFC. Then if we specifically need an additional 'duplicate entry' error we can see if it should be a 409 or 422 code.

[tlohmar]

Well...

indicating that the referenced resource in the request is in a wrong state for the requested operation.

...that is the same meaning defined in RFC 9110 for 409 Conflict:

The 409 (Conflict) status code indicates that the request could not be completed due to a conflict with the current state of the target resource.

Unfortunately, the existing common.yaml has redefined 'CONFLICT' to specifically mean 'duplicate entry'

             status: 409
                code: CONFLICT
                message: A specified resource duplicate entry found.

To avoid confusion with the standard, it may be better to align the existing CAMARA definition for CONFLICT with the RFC. Then if we specifically need an additional 'duplicate entry' error we can see if it should be a 409 or 422 code.

Well, certainly good to align CAMARA with the RFC.

On the new status code: Note the difference between conflict on the target resource compared to conflict on the referenced resource. We are planning to provide this status code as response to a failed POST operation, meaning that the target resource is not yet created.

[Kevsy]

Thanks @tlohmar

On the new status code: Note the difference between conflict on the target resource compared to conflict on the referenced resource. We are planning to provide this status code as response to a failed POST operation, meaning that the target resource is not yet created.

So if I understand that correctly
1 - referenced resource exists, and is referenced in a POST operation to an API endpoint with the intention of creating a new resource
2- due to an issue with the state of that referenced resource, the new resource cannot be created, and the request fails

Have I got that right..?

[tlohmar]

Thanks @tlohmar

On the new status code: Note the difference between conflict on the target resource compared to conflict on the referenced resource. We are planning to provide this status code as response to a failed POST operation, meaning that the target resource is not yet created.

So if I understand that correctly 1 - referenced resource exists, and is referenced in a POST operation to an API endpoint with the intention of creating a new resource 2- due to an issue with the state of that referenced resource, the new resource cannot be created, and the request fails

Have I got that right..?

Yes.

[tlohmar]

Maybe a question for clarification: Why are the 409 Error Codes so called "Syntax Exceptions"?

[Kevsy]

Maybe a question for clarification: Why are the 409 Error Codes so called "Syntax Exceptions"?

Good question: the request can be sent with valid syntax and result in a 409.

The API Design Guide distinguishes 'syntax exceptions' from 'service exceptions' and 'server errors' and that whole section may be worth revisiting...

[rartych]

The API Design Guide distinguishes 'syntax exceptions' from 'service exceptions' and 'server errors' and that whole section may be worth revisiting...

That's what we concluded during the meeting - any suggestions for more intuitive error codes taxonomy/listing are welcome (in new issue)

[PedroDiez]

We talked about modifying to something like "INCOMPATIBLE_STATE"

[Kevsy]

Thanks @PedroDiez - indeed, 'incompatible' is more accurate here, because the state of the referenced resource is only 'wrong' in this specific context (i.e. it is not intrinsically 'wrong').

[PedroDiez]

After some analysis, and also aligned with discussion in 10th November meeting, some simplificacion is also proposed:
to remove the code "ALREADY_EXISTS" due to is mainly the same concept/case than "CONFLICT".

[patrice-conil]

After some analysis, and also aligned with discussion in 10th November meeting, some simplificacion is also proposed: to remove the code "ALREADY_EXISTS" due to is mainly the same concept/case than "CONFLICT".

IMHO, the more generic term "CONFLICT" should be defined in the commonalities, while the more specific term "ALREADY EXISTS" in the context of an API should be defined in its specification. Is it really necessary/desirable to group all API error messages in commonalities? Perhaps we are over-specificating things. ;)

[tlohmar]

Sorry for the delayed update of the PR. Please check.

IMHO, the more generic term "CONFLICT" should be defined in the commonalities, while the more specific term "ALREADY EXISTS" in the context of an API should be defined in its specification. Is it really necessary/desirable to group all API error messages in commonalities? Perhaps we are over-specificating things. ;)

The Error code CONFLICT as sub code for the HTTP status code 409 Conflict is confusing. Thinking along the lines of @patrice-conil suggestion, it feels better to remove the CONFLICT error code from commonalities.

[PedroDiez]

IMHO, the more generic term "CONFLICT" should be defined in the commonalities, while the more specific term "ALREADY EXISTS" in the context of an API should be defined in its specification. Is it really necessary/desirable to group all API error messages in commonalities? Perhaps we are over-specificating things. ;)

Have understood to keep code CONFLICT in commonalities from the comment of Patrice

[patrice-conil]

IMHO, the more generic term "CONFLICT" should be defined in the commonalities, while the more specific term "ALREADY EXISTS" in the context of an API should be defined in its specification. Is it really necessary/desirable to group all API error messages in commonalities? Perhaps we are over-specificating things. ;)

Have understood to keep code CONFLICT in commonalities from the comment of Patrice

You're right @PedroDiez… that's what I was trying to say. The 409_Conflict error with the code "CONFLICT" seems to be a generic error; any other variation would fall under an API's specialization. But if most people think differently, I'll accept it without any problem. I don't want to cause any conflict between us ;-)

[PedroDiez]

I would prefer to keep 409 CONFLICT in Commonalities, mainly to be closer to RFC 9110 wording

[tlohmar]

I would prefer to keep 409 CONFLICT in Commonalities, mainly to be closer to RFC 9110 wording

I dont understand this comment. CAMARA has introduced additional error codes to the HTTP status code 409 CONFLICT. I dont think, there is a request to remove the 409 CONFLICT status code from commonalities. The discussion was about the CAMARA introduced error code CONFLICT, i.e. responses containing the 409 CONFLICT status code with the error code CONFLICT.

Anyway, maybe we should keep this discussion separate from this PR.

[rartych]

LGTM

[PedroDiez]

LGTM

[Kevsy]

LGTM

[patrice-conil]

LGTM

[hdamker]

Currently the changelog entry in the PR description does not fit to the PR content ("WRONG_STATE" vs "INCOMPATIBLE_STATE"). As we want to move towards more automation, this entry get's important.

Beyond that I would have the question if there is enough evidence that his is a traversal error code which is needed by more than one or two APIs. Otherwise we have in a few months a similar discussion as in #562. I couldn't find an issue for this PR where this was discussed.

[hdamker]

@tlohmar

Currently the changelog entry in the PR description does not fit to the PR content ("WRONG_STATE" vs "INCOMPATIBLE_STATE"). As we want to move towards more automation, this entry get's important.

Thanks for update the changelog entry. You can consider to edit also the PR title.

Beyond that I would have the question if there is enough evidence that his is a traversal error code which is needed by more than one or two APIs. Otherwise we have in a few months a similar discussion as in #562. I couldn't find an issue for this PR where this was discussed.

I will dismiss my blocking request for changes, as my analysis (cf. #562 (comment)) has shown that this new error code can replace several API specific error codes. But one prerequisite for it is that it is not restricted to referenced resources but also used if there is a conflicting state of the target resource. We can apply this change of your proposal after we have concluded the discussion in #562.

The proposed code in Commonalities could be something like (description not binding for APIs, need to be adapted to the actual error):

          GENERIC_409_INCOMPATIBLE_STATE:
            description: |
              Resource (target or referenced) is in incompatible state for the requested operation.
              Use for both:
              - Target resource state conflicts (e.g., session not in AVAILABLE state for extension)
              - Referenced resource state conflicts (e.g., device already has active conflicting session)
              Message should clarify which resource and required state.
            value:
              status: 409
              code: INCOMPATIBLE_STATE
              message: "Resource is in an incompatible state for this operation"

see comment #550 (comment)

[tlohmar]

Seems reasonable to make this new EC applicable to both, the target and the referenced resource.

[rartych]

After merging this PR I propose to improve 409 definitions with conclusions from #562