instedd / verboice

Open source toolkit for voice services; with special focus to the needs of medium- and low-income countries, scalable services, and interacting with vulnerable populations
http://verboice.instedd.org/
GNU General Public License v3.0
43 stars 18 forks source link

Properly handle Africa's Talking voice notifications callbacks #902

Closed matiasgarciaisaia closed 2 years ago

matiasgarciaisaia commented 2 years ago

Verboice is generating lots of timeout errors waiting for Africa's Talking to send it a callback - only the callback has already arrived, but is being mishandled.

2022-03-17T09:04:04.668550391Z 09:04:04.668 [info] [<0.25612.18>|25799486] session: Session ("22dacf1b-9a94-4deb-9a9e-ae1bc5e12d95") dial
2022-03-17T09:04:04.798845483Z 09:04:04.798 [info] [<0.25612.18>|25799486] session: Dialing to 256xxxxxxxxx through channel Uganda AfT
2022-03-17T09:04:10.074867770Z 09:04:10.074 [info] [<0.25639.18>|undefined] africas_talking_httpd_module: Receiving callSessionState [{"callSessionState","NotAnswered"},{"hangupCause","SUBSCRIBER_ABSENT"},{"direction","Outbound"},{"callerCountryCode","256"},{"durationInSeconds","0"},{"amount","0"},{"callerNumber","+256xxxxxxxxx"},{"destinationNumber","+256yyyyyyyyyy"},{"callerCarrierName","MTN"},{"status","NotAnswered"},{"sessionId","ATVId_221c9e6c4045a27a3a94e71a49a90b8c"},{"callStartTime","None"},{"isActive","0"},{"currencyCode","UGX"}]
2022-03-17T09:04:10.075767837Z 09:04:10.075 [info] [<0.25612.18>|25799486] session: Session ("22dacf1b-9a94-4deb-9a9e-ae1bc5e12d95") answer
2022-03-17T09:09:10.162577085Z 09:09:10.160 [info] [<0.25612.18>|25799486] session: Call failed with reason {internal_error,{timeout,{gen_server,call,[<0.25640.18>,{capture,{url,"https://surveda-africa.org/audio/c291e180-xxxx-yyyy-zzzz-aaaaaaaaaaaa.mp3"},5,[],1,1},300000]}}}
2022-03-17T09:09:10.170162250Z 09:09:10.161 [warning] [<0.25612.18>|25799486] session: Session ("22dacf1b-9a94-4deb-9a9e-ae1bc5e12d95") terminated with reason: {timeout,{gen_server,call,[<0.25640.18>,{capture,{url,"https://surveda-africa.org/audio/c291e180-xxxx-yyyy-zzzz-aaaaaaaaaaaa.mp3"},5,[],1,1},300000]}}

The call in Verboice is this one: https://verboice.surveda-africa.org/calls/548

See https://developers.africastalking.com/docs/voice/notifications for AfT's official docs. See #894 for the previous issue, and #895 for an incomplete attempt to fix it.

ysbaddaden commented 2 years ago

Some notes (as I read the documentation and the bugs):

Looking at AfT's documentation, they don't define values for callSessionState. It's probably an enum string, but :shrug:

AfT documents that callSessionState is only sent with the final notification, and they even emphasize it, that would mean that its presence or absence could be used to determine if a call is ongoing or done. Hence the initial match to proceed when undefined.

It was noticed in #894 that AfT broke their notification API: callSessionState seems to be always sent. That was taken care of in #895 to proceed unless Completed. Yet, we still don't know all the possible values, and we now have this issue, where callSessionState can be NotAnswered, and we can only guess there are (or will be) some other values :sob:

Ideas:

ping @matiasgarciaisaia

matiasgarciaisaia commented 2 years ago

I agree with using isActive.

It seems that some time ago AfT used to only send callSessionState when the call was finished - that's why we were assuming a finished called without checking. But they must have changed that, and now what defines a 100% surely finished call seems to be the isActive value - so I'd go with that 👌