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

Fixed AsyncStorage exists not throwing errors #325

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kitwtnb
Copy link
Contributor

@kitwtnb kitwtnb commented May 27, 2024

Current existsObject() returns true or throw error, not returning false.
As per StorageAware, existsObject() should not throw an error.
Fix existsObject() to return false instead of error.

@3lvis
Copy link
Collaborator

3lvis commented Jun 4, 2024

Hi @kitwtnb,

Would you be so kind on adding a tests that reproduces the error as well to avoid regressions?

@kitwtnb
Copy link
Contributor Author

kitwtnb commented Jun 5, 2024

Changed Result<Bool, Error> to Bool and removed throws.
So we could express by type that the previous problem does not regression.

Do I need to test separately from these?

@3lvis
Copy link
Collaborator

3lvis commented Jun 19, 2024

Can anyone else verify if this breaks anything for them?

@kitwtnb
Copy link
Contributor Author

kitwtnb commented Jul 13, 2024

I understand what you are trying to say.

This test originally reproduced the problem.
I expect it to throw an Error instead of false if the object does not exist.
https://github.com/hyperoslo/Cache/pull/325/files#diff-360238a58739325a7ce97d0a648b466797db4442b6cae0592f1bb2e26c0f97d7L90

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.

None yet

2 participants