Improve verification error handling

[JesusMcCloud]

Originally raised by @jsiwrk in https://github.com/ehn-dcc-development/hcert-kotlin/issues/32#issuecomment-867072802

Although we removed the errorMessage from the VerificationResult, as such information should be logged, not returned.

I would tend to agree with that in the general sense, if we were talking about an application or a service. However, since this is a reusable library that will presumably be used to build the former, wouldn't it be more appropriate to leave that choice to the library user?

For example, let's consider these scenarios:

• An application using this library wants to print a detailed error to the user indicating why the hcert is "expired". The CWT_EXPIRED error code can be returned for various reasons (hcert issued before signer cert, hcert issued in the future, hcert expired, hcert expiration exceeds signer cert expiration, expiration field not present, etc.), so the code itself is not enough to know the cause. Let's also assume this application is targeted to a type of user who is genuinelly interested about these details (maybe a second-line border control officer that needs to explain to a hcert holder why their document was rejected by the first-line control), and that he/she prefers to see this detail in a user-friendly way (rather than looking at some internal logs).
• An application or service using this library wants to record a log with the full details why the validation has failed, and wants to record the log in its own way. Maybe it needs to send a JSON-formatted log to a SIEM system (e.g. Splunk).

If these are deemed valid use cases for this library, then it seems that including an errorMessage (or, generally speaking, detailed information about the error) in the VerificationResult would be the right thing to do. And the library client would be responsible for making a proper use of this field. What do you think?

[JesusMcCloud]

The issue raised by @jsiwrk needs addressing, however error messages are probably the wrong way to go about this because:

1. Messages can change
2. Messages are comprehensible by humans, but hard to parse
3. Error messages my be incosistent without anyone really noticing it, as long as the stack traces are also interpreted (by a human) for additional context
4. i18n

It is tempting tempting to simply move error messages into a ResourceBundle and be done with it, since this would solve all of these issues. Personally, I'm a bit hesitant on this, as I am not sure whether I'm overlooking something. I therefore welcome any input on this discussion!

[jsiwrk]

Initially I was thinking about returning a simple (and non-internationalized) error message, but as you say, it's true that this would limit the possible use cases of this feature (message recorded in application logs and interpreted by a human expert).

One option would be to introduce a secondary error code (similarly to the secondary status code of SAML). This (optional) secondary error code would refine the primary code when appropriate; e.g. to clarify the reason for CWT_EXPIRED.

Another option would be of course to augment the set of primary codes to render the secondary code unnecessary. (But this could not be appropriate if many additional codes were needed, or if a reduced/concise set of codes is required for interoperability/standardization reasons... I guess.)

A secondary code cannot convey as much information as an error message (e.g. the actual date that has been detected as expired), but this could not be necessary for the intended use case of this feature (deterministic processing by an application or non-expert human).

And there is also the option of returning both: a secondary error code for deterministic processing, and an internal error message for advanced diagnostics (that the application would typically log as is; no internationalization would be required). One could argue though that the internal logs of the library are sufficient for the latter purpose; but on the other hand, the application/service could prefer to handle the logging by itself.

To summarize: no idea of what's the best option either. ;-)

[JesusMcCloud]

Thanks for following up! I did not think about contextual information such as the exact expiration date causing the expiry.

I still maintain that some "fixed" set of error codes/messages/… is the way to go, as it would have the nice side effect of all potential errors being enumerable. When combining this with contextual information, using sealed classes for errors come to mind, as it would allow for adding additional information as desired and still implement an error handler fully capable of dealing with all bit of information ever produced.

[jsiwrk]

Proposed another simple alternative in PR #46, just to see how it looks. It's untyped and surely not as elegant as using sealed error classes, but maybe simpler is best in this case (?). Anyway, feel free to discard this PR without further explanation if you don't like it.

[JesusMcCloud]

GH-46 merged, thanks! I'll leave this open for now, since there are still some error cases, which do not utilise this new feature.