file_picker_writable icon indicating copy to clipboard operation
file_picker_writable copied to clipboard

Errors opening from intent/open-in/copy-to are swallowed

Open amake opened this issue 5 years ago • 5 comments

If an error/exception occurs while trying to open a file sent from another app by intent (Android) or "Open in"/"Copy to" (iOS), the error is logged (e.g. here) but nothing is reported to the Dart layer.

In case the application wants to show an error in this case, it would be good to have an ErrorHandler that one could register (alongside UriHandler, FileInfoHandler) to be notified when an error occurs.

amake avatar Jun 13 '20 14:06 amake

right, but this looks pretty much like the only place? I guess it would make sense to add a dedicated OpenErrorHandler which is for this specific case - when the plugin receives an open intent/callback from the OS.. otherwise for open file/write file/etc. method calls should continue to result in exceptions.

the only thing i'm not sure about is whether the init method should also result in an thrown error when an exception happens during _handleUrl .. i guess to keep consistent it should succeed.. and the error should be reported to the OpenErrorHandler

hpoul avatar Jun 14 '20 14:06 hpoul

right, but this looks pretty much like the only place?

Yes, sorry, I said “e.g.” because the equivalent entry point also exists on Android.

a dedicated OpenErrorHandler which is for this specific case

Yes, it would be single-purpose, but there’s really no other option to catch this case.

i guess to keep consistent it should succeed.. and the error should be reported to the OpenErrorHandler

I agree, I don’t think there’s a better choice.

amake avatar Jun 14 '20 23:06 amake

@amake I have now done a bit of refactoring, and also added such error handler for open-in/copy-to - only iOS right now.. and it only contains a string message .. this should probably something more useful the application can parse.. not sure yet what though..

https://github.com/hpoul/file_picker_writable/commit/f83262129a251adf7ec4ae764948418188e4e3e6#diff-f540102ae087b792214ddead720f73e1R337-R339

the handling works basically the same as for file open handlers.. https://github.com/hpoul/file_picker_writable/commit/f83262129a251adf7ec4ae764948418188e4e3e6#diff-d9a718a3042c2ebab14df5f34ef46a52R102-R112

btw. this is still a bit of a WIP, and I also did a quite bit of refactoring and want to move away from providing temporary files in FileInfo to only providing access to files via a callback..

https://github.com/hpoul/file_picker_writable/blob/f83262129a251adf7ec4ae764948418188e4e3e6/example/lib/main.dart#L162-L169

this way there will be no more left over temporary files.. once the app returns from the callback, file will be deleted immediately.

hpoul avatar Aug 19 '20 09:08 hpoul

Thanks for the update. I think a string message is good enough. Just being able to receive a notification is actually enough for me.

Regarding callbacks, I have to say I'm not a fan. My code is very much designed around Futures and async/await, so it will probably be painful to move to the new API. On the other hand I don't have any other good suggestions for achieving your goals.

amake avatar Aug 19 '20 13:08 amake

@amake the old api still works the same way for now, it's just deprecated.. and it does use futures and async.. basically nothing changes, except you have to process or copy the file to some location before the callback completes..

in AuthPass the change wasn't too painful https://github.com/authpass/authpass/commit/8f7a9528539d277ae469da7eb496d6856c168cf2

The result of the callbacks is passed back to the caller, so if you simply read the whole file at once, you could basically just do that in the callback and return a tuple of the FileInfo plus Uint8List (or string) with the file content ..

Maybe it would even make sense to put this into the package, a default callback which returns a future with the FileInfo plus the file content (either string or bytes), i guess not many use cases have to stream the file contents.. 🤔

hpoul avatar Aug 19 '20 14:08 hpoul