alexrintt / shared-storage

Flutter plugin to work with Android external storage.
http://alexrintt.io/shared-storage/
MIT License
52 stars 26 forks source link

openDocumentFile() returns null #121

Closed mx1up closed 1 year ago

mx1up commented 1 year ago

Describe the bug documentFile.openDocumentFile returns null, even on success

To Reproduce final result = await documentFile.openDocumentFile()

Expected behavior result to be true on success

Smartphone (please complete the following information):

Additional context this problem is reproducable using the sample app: example/lib/utils/document_file_utils.dart:25 launched is null on successful launch

alexrintt commented 1 year ago

Fix was released under v0.7.2, please re-open the issue if this behavior persists.

mx1up commented 1 year ago

hi @alexrintt , thanks for the quick fix. Unfortunately, now when no app is present that supports the file format, I no longer get an exception so I can only show a generic error to the user now. I guess I should have realized that before I created the ticket 🙄 it seems there is a clash in feedback for this method: it both returns a bool and throws exceptions in not so unusual situations.

Since starting the activity does not really provide feedback, I think it would be better to have it return void and throw exceptions like it used to..

alexrintt commented 1 year ago

Seems good, I'll provide this as part of v8 since it's a breaking change. Would be helpful if you could provide any exceptions you would like to handle, for now I can think only on Activity not found, Security Exception (when no read permission) and Unknown.

mx1up commented 1 year ago

cool, the exceptions you listed are indeed the important ones, of which Activity not found would be the most common in my use case.

btw, in theory, I think 0.7.2 introduced a breaking change, not on api signature level, but on behavioral level, by not throwing exceptions anymore in cases where an exception was expected.

alexrintt commented 1 year ago

Think you are right. I was using signature level as metric for breaking vs minor changes.

Not sure how an app would get a exception from these changes though, who was handling exceptions + the boolean return value doesn't need to change anything, in last case the client will just ignore the exception (since it doesn't throw anything now), but anyway thanks for the warning, I'll take better care from now on.

alexrintt commented 1 year ago

Retracted version 0.7.2, since it was released only 8h ago, I think there are only few affected clients besides you, I'll release the fix of this issue at v8.

mx1up commented 1 year ago

great, sorry for all the hassle ;)

alexrintt commented 1 year ago

I'll add an API method openDocumentFileWithResult, it will return a Future<OpenDocumentFileResult> where OpenDocumentFileResult is a enum with all possible results (success, error A, B, C, etc.).

The current openDocumentFile will be a simple openDocumentFileWithResult() == success around this new method.

This let the old users migrate to the fixed openDocumentFile and let people implement custom catches if they want to, using the new openDocumentFileWithResult.

Gonna add details soon.

alexrintt commented 1 year ago

Launched openDocumentFileWithResult at v.0.8, see ref.

I'll mark as done but feel free to re-open if needed.