Fix unwraps in Client #12
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustaceanrob
temporarily deployed
to
infisical-test-workspace
GitHub Actions
Inactive
DanielHougaard
requested changes
Jan 16, 2024
| @@ -20,7 +20,7 @@ pub struct Client { | |||
|
|
|||
| impl Client { | |||
| pub fn new(settings_input: Option<ClientSettings>) -> Self { | |||
| let settings = settings_input.unwrap(); | |||
| let settings = settings_input.unwrap(); // show an error to set the .env? Not actually sure how this throws | |||
There was a problem hiding this comment.
I would honestly say that it's alright for it to throw a panic here. This would happen during the client initialization, if no settings or invalid settings are passed. Since we can't return a normal error, a rust panic would be alright here.
What do you think?
There was a problem hiding this comment.
I think an expect could be a good option because the SDK bindings will show the panic message even in Swift, Kotlin, etc. With a short message it might help point the user in the right direction.
rustaceanrob
temporarily deployed
to
infisical-test-workspace
GitHub Actions
Inactive
|
I tried making an edit straight from github and screwed up the commit haha. Will make a new PR removing that function. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feel free to edit any one of these
In the initialization of a
Clientthere is anunwrapthat I am not sure what to do with besides maybe throwing a specific error to the user that something went wrong with the environment variables or project config.I added an additional
unwrap_orso the entire initialization of the client has default values, but this behavior might not be desired and can be removed if nouser_agentshould throw an error.Lastly, if a lock can't be acquire on the
Mutexthen another thread is using it, and I think the behavior should be the cache does not change, and there is no error. (as opposed to the unwrap)