Closed Bug 978031 Opened 6 years ago Closed 6 years ago

[B2G][Dialer] Incorrect message after trying to use dialer in Airplane mode

Categories

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

x86_64
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed)

RESOLVED FIXED
1.4 S3 (14mar)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed

People

(Reporter: AndreiH, Assigned: rik)

References

Details

(Keywords: qablocker, regression, Whiteboard: [fromAutomation][xfail])

Attachments

(2 files)

Attached image Screenshot.png
STR:

1. Go to Settings and enable Airplane mode.
2. Go to dialer and dial a number and press call.

Expected results:
A message should appear like: Airplane mode activated.

Actual results:
Current message that appears is: Unable to make a phone call now. (see screenshot)

Build:
Gaia      3a9fdec737cdfd41206a332970fea833ec4ca13a                         
Gecko     https://hg.mozilla.org/mozilla-central/rev/4cfb6c61b137          
BuildID   20140227160204                                                   
Version   30.0a1
Blocks: 978024
Whiteboard: [fromAutomation][regression]
QA blocker - dialer automation is blocked due to this.

Why isn't there in tree tests to catch a regression like this?
blocking-b2g: --- → 1.4?
Whiteboard: [fromAutomation][regression] → [fromAutomation]
(In reply to Jason Smith [:jsmith] from comment #1)
> QA blocker - dialer automation is blocked due to this.
> 
> Why isn't there in tree tests to catch a regression like this?

You can't do this flow on desktopb2g. I'm not sure if a unittest could catch it. It probably needs the RIL.
QA Contact: jzimbrick
Regression Window:

Last Working Environmental Variables:
Device: Buri
BuildID: 20140227054712
Gaia: 22d48b62df7901ad45044f66e15e7d8943884a06
Gecko: 0b930fe64a1e
Version: 30.0a1
Base Image: V1.2-device.cfg

First Broken Envrionmental Variables:
Device: Buri
BuildID: 20140227062009
Gaia: 3a9fdec737cdfd41206a332970fea833ec4ca13a
Gecko: f8e801b991bc
Version: 30.0a1
Base Image: V1.2-device.cfg

Last Working Gaia/First Broken Gecko: Issue DOES reproduce.
Gaia: 22d48b62df7901ad45044f66e15e7d8943884a06
Gecko: f8e801b991bc

First Broken Gaia/Last Working Gecko: Issue does NOT reproduce.
Gaia: 3a9fdec737cdfd41206a332970fea833ec4ca13a
Gecko: 0b930fe64a1e

Gecko Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b930fe64a1e&tochange=f8e801b991bc
Regression from bug 969218.
Blocks: 969218
Component: Gaia::Dialer → RIL
Whiteboard: [fromAutomation] → [fromAutomation][xfail]
blocking-b2g: 1.4? → 1.4+
Note that could also be Bug 969280 as the new code was not used until bug 969218 landed.
(In reply to Julien Wajsberg [:julienw] from comment #5)
> Note that could also be Bug 969280 as the new code was not used until bug
> 969218 landed.

Zac, I wonder if you could help to verify this bug with latest Gaia, again. Thanks.
Flags: needinfo?(szchen)
Hi rik,

Could you help do some analysis for this issue.
I quickly go through the modification in Bug 969280

+      if (promiseOrCall && promiseOrCall.then) {
+        promiseOrCall.then(function(call) {
+          installHandlers(call, emergencyOnly, oncall, onconnected,
+                          ondisconnected, onerror);
+        }).catch(function(errorName) {
+          displayMessage('UnableToCall');
+        });
       }

It seems that it always display 'UnableToCall' error when got the promise rejection. Based on the STR, it should received 'RadioNotAvailable' as the errorName. Gaia should show the specific error based on different errorName.
Flags: needinfo?(szchen) → needinfo?(anthony)
(In reply to Jason Smith [:jsmith] from comment #1)
> QA blocker - dialer automation is blocked due to this.
How is that a QA blocker? From what I'm reading, it is displaying the wrong error message, not changing a flow. Maybe I'm not sure about the meaning of QA blocker.

> Why isn't there in tree tests to catch a regression like this?
We have tests, it was caught by some. We just don't have Travis/TBPL tests because we don't have an emulator to test anything related to radios. We have unit tests but I wrote it wrongly.
Flags: needinfo?(anthony)
(In reply to Szu-Yu Chen [:aknow] from comment #7)
> It seems that it always display 'UnableToCall' error when got the promise
> rejection. Based on the STR, it should received 'RadioNotAvailable' as the
> errorName. Gaia should show the specific error based on different errorName.
Could you list all the error names that we can receive when we have a promise rejected?

In the version before promises, when we received a RadioNotAvailable error, it was an error event on an existing call. That's why I assumed we would still get a call in that case.
Flags: needinfo?(szchen)
(In reply to Anthony Ricaud (:rik) from comment #8)
> (In reply to Jason Smith [:jsmith] from comment #1)
> > QA blocker - dialer automation is blocked due to this.
> How is that a QA blocker? From what I'm reading, it is displaying the wrong
> error message, not changing a flow. Maybe I'm not sure about the meaning of
> QA blocker.

Because it caused an automated UI test to be shut down that used to work with the Dialer in airplane mode.
(In reply to Anthony Ricaud (:rik) from comment #9)
> (In reply to Szu-Yu Chen [:aknow] from comment #7)
> > It seems that it always display 'UnableToCall' error when got the promise
> > rejection. Based on the STR, it should received 'RadioNotAvailable' as the
> > errorName. Gaia should show the specific error based on different errorName.
> Could you list all the error names that we can receive when we have a
> promise rejected?

In promise version, the returned promise will only be resolved as a call if gecko successfully dial it out. Otherwise, it will be rejected. Possible errorName are:

"BadNumberError"
"RadioNotAvailable"
"GenericFailure"       => ex: modem dial fail
"InvalidAccessError"   => ex: serviceId param error
"InvalidStateError"    => ex: there is already a dialing call

> In the version before promises, when we received a RadioNotAvailable error,
> it was an error event on an existing call. That's why I assumed we would
> still get a call in that case.

Sorry that we didn't clearly describe the modification.
Flags: needinfo?(szchen)
I suppose this work is on GAIA::Dialer. Feel free to correct me.
Component: RIL → Gaia::Dialer
Assignee: nobody → anthony
Szu-Yu: Could you confirm the unit tests part match the Gecko behaviour? https://github.com/Rik/gaia/blob/promise-error-978031/apps/communications/dialer/test/unit/telephony_helper_test.js#L368-L477
Attachment #8386270 - Flags: review?(etienne)
Attachment #8386270 - Flags: feedback?(szchen)
Comment on attachment 8386270 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16916

Comment on github, I think we still need to trigger the error callback passed to the TelephonyHelper.
Attachment #8386270 - Flags: review?(etienne)
(In reply to Anthony Ricaud (:rik) from comment #13)
> Created attachment 8386270 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/16916
> 
> Szu-Yu: Could you confirm the unit tests part match the Gecko behaviour?
> https://github.com/Rik/gaia/blob/promise-error-978031/apps/communications/
> dialer/test/unit/telephony_helper_test.js#L368-L477

In promise version, it is also possible to receive "BadNumberError" from call.onerror. Suggest to add an unit test for this scenario.

The case could be happened when the number format is correct and our phone successfully dial it out. Now we get a call object. However after it reach the network side. network think it doesn't match a existed destination. Then it might disconnect the call by "BadNumberError".

Except this, I think other cases are proper handled. Thank you.
Comment on attachment 8386270 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16916

Cool, r=me with Aknow's suggested test added.
Attachment #8386270 - Flags: review?(etienne) → review+
I copied the BadNumberError tests into the onerror tests and added some comments explaining why we could receive it both from Gecko and the network.

https://github.com/mozilla-b2g/gaia/commit/a351fe62c11737c722ad33aaff438f6ccd00bd4a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S3 (14mar)
You need to log in before you can comment on or make changes to this bug.