Skip to content

Improvements for Problem guideline#826

Open
ktsypkina wants to merge 2 commits intomainfrom
update-problem-documentation
Open

Improvements for Problem guideline#826
ktsypkina wants to merge 2 commits intomainfrom
update-problem-documentation

Conversation

@ktsypkina
Copy link
Member

@ktsypkina ktsypkina commented Mar 3, 2025

Incorporate API Guild thoughts on detailing Problem Object guidelines (#763)

@zalando-compr-opensource
Copy link

invalid team ID

The team ID in your .zappr.yaml file does not appear to be valid. Please, fix
this before team ID checks will be added back into ComPR's specification check.

You can follow this guideline for help.

@ktsypkina ktsypkina changed the title Addressing Issue #763 Improvements for Problem guideline Mar 3, 2025
Copy link
Member

@tfrauenstein tfrauenstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I like the guideline improvement -- provides more details and clarity. However, I think we should avoid redundant and inconsistent change of the Problem Object specification -- see my comment.

the member MUST be ignored -- i.e., processing will continue as if
the member had not been present.

===== status (integer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should avoid specification of the problem object in the GUI guideline redundantly (and possibly inconsistently) to the open-source specification in https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.1.yaml

Instead we should clearly refer to it and update it as part of the PR, if helpful.
It also allows us to better understand the differences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should avoid redundancies, but having it written out in the guidelines makes it easier to read than just pointing to the file. Not sure what the best way of doing this is, though.

@ktsypkina
Copy link
Member Author

@tfrauenstein @ePaul could you please have another look?

Comment on lines -567 to -573
`application/problem+json` to provide an extensible human and machine readable
failure information beyond the HTTP response status code to transports the
failure kind (`type` / `title`) and the failure cause and location (`instant` /
`detail`). To make best use of this additional failure information, every
endpoints must be capable of returning a Problem JSON on client usage errors
({4xx} status codes) as well as server side processing errors ({5xx} status
codes).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this paragraph.

of the extended failure information.
* Developers: to debug, diagnose and resolve issues during API integration or
operation
* Client applications: to inform users about errors (in user interface) and to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem object is provided for developers, not for application and UI users.
I would dismiss this and the above additions.


*Note:* Problem `type` and `instance` identifiers in our APIs are not meant
to be resolved. {RFC-9457}[RFC 9457] encourages that problem types are URI
{RFC-9457}[RFC 9457] encourages that problem types are URI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old wording is IMO clearer.

support automated processing and error handling when appropriate.

The OpenAPI schema definition of the Problem JSON object can be found
==== Problem schema members, supported by the API guidelines
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
==== Problem schema members, supported by the API guidelines
==== Problem object specification


The OpenAPI schema definition of the Problem JSON object can be found
==== Problem schema members, supported by the API guidelines
Problem detail objects can have the following members described in the OpenAPI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Problem detail objects can have the following members described in the OpenAPI
The Problem object is specified in the OpenAPI

Comment on lines +609 to +612
Clients may utilize properties in the Problem Object, particularly those
documented in the API specification. However, they *should not* depend on these
properties being present or having specific values.
The examples of such cases are:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Clients may utilize properties in the Problem Object, particularly those
documented in the API specification. However, they *should not* depend on these
properties being present or having specific values.
The examples of such cases are:
Clients may use, but *should be robust and not* depend on the Problem
properties being present or having specific values of specific types.
The examples of such cases are:

Comment on lines +623 to +637
The OpenAPI schema definition of the Problem JSON object can be found
https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.1.yaml[on
GitHub]. You can reference it by using:

[source,yaml]
----
responses:
503:
description: Service Unavailable
content:
"application/problem+json":
schema:
$ref: 'https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.1.yaml#/Problem'
----

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be moved to the Problem object Specification above.

include `application/problem+json` in the {Accept}-Header to trigger delivery
of the extended failure information.

==== Automated processing of Problem Object on client side
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
==== Automated processing of Problem Object on client side
==== Client processing of Problem Object

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client aspects mentioned in the sub-chapter above should be also mentioned here, shouldn't it?

Comment on lines +667 to +669
4. For collections of related APIs, using shared type values for similar errors
and distinct values for different errors simplifies automated processing and
error handling for clients working across APIs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a hint for the clients, but for the servers is it?
It is unclear for me, and I not sure we should give this hint.


CustomProblem:
allOf:
- $ref: "#/components/schemas/Problem"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply reference to the standard definition?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants