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

Remove ProblemDetails integer error codes, no implementations seem to use them #1559

Closed
msporny opened this issue Sep 10, 2024 · 2 comments
Closed
Assignees
Labels
CR1 This item was processed during CR1 normative The PR is a normative change to the CR specification pr exists

Comments

@msporny
Copy link
Member

msporny commented Sep 10, 2024

The specification includes integer error codes and at present it does not seem like any implementations benefit from the extra feature. This is a suggestion to remove the integer error codes unless there is an implementation that can provide a good reason for them to continue to exist. One of the strongest arguments against them is that we have URLs to identify the warnings and the errors and therefore having yet another identifier does not help things and it can be harmful if the integer code and the URL error/warning code do not line up.

If this change is made, it should also be reflected in the Data Integrity specifications.

@msporny msporny added the ready for PR This issue is ready for a Pull Request to be created to resolve it label Sep 10, 2024
@msporny msporny self-assigned this Sep 10, 2024
@msporny msporny added the normative The PR is a normative change to the CR specification label Sep 11, 2024
@iherman
Copy link
Member

iherman commented Sep 11, 2024

The issue was discussed in a meeting on 2024-09-11

  • no resolutions were taken
View the transcript

3.9. Remove ProblemDetails integer error codes, no implementations seem to use them (issue vc-data-model#1559)

See github issue vc-data-model#1559.

Brent Zundel: last issue to look at is 1559, remove problem details integer error codes.

Manu Sporny: we were having some discussions with implementers in the VC API group and it turns out that, we added integer error codes in an attempt to say, if you have an IOT device/software library that cannot return strings or objects in a verify call, we will give well known integer codes that you can respond with.
… none of the implementers that we know of use these integer error codes, and the issue is that we need to allocate them, theory was to allocate them based on level of library (low-high).
… people were like, what's the use of having 32-64 be implementation defined, you cannot depend on the error code you get back.
… people unsure that this was needed, nobody spoke up, suggestion to remove this, doesn't mean implementers cannot do it.

Brent Zundel: I love removing things from the spec.

Ivan Herman: yes that is no problem, just one warning, if you do that in the spec ideally you have to do it in the vocabs as well.
… these objects are defined as terms in the vocabulary as well.
… keep that in mind.

Rob De Feo: just a note on this, from implementer perspective, when reading through the spec and looking at verification endpoints/response, it states that response should be an array of strings, there is a mention further down for problem details, but we couldn't figure out how to return it.
… that was partly why we didn't do it, don't know if other implementers have the same problem.

Manu Sporny: could you point out exactly where in the spec you read that.
… RobDeFeo if you could post this to Zoom we can raise an issue.

Brent Zundel: next topic, we can post link later.
… next on agenda is controller document.

@msporny msporny added pr exists CR1 This item was processed during CR1 and removed ready for PR This issue is ready for a Pull Request to be created to resolve it labels Sep 15, 2024
@msporny
Copy link
Member Author

msporny commented Sep 26, 2024

PR #1562 has been merged, closing.

@msporny msporny closed this as completed Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 This item was processed during CR1 normative The PR is a normative change to the CR specification pr exists
Projects
None yet
Development

No branches or pull requests

2 participants