[JorisHeadease]

Follow-up to this question in #4660: the API reference showed default instead of the actual error codes, because most operations declared their errors only as default, with the codes as text in their description ("error returns: …"). This declares the codes in the specs, so Scalar, Postman and the generated clients show them.

Preview: https://jorisheadease.github.io/nuts-node-docs-preview/pages/integrating/api.html

Changes in the specs

• Each code from an operation's "error returns" list is now a response of its own, with the reason as its description and a shared ProblemDetails body, new in common/ssi_types.yaml. The lists are removed from the descriptions.
• Each of these responses has an example with the title and status the node returns, such as IssueVC failed and 412. The example's detail is the reason; in an actual response it is the node's error message, as the detail description now says.
• YAML anchors keep these responses short: the example's detail is an alias of the description, and each spec anchors the reference to ProblemDetails once. Most responses take six lines instead of ten.
• Generic 500 lines are dropped: every operation can return 500, which stays covered by default. Its description says so and refers to RFC 9457, which obsoletes RFC 7807. Its example illustrates a 500 with a placeholder for the operation name, as all operations share it. Its body refers to the same ProblemDetails, so the schema is defined once.
• The Discovery server spec used the shared response for the explicit 400 of registerPresentation as well. That 400 now has its own description and example, and refers to ProblemDetails too.
• The merge script passes error_response.yaml through paste() like the specs, so its reference points into the combined document.
• Response codes in the Auth v1 and DID Manager specs are now quoted, as OpenAPI requires.

Corrections

The lists were checked against each API's error handling:

• Auth v2 never returns 503, as an unreachable authorizer results in 500 or 502. requestUserAccessToken and requestOpenid4VCICredentialIssuance don't check credentials, so they can't return 412 either.
• listTrusted and listUntrusted return an empty list for an unknown credential type, not 404.
• requestOpenid4VCICredentialIssuance returns 404 for an unknown subject.
• Every Network operation returns 400 when did:nuts support is not configured.
• retrieveAccessToken, getCredentialsInWallet, getContactInformation and updateContactInformation had no list; they now declare the codes their handlers return.

Generated code

• The codegen configs map common/ssi_types.yaml to its Go package with import-mapping, so all API packages use the same ProblemDetails type. api/ssi_types_test.go becomes an external test package, as it imports the VCR API, which now imports api.
• A typedef.tmpl override in common_ssi_types.yaml generates ProblemDetails as a type alias of the struct that oapi-codegen generated inline for each error body before, so code that uses that struct type keeps compiling.
• In total, the generated code ends up 37 lines smaller than on master. The diff grows in the specs (about +425 lines), with the per-code responses and their descriptions and examples.
• Generated clients now parse a declared error into its own field, such as ApplicationproblemJSON404, instead of ApplicationproblemJSONDefault. The JSON is unchanged. The browser e2e helpers that read the default field now check for a missing 200 response instead.
• vdr/api/v2/generated.go also picks up a description change in its spec that was never regenerated on master.

Verified

• go build ./..., go vet for the API packages and for ./e2e-tests/browser/... with -tags e2e_tests, and the full go test ./... pass.
• The merge script combines the specs into 85 operations without broken references. Scalar shows the codes with their reasons and examples.
• Sphinx builds with the same four warnings as master.
• The YAML anchors don't change the parsed specs (checked with js-yaml and a Go YAML parser), and the generated code is byte-identical.

Backports to V6.2 and V5.4 will follow after merge.

[reinkrul]

Is this actually better in terms of maintainability? This leads to lots of duplication...

[qltysh]

25 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 6): ParseDrawUpContractResponse	25

[JorisHeadease]

It's a trade-off between maintainability vs. usability of the docs. Without theses examples, users see the following as error:

Which is... far from ideal. OpenAPI 3.0 doesn't really offer a way to prevent the duplication afaik. I did make the blocks shorter with YAML anchors and aliases: six lines instead of ten, and the example no longer repeats the reason.

As for the maintainability, I don't think we have to "maintain" the error codes too much?

Let me know what you think about the trade-off @reinkrul, can always remove the examples