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.

Sign up for GitHub

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

Jump to bottom

Add test for error propagation. #120

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Add test for error propagation. #120

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions suites/create.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,19 @@ export function runDataIntegrityProofFormatTests({
reason: 'Expected issuer to error when VC has an undefined term.'
});
});
it('If the algorithm produces an error, the error MUST be propagated ' +
'and SHOULD convey the error type.', async function() {
this.test.link = 'https://w3c.github.io/vc-data-integrity/#dfn-input-document:~:text=If%20the%20algorithm%20produces%20an%20error%2C%20the%20error%20MUST%20be%20propagated%20and%20SHOULD%20convey%20the%20error%20type';
const noSubject = structuredClone(credential);
delete noSubject.credentialSubject;
// FIXME this test should look for a propagated error such as
// PROOF_GENERATION_ERROR
Comment on lines +253 to +254
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this fixable before we merge this one?

Copy link
Contributor Author

@aljones15 aljones15 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we need to inform the implementers that they need to start returning error objects when issuance fails and verification fails. Currently most implementers do return verification results. Basically we need to make implementers aware that https://w3c-ccg.github.io/vc-api/#error-handling VC-API Error Handling is a thing. I also thought the VC-API group came to consensus at some point about standardized error handling, but it looks like they have not although error codes in the specs do suggest there is a common error object for data integrity at least. So perhaps we should ask implementers for the test suite to return https://w3c.github.io/vc-data-integrity/#processing-errors Problem Details in the future. Basically right now we not have informed implementers to return problem details so asserting on those error codes in data returned on fail seems unfair. I can add a section to the README about this as we really need those problem details for the suite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BigBlueHat revised README is here: #122

Those details should probably be in the implementations README to.

Copy link
Collaborator

Choose a reason for hiding this comment

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

w3c-ccg/vc-api#418

this is the PR that defines it, ideally there would be a problemDetails property in the response and we just need to ensure there's at least 1 item in that array, since there's no instruction on what to look for in that statement.

await shouldFailIssuance({
credential: noSubject,
issuer,
reason: 'Expected issuer to error when VC has no credentialSubject.'
});
});
if(cryptosuiteName) {
it('The value of the cryptosuite property MUST be a string that ' +
'identifies the cryptographic suite.', async function() {
Expand Down
Loading