search

jfversluis / FilePicker-Plugin-for-Xamarin-and-Windows

FilePicker Plugin for Xamarin and Windows
MIT License
157 stars 80 forks source link

NRE as since 2.1.xx the plugin is released to NuGet in Debug configuration #142

Closed VitalyKnyazev closed 5 years ago

VitalyKnyazev commented 5 years ago

As since 2.1.xx the plugin is released to NuGet in Debug configuration (surprisingly enough) this line causes NRE for some Android versions #143

Description of Change

Moved Debug.Write into try/catch block to fix NRE crash when "data" is null.

Issues Resolved

Platforms Affected

vividos commented 5 years ago

Thanks for your PR! I don't know if this really fixes the NRE. Did you test that on the actual device? Does it fix the error?

My reason is that when you move the System.Diagnostics.Debug.Write() call into the try-catch block, and the call actually throws an exception, the Write() method in the catch block then would also throw an exception, which doesn't help you in fixing that bug. You're leaving the OnActivityResult() method, Android will catch and throw away your exception, but FilePickCancelled?.Invoke() is not called.

VitalyKnyazev commented 5 years ago

@vividos I have not seen the error on physical device, just exception reports in MS App Center. I have moved the Write call under the common try/catch so that if "data" is null we get NRE and catch block gets executed thus calling FilePickCancelled.

vividos commented 5 years ago

But there's a Write() call in the catch block as well, so when that also throws, FilePickCancelled is also not called. The real fix is to distribute Release assemblies, not Debug. Please check out the latest beta 2.1.31-beta, which contains Release binaries. See also your issue #143.

VitalyKnyazev commented 5 years ago

@vividos No, Write call in catch block just outputs exception details, it has nothing to do with "data" variable which is null. I agree that Release config is the best way to resolve the issue. The fix is only necessary if you had a legitimate reason to use Debug config in prod or just if someone compiles the plugin in Debug mode and uses it in their app. I downloaded 2.1.31-beta, will release my app soon and watch for a few weeks for similar crash reports.

vividos commented 5 years ago

Ah, now I understand. The NRE is not due to Write() being called in Release builds, but because the data variable is null.

VitalyKnyazev commented 5 years ago

Throwing when data is null, please let me know if you would prefer different wording.

VitalyKnyazev commented 5 years ago

Done

jfversluis commented 5 years ago

Voila https://www.nuget.org/packages/Xamarin.Plugin.FilePicker/2.1.32-beta