Anki-android: [2.9 ACRA] crash adding bitmap to card

[mikehardy]

https://couchdb.ankidroid.org/acralyzer/_design/acralyzer/index.html#/report-details/47c1cab0-f57c-4162-aa09-f09d416bd11b

java.lang.RuntimeException: Failure delivering result ResultInfo{who=null, request=2, result=-1, data=Intent { act=inline-data (has extras) }} to activity {com.ichi2.anki/com.ichi2.anki.multimediacard.activity.MultimediaEditFieldActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
at android.app.ActivityThread.deliverResults(ActivityThread.java:4382)
at android.app.ActivityThread.handleSendResult(ActivityThread.java:4426)
at android.app.ActivityThread.-wrap20(Unknown Source:0)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1685)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loop(Looper.java:164)
at android.app.ActivityThread.main(ActivityThread.java:6626)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:811)
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
at com.ichi2.anki.multimediacard.fields.BasicImageFieldController.rotateAndCompress(BasicImageFieldController.java:6)
at com.ichi2.anki.multimediacard.fields.BasicImageFieldController.onActivityResult(BasicImageFieldController.java:9)
at com.ichi2.anki.multimediacard.activity.MultimediaEditFieldActivity.onActivityResult(MultimediaEditFieldActivity.java:3)
at android.app.Activity.dispatchActivityResult(Activity.java:7305)
at android.app.ActivityThread.deliverResults(ActivityThread.java:4378)
... 9 more

[Arthur-Milchior]

Is there a standard process to show a user an error message ?
Because I believe that the only thing which can reasonably done for this error is to state to the user that «the image could not be read as a bitmap file» (too technical ? «as an image» maybe ?) and that it failed.

The original problem is BitmapUtil#decodeFile, which returns null when it gets IOException, and this null is not taken into account

[timrae]

There's no standard process, but see #5561 for a good example

[david-allison-1]

Cause:

https://github.com/ankidroid/Anki-Android/blob/b3d6cb38f3412905462ca49c74ba8baaf66282b6/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldController.java#L222-L227

On Exception, BitmapUtil.decodeFile returns null.

https://github.com/ankidroid/Anki-Android/blob/b4361f4f6c2b71f59f851e79db8cc5be7c4f3171/AnkiDroid/src/main/java/com/ichi2/utils/BitmapUtil.java#L37-L67

We can rotate a null, but we can't compress a null.

[david-allison-1]

Our context is that we're in rotateAndCompress, we have a file path, and we want to decode, shrink, rotate and compress the image, and return a file path to the new image.

If we can't generate a Bitmap, then we can't do any of that.

Proposal:

• Display a Toast stating that file compression failed
• Add the filesize to the screen.

[mikehardy]

@david-allison-1 that sounds fine for user notification (the toast), when you say "add filesize to the screen" do you mean to the toast? As in "Failed to compress 0 bytes" or similar? Wording might be better if it is not technical also, e.g. "Failed to add image of 0 bytes" the card. There will be the problem of making the quantity human-friendly perhaps? Failed to add 345,456,432 bytes may not be meaningful, kilobytes might be the best tradeoff for length / precision? But implementor's choice in many ways - I'm just thinking through it out loud on the first cup of coffee

[david-allison-1]

@mikehardy

I'd like the toast to display "Failed to compress image" and the screen to appear as such:

This gives the user the choice about what to do with the image, rather than completely failing.

[david-allison-1]

I was just testing this, and the compression made my images bigger. 1.46-> 1.78 MB, so wasn't really sure how to handle that.

[mikehardy]

Okay - what's the decision tree for the user? They see the toast and they're thinking what to do - if I'm the user I am now nervous to hit any of the buttons. If I save it is the image busted? Do I retake?

Also just in general in the area cropping (or I should say - allowing the option to farm out an image edit via Intent) was really close to done #5301

[david-allison-1]

This is one that I'm not too happy with, I'll have a think and get back to it later.

• Allowing the user to take images is better than failing the process if they can't be compressed.
• We don't know why the images are failing to be compressed, and we should get this information to see if there's a common case that we can avoid.
• We likely want to inform the user that the image may be larger than normal
  
  
  
  • An advanced user will understand that
  
  
  • With a less advanced user, I'm unsure.
  
  

[mikehardy]

We may gather information via sendExceptionReport for non fatal things, and you can also record analytics events (though they have less data) if you're interested in a feature's usage in general. Let me see about getting you access to acralyzer (for crashes/exception reports) and analytics (for feature usage) so you have the ability to analyze those

[david-allison-1]

OK, had a think: frame it as a warning about increasing the collection size for now, and we can deal with this much better once opening the file in an external application is supported.

New plans:

• Get a fix for the crash out
• If we get a failure, warn the user that the image is large, and the impact of that in a TextView without mentioning the compression failure.
• Add a logcat & exception report for the failure
• Add a follow-up issue to deal with improving usability further.

[mikehardy]

Sounds good