1154993 - [flame][3.0]Can't view SMS that has no phone number associated

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Robert Kaiser]

SMS I get from some services, including from my bank for transaction codes, or from my operator for missed calls, only have a name but no phone number assigned.

Right now on master/nightly ("3.0") with my Flame, I neither get a notification for them, nor can I view them in the SMS app.

When I tap the unread message in the SMS app to try and view it, I get the following messages in logcat:

I/Gecko   (  207): ContactDB: normalized filterValue is empty, can't perform match search.
E/Messages( 1353): Content JS ERROR: Got an error while looking up contacts. UnknownError [object DOMError] 
E/Messages( 1353):     at contacts_findBy/< (app://sms.gaiamobile.org/gaia_build_defer_index.js:419:31)
E/Messages( 1353): Content JS ERROR: Navigation.toPanel: got an exception: ([object DOMError]) undefined 
E/Messages( 1353):     at catchError (app://sms.gaiamobile.org/gaia_build_defer_index.js:378:193)
E/Messages( 1353):     at rejected (app://sms.gaiamobile.org/gaia_build_defer_index.js:396:170)

Comment 1

[Steve Chung [:steveck]]

May I know what kind of services could send the message without number but still have name displayed? Maybe some more image for the receiving the notification and message threads view would be helpful, thanks!

Comment 2

[Julien Wajsberg [:julienw]]

[Blocking Requested - why for this release]:

Likely a regression from bug 1142540.

I do have such cases as well. I think the "sender" property contains an arbitrary string that's not a number. As seen above, we get an error from mozContacts in that case (if there are only non-digits).

Comment 3

[Julien Wajsberg [:julienw]]

So we have a rejected promise at the end of the chain, because we assumed mozContacts error should not be recoverable. But we see here that at least one case should be recoverable.

But we can't distinguish easily between a real error and this error.

So here are 2 possible fixes:
* do not call mozContacts if no character is a digit.
* mozContacts should not return an error if the input is non-empty but is normalized as an empty value. It should just return an empty result.

NI Gregor on the second possible fix, which looks like the most correct to me.

Comment 4

[Gregor Wagner [:gwagner]]

Reuben, can you take a look here?

Comment 5

[Julien Wajsberg [:julienw]]

I can also do the change if you think the change should be in the Contacts API.

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Just my 2p: this is horrible for dogfooding, if it cannot be fixed very quickly backing out bug 1142540 (which fixes the issue, tested locally) and re-landing it once the underlying problem is solved. To give you the feeling of how bad it is I can't access messages from 2FA services that send you codes including GitHub, my bank and the Dutch DigiD system for accessing government sites.

Comment 7

[Julien Wajsberg [:julienw]]

Let me take this, I'll prepare a patch for mozContacts today.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] julienw:1154993-recover-contacts-errors > mozilla-b2g:master

Comment 9

[Julien Wajsberg [:julienw]]

Comment on attachment 8594706 [details] [review]
[gaia] julienw:1154993-recover-contacts-errors > mozilla-b2g:master

After further thinking, I now think it's always bad to choke on some API errors; basic functionality should never break.

So let's recover from mozContacts error which is what we used to do before bug 1142540. I checked that this fixes the issue for me.

I still think that mozContacts should not send an error in this case, but this could be handled in a separate bug.

Comment 10

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8594706 [details] [review]
[gaia] julienw:1154993-recover-contacts-errors > mozilla-b2g:master

PR looks good to me, but how do you feel about small integration test for this bug to avoid such breakage in the future? 

Please, re-request review if you decide to do it in this PR, otherwise I'm fine with test-only follow-up.

Comment 11

[Julien Wajsberg [:julienw]]

Filed bug 1156288 as a follow-up for integration tests.

Comment 12

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/36f7977a13a9b3b42de25af5fa69c115c5d68bfb

Comment 14

[Johan Lorenzo [:jlorenzo]]

Already landed in 3.0.

Comment 15

[Reuben Morais [:reuben]]

There's no contract that says API errors are all unrecoverable, and in fact this isn't the only recoverable error we throw in the API. I don't think changing the behavior is worth the trouble in this case.