Better error handling

[OR13]

When errors occur, there should be helpful messages returned over http.

There should be an error message structure.

The structure should be used when handling 200 responses where the verification failed

There should also be a 400 error message structure, for cases where the client violates the api definition.

There should be test cases for errors, and each variant of an error.

There should be specific cases for revocation, expiration, preactivation.

[OR13]

We use a structure like this internally:

VerificationResult:
      title: Verification Check
      description: Verification result for a Verifiable Credential
      type: array
      items:
        type: object
        properties:
          status:
            type: string
            example: 'good'
          title:
            type: string
            example: 'Activation'
          description:
            type: string
            example: 'This credential activated a month ago'

Turning checks into readable sentences improves devx, and ui rendering consistency.

[OR13]

We need better error handling, we can't tell why things fail.

See https://github.com/w3c-ccg/vp-request-spec/pull/12

We need 400 errors with more information...

and 200 errors with more information....

we need better schemas for communicating failure and success of flows.

[nissimsan]

This is between the presenting parties: define happy path and sad path interactions. What are the 400 error we expect can occur? What are relevant error messages which will help resovle?

[nissimsan]

I might pick this up next time when I have better bandwidth.

[OR13]

awaiting https://github.com/w3c-ccg/vc-api/pull/258

[nissimsan]

next step: port over VC API error schemas, add postman for testing

[OR13]

Need to port over the schemas and write postman tests that produce errors for all documented error cases.

[OR13]

We still need to port over some error examples, ideally covering verify errors such as revocation.

[nissimsan]

Necessary for addressing this:

• Make negative tests.
• Define error JSONs.

[OR13]

Use english language to describe error cases, then ensure that there is a postman collection that asserts the json structure for each error condition.

[mkhraisha]

Initial Error cases when issuing a credential:

requests:

• [ ] issuer that does not support suite required by options
• [ ] revocation type requested that is not supported by the issuer
• [ ] missing credential subject
• [ ] term not defined in context

responses:

• [ ] missing credential subject
• [ ] the credential subject is not an object or a string
• [ ] missing context
• [ ] missing proof
• [ ] missing credential status when the revocable credential was requested
• [ ] missing credential identifier when the revocable credential was requested
• [ ] signature type of proof does not match the requested suite
• [ ] issuance date is not present

[OR13]

We have a generic error response today:

type: object
required:
  - code
  - message
properties:
  code:
    type: integer
    format: int32
  message:
    type: string
  details:
    type: object

IMO, its a best practice to make a table with error codes for each case, and publish detailed objects along with the code optionally... in higher security environments you just drop the details.

Here are some of the errors listed above, in yaml format:

errors:
  '/credentials/issue':
    - message: 'credential.@context is not present'
    - message: 'credential.type is not present'
    - message: 'credential.issaunceDate is not present'
    - message: 'issuer does not support requested suite'
    - message: 'revocation type requested that is not supported by the issuer'
    - message: 'credential subject is not an object or a string'
    - message: 'term not defined in context'
    - message: 'credential.credentialStatus cannot be present when requesting a revocable credential issuance'
    - message: 'credential.id is required when issuance a revocation list credential'
    - message: 'issuer of revocation list is not the same as issuer of revocable credential'

[OR13]

Before we go further, an entire category of '400' errors is solved for by:

instance data is not of the required schema type

For example:

credential is not of type Credential.

Where Credential says, @context, issuer, issuanceDate are required.

I propose we solve for these kinds of errors consistnetly... a very common pattern looks like this:

(req, res)=>{
try {
  requireValidJSONSchema(req.body, IssueCredentialRequestBody)
} catch (e){
  res.400.send(message: e.message)  // JSON-Schema errors
}
try {
  issue(req.body)
} catch (e){
  res.400.send(message: e.message) // JSON-LD errors
}

[nissimsan]

I assigned 401 and 403 today as I was adding security. More can be added, so let's keep this open, will continue when the above is merged.

[OR13]

I think this is blocked by having a negative test suite, and interesting jsons that produce 4xx errors when sent to the apis.

[brownoxford]

Self-assigning to begin work on negative test suite.

[BenjaminMoe]

I think that having specific error messages can drift easily, but I agree with @OR13 that a test suite is the logical step for addressing these details.

[nissimsan]

@brownoxford has some stuff he'll PR to indicate direction on this.

[brownoxford]

Draft PR #253 is where this will be added

[brownoxford]

I propose that we define generic values for the message field, and leave the description field up to the implementors. There would be quite a lot of very specific error messages that need to be maintained if we did not do that, and that seems counter-productive.

[brownoxford]

I think this is blocked by having a negative test suite, and interesting jsons that produce 4xx errors when sent to the apis.

Conformance test suite is implementing the negative tests with mutated JSON to produce 4xx errors, and BadRequest.yml, Unauthenticated.yml, and Unauthorized.yml have been updated to specify response code and message (leaving description up to implementor).

Can we close this issue?

[nissimsan]

Conformance tests will take this over. Closing.