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

Low level Linux libsecret operations should throw exceptions #4497

Merged
merged 3 commits into from
Jan 13, 2024

Conversation

bgavrilMS
Copy link
Member

@bgavrilMS bgavrilMS commented Jan 11, 2024

Fixes #4493

Changes proposed in this request

Testing

Performance impact

Documentation

  • All relevant documentation is updated.

@trwalke
Copy link
Member

trwalke commented Jan 12, 2024

I am trying to understand what is happening here so my question is how does this address the lack of clarity mentioned in the original issue in regards to This error? Im guessing these new exceptions be thrown instead of the one mentioned above to provide more clarity that way?

@bgavrilMS
Copy link
Member Author

I am trying to understand what is happening here so my question is how does this address the lack of clarity mentioned in the original issue in regards to This error? Im guessing these new exceptions be thrown instead of the one mentioned above to provide more clarity that way?

Correct, the accessor should throw meaningful exceptions with details, it should not hide errors. This is what Mac and Windows accessors do today - they bubble up exceptions, but on Linux it looks like we're just logging.

It's the responsability of higher level APIs in this library to hide exceptions, if needed.

@gladjohn gladjohn enabled auto-merge (squash) January 13, 2024 04:10
@gladjohn gladjohn merged commit 25a286e into main Jan 13, 2024
6 checks passed
@gladjohn gladjohn deleted the bogavril/4493 branch January 13, 2024 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message when token cache persistence fails could be more descriptive
4 participants