From a3c7f3b20482ae8ea3a3ef76df7993c291e33a97 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:58:36 +0530 Subject: [PATCH] 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 --- .../dashboard/composables/useCallSession.js | 1 + app/javascript/dashboard/helper/voice.js | 11 ++- config/locales/en.yml | 1 + .../v1/accounts/whatsapp_calls_controller.rb | 6 ++ .../app/services/whatsapp/call_service.rb | 5 +- .../whatsapp/incoming_call_service.rb | 72 +++++++++++++++---- enterprise/lib/voice/call_errors.rb | 1 + lib/redis/redis_keys.rb | 2 + .../whatsapp_calls_controller_spec.rb | 9 +++ .../services/whatsapp/call_service_spec.rb | 4 +- .../whatsapp/incoming_call_service_spec.rb | 28 +++++++- 11 files changed, 117 insertions(+), 23 deletions(-) diff --git a/app/javascript/dashboard/composables/useCallSession.js b/app/javascript/dashboard/composables/useCallSession.js index 354f3a7b3..275a6337d 100644 --- a/app/javascript/dashboard/composables/useCallSession.js +++ b/app/javascript/dashboard/composables/useCallSession.js @@ -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); diff --git a/app/javascript/dashboard/helper/voice.js b/app/javascript/dashboard/helper/voice.js index 224dce88a..4f5ad25fe 100644 --- a/app/javascript/dashboard/helper/voice.js +++ b/app/javascript/dashboard/helper/voice.js @@ -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, diff --git a/config/locales/en.yml b/config/locales/en.yml index 22d3630af..d27c5d962 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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' diff --git a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb index 0301d428b..28dd378da 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -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 diff --git a/enterprise/app/services/whatsapp/call_service.rb b/enterprise/app/services/whatsapp/call_service.rb index 362af33cb..bd0dd6ae5 100644 --- a/enterprise/app/services/whatsapp/call_service.rb +++ b/enterprise/app/services/whatsapp/call_service.rb @@ -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! diff --git a/enterprise/app/services/whatsapp/incoming_call_service.rb b/enterprise/app/services/whatsapp/incoming_call_service.rb index f6185050d..11b35d95a 100644 --- a/enterprise/app/services/whatsapp/incoming_call_service.rb +++ b/enterprise/app/services/whatsapp/incoming_call_service.rb @@ -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. diff --git a/enterprise/lib/voice/call_errors.rb b/enterprise/lib/voice/call_errors.rb index 6b53ddbdc..e45edf0c0 100644 --- a/enterprise/lib/voice/call_errors.rb +++ b/enterprise/lib/voice/call_errors.rb @@ -8,4 +8,5 @@ module Voice::CallErrors class CallFailed < StandardError; end class NotRinging < StandardError; end class AlreadyAccepted < StandardError; end + class CallAlreadyEnded < StandardError; end end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index bc7fb551e..6d503df22 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -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::%s'.freeze OPENAI_CONVERSATION_KEY = 'OPEN_AI_CONVERSATION_KEY::V1::%s::%d::%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::%s'.freeze ## Sempahores / Locks # We don't want to process messages from the same sender concurrently to prevent creating double conversations diff --git a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb index 500255983..e0027e273 100644 --- a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb @@ -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 diff --git a/spec/enterprise/services/whatsapp/call_service_spec.rb b/spec/enterprise/services/whatsapp/call_service_spec.rb index 926771a65..4620ca588 100644 --- a/spec/enterprise/services/whatsapp/call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/call_service_spec.rb @@ -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 diff --git a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb index 4651b5f13..a3c5246d2 100644 --- a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb @@ -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