962447 - [fugu][b2g]dial a number out the fdn list,no error message be showed

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Ming]

steps to reproduce:
1.enable fdn
2.dial a number not in the fdn list

expect: error message tells failed by fdn.

but: i got nothing


logs maybe help: 
I/Gecko   (  123): TelephonyProvider: Dialing 10010
I/Gecko   (  123): RIL Worker[0]: Received chrome message {"number":"10010","isDialEmergency":false,"rilMessageToken":48,"rilMessageType":"dial"}

I/Gecko   (  123): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callError","callIndex":-1,"errorMsg":"GenericFailure"}
I/Gecko   (  123): TelephonyProvider: listener for notifyError threw an exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsITelephonyListener.notifyError]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js :: _notifyAllListeners :: line 158"  data: no]

Comment 1

[Ming]

dear vyang  
pls confirm this

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I will take a look.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ming from comment #0)
> steps to reproduce:
> 1.enable fdn
> 2.dial a number not in the fdn list
> 
> expect: error message tells failed by fdn.
> 
> but: i got nothing
> 
> 
> logs maybe help: 
> I/Gecko   (  123): TelephonyProvider: Dialing 10010
> I/Gecko   (  123): RIL Worker[0]: Received chrome message
> {"number":"10010","isDialEmergency":false,"rilMessageToken":48,
> "rilMessageType":"dial"}
> 
> I/Gecko   (  123): -*- RadioInterface[0]: Received message from worker:
> {"rilMessageType":"callError","callIndex":-1,"errorMsg":"GenericFailure"}
> I/Gecko   (  123): TelephonyProvider: listener for notifyError threw an
> exception: [Exception... "Component returned failure code: 0x8000ffff
> (NS_ERROR_UNEXPECTED) [nsITelephonyListener.notifyError]"  nsresult:
> "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame ::
> jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js ::
> _notifyAllListeners :: line 158"  data: no]

These exception messages are not really errors. In Telephony.cpp, we check if a Telephony instance is the one who requested dial() while we receive "notifyError" callback. If not, we don't dispatch error event on that instance. That's why we see NS_ERROR_UNEXPECTED from the log.

Then why is there no error messages popping out on dialer screen?
My observation:
1) From the log you pasted, the fugu rild/modem returns "GenericFailure." Nevertheless, [1] doesn't check "GenericFailure." 

Fugu rild/modem should make sure a specific error message is returned so that gaia could show the definite information to user.
 
2) [1] checks "FDNBlockedError." However, there's another error message "FdnCheckFailure" that would be sent by rild/modem. I tried some devices and "FdnCheckFailure" has been the only error type I got so far, when I was reproducing this issue. I am not very sure when "FDNBlockedError" is gonna happen though this is indeed defined in android ril.h.

So I'd suggest [1] adds one more supported error type -- FdnCheckFailure, in order to not miss this error message.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L101

Comment 4

[Ming]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)

> Then why is there no error messages popping out on dialer screen?
> My observation:
> 1) From the log you pasted, the fugu rild/modem returns "GenericFailure."
> Nevertheless, [1] doesn't check "GenericFailure." 
> 
> Fugu rild/modem should make sure a specific error message is returned so
> that gaia could show the definite information to user.

hi ,the specific cause for the failer of the last call can be get by ril request of  REQUEST_GET_LAST_CALL_FAIL. So why don't ril_worker do the request of REQUEST_GET_LAST_CALL_FAIL.
I remember that v1.2 does well on this case.
In fact ril/modem did told the specific cause :
D/AT      (  116): [w] Channel1: AT> ATD10010;
D/AT      (  116): [w] Channel0: AT< +ECEER: 241
D/AT      (  116): [w] Channel0: AT< ^CEND: 255,,101,241
D/RIL     (  116): [w] The last call fail cause: 241  
D/AT      (  116): [w] Channel1: AT< +CME ERROR: 21

this.CALL_FAIL_FDN_BLOCKED = 241;

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ming from comment #4)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)
> 
> > Then why is there no error messages popping out on dialer screen?
> > My observation:
> > 1) From the log you pasted, the fugu rild/modem returns "GenericFailure."
> > Nevertheless, [1] doesn't check "GenericFailure." 
> > 
> > Fugu rild/modem should make sure a specific error message is returned so
> > that gaia could show the definite information to user.
> 
> hi ,the specific cause for the failer of the last call can be get by ril
> request of  REQUEST_GET_LAST_CALL_FAIL. So why don't ril_worker do the
> request of REQUEST_GET_LAST_CALL_FAIL.
> I remember that v1.2 does well on this case.
> In fact ril/modem did told the specific cause :
> D/AT      (  116): [w] Channel1: AT> ATD10010;
> D/AT      (  116): [w] Channel0: AT< +ECEER: 241
> D/AT      (  116): [w] Channel0: AT< ^CEND: 255,,101,241
> D/RIL     (  116): [w] The last call fail cause: 241  
> D/AT      (  116): [w] Channel1: AT< +CME ERROR: 21
> 
> this.CALL_FAIL_FDN_BLOCKED = 241;

We do check that even in the current code.
However, the error happens on REQUEST_DIAL. That means, we don't even successfully make a call out. In this case it makes no sense to have additional call to REQUEST_GET_LAST_CALL_FAIL_CAUSE.

RIL_Errno in ril.h defines a specific error number "RIL_E_FDN_CHECK_FAILURE = 14" and says very clearly that the number is returned due to command failed because recipient is not on FDN list.

Comment 6

[Ming]

As you see , ril/modem get the error CME ERROR:21 ,which means "+CME ERROR: invalid index" .
So which RIL_Errno should it be ? It doesn't match the cause  "RIL_E_FDN_CHECK_FAILURE = 14"

RIL_E_FDN_CHECK_FAILURE = 14"

Code of v1.2  works well :

RIL[REQUEST_DIAL] = function REQUEST_DIAL(length, options) {
  if (options.rilRequestError) {
    // The connection is not established yet.
    options.callIndex = -1;
    this.getFailCauseCode(options);
    return;
  }
};

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Attachment: 962447.patch

Per our partner's request, we should use 'getFailCauseCode()' to get the error message even the call connection hasn't been established yet.

It works on other vendor's solution as well.

Comment 8

[Ming]

tks,also I will verify this with the modem team.

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ming from comment #8)
> tks,also I will verify this with the modem team.

No problem! Glad to see the issue being fixed as well :)

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

How does the solution here affect bug 944589?

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #10)
> How does the solution here affect bug 944589?

Hey Vicamo, 

Bug 944589 reported an issue: if a call is released unexpectedly, in TelephonyProvider.js the code path would be 'notifyCallError' instead of normal 'handleCallDisconnected.' Sadly, TelephonyProvider.js updated audio state only when handleCallDisconnected() been called. This fix of bug 944589 is to make sure we handle a disconnected call well no matter what the fail cause of a call is.

This patch changes the fail cause of a call that wouldn't and shouldn't affect bug 944589.

Also, here is how AOSP addresses this:

In AOSP, the response of REQUEST_DIAL is null. AOSP doesn't address the error of the request. Right after calling REQUEST_DIAL, a new call connection is created and tracked. Then, handlePollCalls() is called. If the new connection doesn't show up in CLCC list in polling, the connection is removed from the tracked list and getLastCallFailCause() is called. Though AOSP doesn't handle error of REQUEST_DIAL, different from us, this patch shares the same logica as AOSP regarding acquiring a fail cause of a call connection which isn't established or isn't in CLCC. Thanks!

Comment 12

[Ming]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)

> Fugu rild/modem should make sure a specific error message is returned so
> that gaia could show the definite information to user.

Hi hsin , which specific should i refer to?

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ming from comment #12)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)
> 
> > Fugu rild/modem should make sure a specific error message is returned so
> > that gaia could show the definite information to user.
> 
> Hi hsin , which specific should i refer to?

Hey Ming,

Our RIL is going to use getFailCauseCode(). The error expected for fdn case is "this.CALL_FAIL_FDN_BLOCKED = 241;" and I believe the fugu already supports per your comment 4.

Comment 14

[Ming]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #13)
Ok , tks
Yes , supports .

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

b2g-inboud TREE is closed. I'll push the patch once it is open again.

Comment 16

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #15)
> b2g-inboud TREE is closed. I'll push the patch once it is open again.

Still closed... waiting...

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/2a4544efc702

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2a4544efc702

Comment 19

[Ming]

pls merge this patch into v1.3 ,needed by TARA

Comment 20

[Ryan VanderMeulen [:RyanVM]]

All 1.3 uplifts have to be approved by Fabrice. Please nominate the patch for approval-mozilla-b2g28.

Comment 22

[Preeti Raghunath(:Preeti)]

1.3+ for the same. Bad FDN error

Comment 23

[Ryan VanderMeulen [:RyanVM]]

This can't be uplifted to v1.3 until the patch has approval-mozilla-b2g28.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #23)
> This can't be uplifted to v1.3 until the patch has approval-mozilla-b2g28.

Comment 25

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Attachment: 962447-v1_3.patch -- patch for b2g28_v1_3

patch for b2g28_v1_3

Comment 26

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8378800 [details] [diff] [review]
962447-v1_3.patch -- patch for b2g28_v1_3

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 962447
User impact if declined: user has no idea a phone number can't be dialed out because the number is blocked by FDN
Testing completed: yes
Risk to taking this patch (and alternatives if risky): very low
String or UUID changes made by this patch: no

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/53254f306e44