Submitting almost any form from the mobile app throws and catches a Java error

[dianabarsan]

In 3.5 we introduced the possibility of sending webapp forms to the server vis SMS: #4812
The code that generates the SMS content is here: https://github.com/medic/cht-core/blob/master/webapp/src/js/services/form2sms.js
The code that actually attempts sms sending is here: https://github.com/medic/cht-core/blob/master/webapp/src/js/services/submit-form-by-sms.js

We check for a field xml2sms in the form doc and fallback to the default odk-xform-compact-record-representation-for-sms - which is documented here: https://opendatakit.github.io/xforms-spec/#compact-record-representation-(for-sms)
In order to have a form be compactable to SMS, root element needs to have a prefix attribute. However, all form templates that I've seen used in our configurations have this prefix attribute, so all are compactable.
examples:
https://github.com/medic/cht-core/blob/master/config/default/forms/app/pregnancy.xml#L671
https://github.com/medic/cht-core/blob/master/config/default/forms/app/undo_death_report.xml#L47
https://github.com/medic/cht-core/blob/master/config/default/forms/app/pregnancy_facility_visit_reminder.xml#L68
And every other default config form.

When submitting such a form in the mobile app, an error is thrown and caught by the SubmitFormBySMS service:

Execption thrown in JavascriptInterface function: java.lang.SecurityException: Sending SMS message: uid 10272 does not have android.permission.SEND_SMS.;  at android.os.Parcel.createException(Parcel.java:1950);  at android.os.Parcel.readException(Parcel.java:1918);  at android.os.Parcel.readException(Parcel.java:1868);  at com.android.internal.telephony.ISms$Stub$Proxy.sendTextForSubscriber(ISms.java:881);  at android.telephony.SmsManager.sendTextMessageInternal(SmsManager.java:381);  at android.telephony.SmsManager.sendTextMessage(SmsManager.java:364);  at android.telephony.SmsManager.sendMultipartTextMessageInternal(SmsManager.java:697);  at android.telephony.SmsManager.sendMultipartTextMessage(SmsManager.java:663);  at org.medicmobile.webapp.mobile.j.sms_send(MedicAndroidJavascript.java:2063);  at org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method);  at org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:39);  at android.os.Handler.dispatchMessage(Handler.java:107);  at android.os.Looper.loop(Looper.java:198);  at android.os.HandlerThread.run(HandlerThread.java:65); 

This ends up polluting the console and nearly every feedback document generated (since it includes the latest n console messages).
It also contains a typo Execption.
Typo appears to be coming from here: https://github.com/medic/medic-android/blob/master/src/main/java/org/medicmobile/webapp/mobile/MedicAndroidJavascript.java#L407

This also affects live instances - I first saw the error message in a feedback document generated in muso-mali instance. The feedback document reported a different error (fixed here: #5863) but it included a couple of previous log entries.

To Reproduce
Steps to reproduce the behavior:

1. Deploy default configuration.
2. Login as any user in the mobile app and watch the console
3. Submit any form.
4. See that after submission, you get an error in the console.
   Optional: open the report a bug interface and submit. Check the document uploaded to the meta database on the server and observe it contains the Java exception log.

Expected behavior
We should not attempt sending SMS when the app does not have permissions.

Logs

submitFormBySmsIfApplicable() failed: Error: Java exception was raised during method invocation(anonymous function) @ angular.js:15567(anonymous function) @ submit-form-by-sms.js:12processQueue @ angular.js:17945(anonymous function) @ angular.js:17993$digest @ angular.js:19112(anonymous function) @ angular.js:19432completeTask @ angular.js:21231(anonymous function) @ angular.js:6812
feedback.js:39 Execption thrown in JavascriptInterface function: java.lang.SecurityException: Sending SMS message: uid 10272 does not have android.permission.SEND_SMS.;  at android.os.Parcel.createException(Parcel.java:1950);  at android.os.Parcel.readException(Parcel.java:1918);  at android.os.Parcel.readException(Parcel.java:1868);  at com.android.internal.telephony.ISms$Stub$Proxy.sendTextForSubscriber(ISms.java:881);  at android.telephony.SmsManager.sendTextMessageInternal(SmsManager.java:381);  at android.telephony.SmsManager.sendTextMessage(SmsManager.java:364);  at android.telephony.SmsManager.sendMultipartTextMessageInternal(SmsManager.java:697);  at android.telephony.SmsManager.sendMultipartTextMessage(SmsManager.java:663);  at org.medicmobile.webapp.mobile.j.sms_send(MedicAndroidJavascript.java:2063);  at org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method);  at org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:39);  at android.os.Handler.dispatchMessage(Handler.java:107);  at android.os.Looper.loop(Looper.java:198);  at android.os.HandlerThread.run(HandlerThread.java:65); 

Screenshots
If applicable, add screenshots to help explain your problem.

Environment

• Browser: mobile app
• Client platform: Android
• App: webapp
• Version: 3.5+

[garethbowen]

I think we probably need a better way to detect whether the form should be sent by sms as even if you have the capability to send via SMS you probably don't want to do that for all form types. Clearly the prefix is not the right flag to use. Consider adding a new property to the form doc as well.

[SCdF]

Ready for AT in 6162-forms-dont-send-sms-by-default

• See documentation in Don't default App forms to send SMS medic-docs#204 , it should work as this documentation states (and the documentation should make sense)

[SCdF]

We are having heaps of trouble reproducing this because it is challenging to get the logs off the android app, since Chromium no longer attaches its debugger properly and a very clever hack by Diana around using feedback docs isn't accurate enough to catch the log.

I'm going to have a think and see if there is another way we can AT this, but we may just say that because the act of saving reports still works this hasn't broken anything

[dianabarsan]

I managed to replicate this this morning by submitting a form and then immediately submitting a bug report. The resulting "feedback" doc contained the error in the logs.

I've replicated it on two devices, the Tecno and my personal Nokia, so I'd say it's doable.

[garethbowen]

Another option would be to install an old version of Chrome/Chromium and connect that to the device.

[dianabarsan]

I believe the problem is Chrome removing an old deprecated feature (https://developer.mozilla.org/en-US/docs/Web/API/Document/registerElement) that's used by the old inspector script. (If you open devtools in that blank debugger page, you can see an error about registerElement not being defined).

I had this idea that browser extensions might be loaded on that page, so we could leverage those and inject a polyfill of registerElement ourselves. Unfortunately, they're not loaded by default on pages with devtools:// scheme. However, the extension can extend the DevTools themselves and force a script injection when DevTools are open.
However, polyfilling registerElement to use customElements.define is a more complicated than I expected, and every crash I fix just surfaces the next one (registerElement is not the only deprecated feature that's been removed).
I'm determined, though, to try some more, probably during the weekend.

[garethbowen]

In the meantime I believe AT can continue on this issue using the feedback doc trick Diana mentioned above.

[SCdF]

After far too much faffing around I managed to reproduce this. No more log, everything else worked for me AFAICT. Back to you @dianabarsan to merge!

[dianabarsan]

Merged