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

Error handling and edge cases #229

Closed
ashcherbakov opened this issue Jul 15, 2021 · 7 comments · Fixed by #232
Closed

Error handling and edge cases #229

ashcherbakov opened this issue Jul 15, 2021 · 7 comments · Fixed by #232
Assignees
Labels
PR Needed v2.0 necessary for spec completion

Comments

@ashcherbakov
Copy link

It looks like the spec doesn't say how to handle errors and edger cases.
For examples:

  • incompatible curves (keyAgreement types in DID DOC) between sender and receiver
  • invalid data
  • attacks (such as Need details about invalid curve and weak point attack resistance #217)
  • the crypto algorithm that sender wants to use is not supported on the receiver side (in case when DID Comm spec is not fully supported on of the sides, or when new crypto is added to the spec).

It would be great if the spec can address some of this edge cases or provide a general strategy what to do in this cases: just terminate communication, send a specific error message, etc.

@dhh1128
Copy link
Contributor

dhh1128 commented Jul 15, 2021

I am assigning this issue to myself and would like to produce a PR for the spec about it.

@kdenhartog kdenhartog added PR Needed v2.0 necessary for spec completion labels Jul 15, 2021
@kdenhartog
Copy link
Contributor

Labeled/assigned to you so it's easier to track it

@kdenhartog
Copy link
Contributor

@dhh1128 can you consider the usage of #83 when describing this. I think this issue should be more specific to returning specific errors in general scenarios around the payload layer and #83 is going to be about defining the generic structure and default errors I'd think.

@dhh1128
Copy link
Contributor

dhh1128 commented Jul 17, 2021

@kdenhartog : I did read #83 and think about it. But I am not sure I understand the distinction between specific errors in general scenarios, and generic structure and default errors. Did you want me to define a few generic error codes (e.g., like the ones in errno.h from *nix)? Please feel free to push me to change the content; I just wasn't sure if I was covering your thinking enough.

@kdenhartog
Copy link
Contributor

Based on what you're going towards in #232 I think we're on the right track. My thoughts on the topic aren't completely fully formed yet, but I'm aiming for the case where an error code response can be branched on to reduce the logic in the error code flows.

Specifically, I was thinking more along the lines of generic structure and default errors. Then these would be extended for more specific scenarios if needed, or if debugging is going on. I need to spend some time re-reading the problem report RFC before I think I can get a fully formed opinion on the topic.

@dhh1128
Copy link
Contributor

dhh1128 commented Jul 17, 2021

You mean like an error code hierarchy -- so you can branch on errors at a high level even if you don't understand the low-level detail? Something like recoverable-error.resources.out-of-memory and recoverable-error.resources.out-of-disk and recoverable-error.resources.bandwidth, so you could handle recoverable-error or recoverable-error.resources without knowing the details about the right side of the codes?

@kdenhartog
Copy link
Contributor

We discussed this on the 7-19 call and I thought this would be alright. It allowed for error branching based on classes of errors which is one of the things I was going for with default errors. I think we'll need to also define the basic errors that we want to support as well similar to how HTTP defined the original codes. Probably worth allowing this to be extensible to a certain degree with some limitations so the defaults actually remain generic enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Needed v2.0 necessary for spec completion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants