fix: finalize WhatsApp calls when terminate webhook overtakes connect (#14836)
## Description Some inbound WhatsApp calls stayed stuck in "ringing" forever. When a caller hung up within ~1s of dialing, Meta delivered the terminate webhook before the connect webhook. The terminate arrived with no call record yet and was dropped, then connect created the call in ringing with nothing left to close it. These calls now correctly land as missed (no_answer), and an agent who taps Accept on a call that already ended gets a clean "call ended" instead of a generic error. ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - local UI testing ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
@@ -161,6 +161,7 @@ const buildCallActions = ({ callsStore, whatsappSession, t }) => {
|
||||
return { conferenceSid: joinResponse?.conference_sid };
|
||||
} catch (error) {
|
||||
useAlert(error?.response?.data?.error || t('CONTACT_PANEL.CALL_FAILED'));
|
||||
// 409 = the call already ended before accept landed (e.g. caller hung up mid-ring).
|
||||
if (error?.response?.status === 409) {
|
||||
TwilioVoiceClient.endClientCall();
|
||||
markDismissed(callSid);
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
import { CONTENT_TYPES } from 'dashboard/components-next/message/constants';
|
||||
import {
|
||||
CONTENT_TYPES,
|
||||
VOICE_CALL_STATUS,
|
||||
} from 'dashboard/components-next/message/constants';
|
||||
import { MESSAGE_TYPE } from 'shared/constants/messages';
|
||||
import { useCallsStore } from 'dashboard/stores/calls';
|
||||
import types from 'dashboard/store/mutation-types';
|
||||
@@ -101,6 +104,7 @@ export function handleVoiceCallCreated(
|
||||
callSid,
|
||||
callId,
|
||||
provider,
|
||||
status,
|
||||
callDirection,
|
||||
conversationId,
|
||||
inboxId,
|
||||
@@ -108,6 +112,11 @@ export function handleVoiceCallCreated(
|
||||
senderId,
|
||||
} = extractCallData(message);
|
||||
|
||||
// A voice_call message can be created already terminal when the caller hangs
|
||||
// up before connect. Only ring while the call is actually ringing; mirrors the
|
||||
// guard in seedCallsFromHydratedMessages.
|
||||
if (status !== VOICE_CALL_STATUS.RINGING) return;
|
||||
|
||||
if (
|
||||
!shouldShowCall({
|
||||
callDirection,
|
||||
|
||||
@@ -134,6 +134,7 @@ en:
|
||||
not_supported: 'Reauthorization is not supported for this type of WhatsApp channel.'
|
||||
calls:
|
||||
not_enabled: 'Calling is not enabled for this inbox'
|
||||
already_ended: 'This call has already ended'
|
||||
no_recording: 'No recording file provided'
|
||||
no_message: 'Call has no associated message'
|
||||
sdp_offer_required: 'sdp_offer is required'
|
||||
|
||||
@@ -13,6 +13,7 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro
|
||||
Voice::CallErrors::AlreadyAccepted,
|
||||
Voice::CallErrors::CallFailed,
|
||||
with: :render_call_error
|
||||
rescue_from Voice::CallErrors::CallAlreadyEnded, with: :render_call_ended
|
||||
rescue_from Voice::CallErrors::NoCallPermission, with: :render_permission_request
|
||||
|
||||
def show; end
|
||||
@@ -190,4 +191,9 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro
|
||||
def render_call_error(error)
|
||||
render_could_not_create_error(error.message)
|
||||
end
|
||||
|
||||
# 409 (not 422) so the FE can tell "already ended" from a generic failure and dismiss the ringing UI.
|
||||
def render_call_ended
|
||||
render json: { error: I18n.t('errors.whatsapp.calls.already_ended') }, status: :conflict
|
||||
end
|
||||
end
|
||||
|
||||
@@ -48,9 +48,10 @@ class Whatsapp::CallService
|
||||
private
|
||||
|
||||
def transition_to_in_progress!
|
||||
# Order matters: in_progress and terminal both make ringing? false, so we have to
|
||||
# branch on in_progress? first to surface the distinct AlreadyAccepted state.
|
||||
# in_progress and terminal both make ringing? false; branch in order to surface the
|
||||
# distinct AlreadyAccepted / CallAlreadyEnded states (caller can hang up mid-ring).
|
||||
raise Voice::CallErrors::AlreadyAccepted, 'Call already accepted by another agent' if call.in_progress?
|
||||
raise Voice::CallErrors::CallAlreadyEnded, 'Call already ended' if call.terminal?
|
||||
raise Voice::CallErrors::NotRinging, 'Call is not in ringing state' unless call.ringing?
|
||||
|
||||
forward_answer_to_meta!
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
class Whatsapp::IncomingCallService
|
||||
pattr_initialize [:inbox!, :params!]
|
||||
|
||||
# Lifespan of a terminate-before-connect tombstone; the paired connect arrives within ~1s.
|
||||
TERMINATE_TOMBSTONE_TTL = 60
|
||||
|
||||
def perform
|
||||
return unless inbox.channel.voice_enabled?
|
||||
|
||||
@@ -79,16 +82,32 @@ class Whatsapp::IncomingCallService
|
||||
end
|
||||
|
||||
sdp_offer = payload.dig(:session, :sdp)
|
||||
call = build_inbound_call(payload, sdp_offer)
|
||||
|
||||
return if call.terminal? # terminated before pickup; no ringing widget to surface
|
||||
|
||||
update_conversation(call)
|
||||
broadcast_incoming(call, sdp_offer)
|
||||
end
|
||||
|
||||
# If a terminate already arrived (caller hung up before pickup), finalize it in the
|
||||
# SAME transaction as the build so the message's after_create_commit fires (at outer
|
||||
# commit) already terminal, never `ringing` — agents aren't rung for a dead call.
|
||||
def build_inbound_call(payload, sdp_offer)
|
||||
ActiveRecord::Base.transaction do
|
||||
call = Voice::InboundCallBuilder.perform!(inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id],
|
||||
provider: :whatsapp, extra_meta: inbound_extra_meta(payload, sdp_offer))
|
||||
tombstone = consume_terminate_tombstone(payload[:id])
|
||||
finalize_terminate(call, tombstone['duration'], tombstone['terminate_reason']) if tombstone
|
||||
call
|
||||
end
|
||||
end
|
||||
|
||||
def inbound_extra_meta(payload, sdp_offer)
|
||||
extra_meta = { 'sdp_offer' => sdp_offer, 'ice_servers' => Call.default_ice_servers }
|
||||
name = caller_profile_name(payload)
|
||||
extra_meta['contact_name'] = name if name.present?
|
||||
|
||||
call = Voice::InboundCallBuilder.perform!(
|
||||
inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id],
|
||||
provider: :whatsapp, extra_meta: extra_meta
|
||||
)
|
||||
update_conversation(call)
|
||||
broadcast_incoming(call, sdp_offer)
|
||||
extra_meta
|
||||
end
|
||||
|
||||
# Match strictly on wa_id (== calls[].from): in a batched payload missing this
|
||||
@@ -122,23 +141,24 @@ class Whatsapp::IncomingCallService
|
||||
def handle_terminate(payload)
|
||||
call = Call.whatsapp.find_by(provider_call_id: payload[:id])
|
||||
if call.nil?
|
||||
# No row yet means either an out-of-order terminate (rare in practice — Meta
|
||||
# delivery is FIFO) or, more dangerously, an outbound terminate landing in
|
||||
# the window between the controller's Meta API call and Call.create!.
|
||||
# Materialising as inbound here would collide with the unique
|
||||
# (provider, provider_call_id) index. Skip; controller commits seal it.
|
||||
Rails.logger.warn "[WHATSAPP CALL] Terminate for unknown call #{payload[:id]}; skipping"
|
||||
# Terminate overtook its connect (Meta isn't strictly ordered); tombstone it for the
|
||||
# connect handler to consume. An outbound tombstone just expires unused.
|
||||
record_terminate_tombstone(payload)
|
||||
return
|
||||
end
|
||||
|
||||
finalize_terminate(call, payload[:duration], payload[:terminate_reason])
|
||||
end
|
||||
|
||||
def finalize_terminate(call, duration, reason)
|
||||
duration = duration&.to_i
|
||||
reason = reason.to_s
|
||||
call.with_lock do
|
||||
# Webhook retries can re-deliver terminate after we've already finalized the
|
||||
# call; don't recompute status or a duration=0 retry can flip a completed
|
||||
# short call back to no_answer.
|
||||
next if call.terminal?
|
||||
|
||||
duration = payload[:duration]&.to_i
|
||||
reason = payload[:terminate_reason].to_s
|
||||
status = derive_terminate_status(call, duration, reason)
|
||||
meta = (call.meta || {}).merge('ended_at' => Time.zone.now.to_i)
|
||||
update_call!(call, status, duration_seconds: duration, end_reason: reason, meta: meta)
|
||||
@@ -146,6 +166,28 @@ class Whatsapp::IncomingCallService
|
||||
end
|
||||
end
|
||||
|
||||
def record_terminate_tombstone(payload)
|
||||
Redis::Alfred.setex(
|
||||
terminate_tombstone_key(payload[:id]),
|
||||
{ 'duration' => payload[:duration], 'terminate_reason' => payload[:terminate_reason] }.to_json,
|
||||
TERMINATE_TOMBSTONE_TTL
|
||||
)
|
||||
Rails.logger.info "[WHATSAPP CALL] Terminate before connect for #{payload[:id]}; tombstoned"
|
||||
end
|
||||
|
||||
def consume_terminate_tombstone(provider_call_id)
|
||||
key = terminate_tombstone_key(provider_call_id)
|
||||
raw = Redis::Alfred.get(key)
|
||||
return nil if raw.blank?
|
||||
|
||||
Redis::Alfred.delete(key)
|
||||
JSON.parse(raw)
|
||||
end
|
||||
|
||||
def terminate_tombstone_key(provider_call_id)
|
||||
format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: provider_call_id)
|
||||
end
|
||||
|
||||
# Provider-reported failures trump the answered/no_answer heuristic. An
|
||||
# in_progress call that Meta later terminates with a failure reason would
|
||||
# otherwise be recorded as 'completed' purely because it had been accepted.
|
||||
|
||||
@@ -8,4 +8,5 @@ module Voice::CallErrors
|
||||
class CallFailed < StandardError; end
|
||||
class NotRinging < StandardError; end
|
||||
class AlreadyAccepted < StandardError; end
|
||||
class CallAlreadyEnded < StandardError; end
|
||||
end
|
||||
|
||||
@@ -58,6 +58,8 @@ module Redis::RedisKeys
|
||||
# Check if a message create with same source-id is in progress?
|
||||
MESSAGE_SOURCE_KEY = 'MESSAGE_SOURCE_KEY::%<id>s'.freeze
|
||||
OPENAI_CONVERSATION_KEY = 'OPEN_AI_CONVERSATION_KEY::V1::%<event_name>s::%<conversation_id>d::%<updated_at>d'.freeze
|
||||
# Bridges a WhatsApp call `terminate` that overtook its `connect` so the later connect can finalize it.
|
||||
WHATSAPP_CALL_TERMINATE_TOMBSTONE = 'WHATSAPP_CALL_TERMINATE_TOMBSTONE::%<call_id>s'.freeze
|
||||
|
||||
## Sempahores / Locks
|
||||
# We don't want to process messages from the same sender concurrently to prevent creating double conversations
|
||||
|
||||
@@ -58,6 +58,15 @@ RSpec.describe 'WhatsApp Calls API', type: :request do
|
||||
|
||||
expect(response).to have_http_status(:unprocessable_entity)
|
||||
end
|
||||
|
||||
it 'returns 409 when the call has already ended (caller hung up mid-ring)' do
|
||||
call.update!(status: 'no_answer')
|
||||
|
||||
post "/api/v1/accounts/#{account.id}/whatsapp_calls/#{call.id}/accept",
|
||||
params: { sdp_answer: 'sdp_answer' }, headers: agent.create_new_auth_token
|
||||
|
||||
expect(response).to have_http_status(:conflict)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/accounts/:account_id/whatsapp_calls/:id/reject' do
|
||||
|
||||
@@ -57,11 +57,11 @@ describe Whatsapp::CallService do
|
||||
.to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::AlreadyAccepted') }
|
||||
end
|
||||
|
||||
it 'raises NotRinging when the call has reached a terminal state' do
|
||||
it 'raises CallAlreadyEnded when the call has reached a terminal state' do
|
||||
call.update!(status: 'completed')
|
||||
|
||||
expect { described_class.new(call: call, agent: agent, sdp_answer: sdp_answer).accept }
|
||||
.to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::NotRinging') }
|
||||
.to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::CallAlreadyEnded') }
|
||||
end
|
||||
|
||||
it 'raises CallFailed when sdp_answer is missing' do
|
||||
|
||||
@@ -149,17 +149,39 @@ describe Whatsapp::IncomingCallService do
|
||||
end
|
||||
|
||||
describe 'terminate with no local row yet' do
|
||||
it 'logs and skips instead of materialising an inbound missed-call row' do
|
||||
allow(Rails.logger).to receive(:warn)
|
||||
# Unique per example: the 60s tombstone isn't rolled back between specs.
|
||||
let(:tombstone_call_id) { "wacid.#{SecureRandom.hex(6)}" }
|
||||
|
||||
after { Redis::Alfred.delete(format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: tombstone_call_id)) }
|
||||
|
||||
it 'tombstones the terminate instead of materialising an inbound missed-call row' do
|
||||
allow(ActionCable.server).to receive(:broadcast)
|
||||
|
||||
params = call_payload(event: 'terminate', duration: 0, terminate_reason: 'no_answer')
|
||||
params[:calls][0][:id] = tombstone_call_id
|
||||
|
||||
expect { described_class.new(inbox: inbox, params: params).perform }
|
||||
.not_to change(Call, :count)
|
||||
expect(Rails.logger).to have_received(:warn).with(/Terminate for unknown call/)
|
||||
key = format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: tombstone_call_id)
|
||||
expect(Redis::Alfred.get(key)).to be_present
|
||||
expect(ActionCable.server).not_to have_received(:broadcast)
|
||||
end
|
||||
|
||||
it 'finalizes the call as no_answer when the connect arrives after the tombstone' do
|
||||
allow(ActionCable.server).to receive(:broadcast)
|
||||
|
||||
terminate = call_payload(event: 'terminate', duration: 0, terminate_reason: 'no_answer')
|
||||
terminate[:calls][0][:id] = tombstone_call_id
|
||||
described_class.new(inbox: inbox, params: terminate).perform
|
||||
|
||||
connect = call_payload(event: 'connect', session: { sdp: 'v=0', sdp_type: 'offer' })
|
||||
connect[:calls][0][:id] = tombstone_call_id
|
||||
expect { described_class.new(inbox: inbox, params: connect).perform }
|
||||
.to change(Call, :count).by(1)
|
||||
expect(Call.find_by(provider_call_id: tombstone_call_id).status).to eq('no_answer')
|
||||
expect(ActionCable.server).to have_received(:broadcast)
|
||||
.with(anything, hash_including(event: 'voice_call.ended')).at_least(:once)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'outbound connect with no local row yet' do
|
||||
|
||||
Reference in New Issue
Block a user