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

server_license module requires non obvious and error-prone control flow #269

Open
CBenoit opened this issue Nov 7, 2023 · 0 comments
Open
Labels
A-core Area: Core tier A-technical-debt Area: Internal cleanup work bug Something isn't working P-medium Medium priority

Comments

@CBenoit
Copy link
Member

CBenoit commented Nov 7, 2023

LICENSE_ERROR_MESSAGE licensing packets should not be returned as errors by the parser.
Such packets should be handled by the caller, and the caller is responsible for turning those into Result::Err if necessary.
It should be possible to decode a LICENSE_ERROR_MESSAGE structure like any other PDU.
Otherwise it requires the caller to match on the error kind in order to check for variants that are not actual errors (notably because of the protocol-level error code STATUS_VALID_CLIENT), and it makes the flow of control harder to write correctly and less obvious, i.e.: using the error variant for control flow is an anti-pattern the same way that using exceptions for control flow in languages such as C# or Java is an anti-pattern.
See ConnectionConfirm from the nego module for prior art.

Prior bug caused by this pattern: #268

@CBenoit CBenoit added bug Something isn't working A-core Area: Core tier A-technical-debt Area: Internal cleanup work P-medium Medium priority labels Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core tier A-technical-debt Area: Internal cleanup work bug Something isn't working P-medium Medium priority
Development

No branches or pull requests

1 participant