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

Implements the use of ProblemDetail for HTTP error responses. (issue #149 and PR #173) #175

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

joaobertholino
Copy link
Contributor

Implements the use of ProblemDetail according to issue #149, separating it from other changes according to PR #173.

@joaobertholino joaobertholino changed the title feat: Implements the use of ProblemDetail for HTTP error responses. (issue #149 and PR #173) Implements the use of ProblemDetail for HTTP error responses. (issue #149 and PR #173) Nov 19, 2024
@arey
Copy link
Member

arey commented Nov 23, 2024

Could you please provide an example of an error response?

private record ErrorInfo(String className, String exMessage) {
public ErrorInfo(Exception ex) {
this(ex.getClass().getName(), ex.getLocalizedMessage());
}
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we have to rework the openapi.yml file in order to reference the problem details schema instead of the RestError one.

Copy link
Contributor Author

@joaobertholino joaobertholino Nov 23, 2024

Choose a reason for hiding this comment

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

Of course, I'll send the commit with the changes in the openapi.yml, along with some improvements to the ProblemDetail itself.

@joaobertholino
Copy link
Contributor Author

Could you please provide an example of an error response?

After the new changes I sent today, the error response is as follows.
image

title: Type
description: Full URL that originated the error response.
type: string
format: text
Copy link
Member

Choose a reason for hiding this comment

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

issue: we have to use theuri format
I think you could use the schema of this repository: https://github.com/belgif/openapi-problem/blob/master/src/main/openapi/problem/v1/problem-v1.yaml
It looks like very complete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it just be changing the value of the "format" property from "text" to "uri"?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, as a minimum. I think we can take inspiration from what @pvdbosch with his agreement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will implement this change in the "format" property and maybe a few more according to the @pvdbosch.

Choose a reason for hiding this comment

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

Hi, OK to use from the belgif openapi-problem repository! License is the same (both Apache License 2.0), so no problem there.

description: The long error message.
type: string
example: 'Request failed schema validation'
example: '2024-11-23T13:59:21.3820407'

Choose a reason for hiding this comment

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

Time offset is mandatory in the date-time format (which uses https://www.rfc-editor.org/rfc/rfc3339#section-5.6 ), so I believe this should be something like '2024-11-23T13:59:21.3820407Z' or '2024-11-23T13:59:21.3820407+00:00'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

O deslocamento de tempo é obrigatório no formato de data e hora (que usa https://www.rfc-editor.org/rfc/rfc3339#section-5.6 ), então acredito que deveria ser algo como '2024-11-23T13:59:21.3820407Z' ou '2024-11-23T13:59:21.3820407+00:00'

Hello @pvdbosch, I could implement it without any problems

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

Successfully merging this pull request may close these issues.

3 participants